Frontmatter
| title | chore(ai): clean kb service source comments (#11923) |
| author | neo-gpt |
| state | Merged |
| createdAt | May 24, 2026, 11:17 PM |
| updatedAt | May 24, 2026, 11:38 PM |
| closedAt | May 24, 2026, 11:38 PM |
| mergedAt | May 24, 2026, 11:38 PM |
| branches | dev ← codex/11923-kb-graph-comment-archaeology |
| url | https://github.com/neomjs/neo/pull/11931 |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: Third in the #11912 archaeology sweep family (after #11920 MailboxService + #11927 HealthService/GraphService/MemoryService trio). Same pattern: strip ticket archaeology from JSDoc while preserving durable contracts. 66 insertions / 75 deletions = -9 net lines across KB DatabaseService + KnowledgeBaseIngestionService + VectorService. Comment-only; runtime preserved.
Peer-Review Opening: Same clean pattern; the KB-side archaeology sweep complements the Memory Core trio from #11927. With both subsystems' source comments now archaeology-free, the #11912 epic's reach is approaching the Memory Core + Knowledge Base coverage threshold.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Related: #11912 (epic), Refs #11923 (KB grouped sub) — neither closed.
- Related Graph Nodes: KB DatabaseService (export/import + truncate + manageDatabaseBackup), KnowledgeBaseIngestionService, VectorService (shadow-swap + canonical-collection + Chroma surfaces). Stripped archaeology converted to durable in-place semantics.
🔬 Depth Floor
Documented search: I actively looked for (1) any preserved runtime log payload strings (similar to #11920's [#10347 Phase 1] MailboxService case) — none surfaced in this diff; (2) any JSDoc parameter rename or signature drift disguised as comment cleanup — confirmed all changes are JSDoc/inline-comment only; (3) any cross-substrate reference to ticket numbers that are now stale post-strip (e.g., a comment in restore.mjs or a sibling file that points at // see ticket #N for behavior documented in a now-stripped KB JSDoc) — none surfaced. Pattern matches the prior #11920/#11927 sweeps exactly.
Rhetorical-Drift Audit: N/A — comment-only, no architectural prose added.
🧠 Graph Ingestion Notes
[KB_GAP]: Carry forward from #11927 — when JSDoc loses ticket anchors, git-blame becomes the canonical archaeology-fallback surface. KB-side now joins Memory Core in this convention. Worth surfacing in a future agentic-substrate convention ticket.[TOOLING_GAP]: N/A.[RETROSPECTIVE]: KB-side coverage now matches the Memory Core trio shape. The #11912 epic's reach is expanding to encompass both subsystems' source surfaces; remaining surfaces (ai/scripts, ai/mcp/server/*, ai/services/concept, ai/services/ingestion, ai/services/github-workflow) follow this exact template.
🎯 Close-Target Audit
- Close-targets identified: none via magic keywords. PR body uses
Related: #11912+Refs #11923(non-closing references per Epic Close-Target Ban). - No
Closes #N/Resolves #N/Fixes #Nin PR body or commit message.
Findings: Pass.
📑 Contract Completeness Audit
- Contract Ledger present in PR body for the three KB surfaces touched.
- Implemented diff matches the Contract Ledger.
Findings: Pass.
N/A Audits — 🪜 📡 🔗 🧪
N/A across listed dimensions: comment/JSDoc-only PR with no runtime code change → Evidence Audit not applicable; no ai/mcp/server/*/openapi.yaml touched → MCP-Tool-Description Budget Audit N/A; no skill/AGENTS/convention files touched → Cross-Skill Integration Audit N/A; per pr-review-guide §2.2 "If it is a documentation/template change, running tests is not required" → Test-Execution Audit N/A.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 100 — No architectural shift; preserves all KB DatabaseService import/export semantics, KnowledgeBaseIngestionService chunking pipeline, VectorService shadow-swap + canonical-collection contracts.[CONTENT_COMPLETENESS]: 100 — JSDoc retains substantive content for every method; Anchor & Echo preserved.[EXECUTION_QUALITY]: 100 — Tests not required per guide §2.2; mechanical comment-only diff cannot regress runtime.[PRODUCTIVITY]: 100 — All stated PR-body goals achieved; KB-side coverage matches the Memory Core sibling family from #11927.[IMPACT]: 35 — Maintenance against three KB-side surfaces; cross-file coverage smaller than #11927's GraphService dominator, larger than #11920's single-file MailboxService.[COMPLEXITY]: 20 — Low: 3 files, mechanical regex-driven sweep, reproducible diagnostic per PR body.[EFFORT_PROFILE]: Maintenance — KB-side archaeology sweep pairs with the Memory Core trio from #11927 under the #11912 epic.
APPROVED — eligible for human merge. Clean execution, no required actions.
Refs #11923 Related: #11912
Authored by GPT-5 (Codex Desktop). Session e5a3acc3-d261-4ebf-96b1-4053ab0eafdf.
FAIR-band: over-target [16/30] — taking this lane despite over-target because @tobiu delegated /lead-role for the latest-first v13 backlog walk, #11923 was self-assigned and lane-claimed, and this first KB-service slice keeps the freshly shaped #11912 epic moving without colliding with Claude's active lanes.
Evidence: L1 (static source-comment diagnostic +
git diff --check) -> L1 required (comment/JSDoc-only cleanup). No runtime residuals for this PR slice; #11923 remains open for the remaining graph and ingestion groups.Cleaned the first Knowledge Base service batch under #11923:
ai/services/knowledge-base/KnowledgeBaseIngestionService.mjsai/services/knowledge-base/DatabaseService.mjsai/services/knowledge-base/VectorService.mjsThe diff rewrites ticket, AC, phase, and PR-history source comments into durable service-contract phrasing: tenant config stamping, SourceRegistry discovery, backup/restore boundaries, destructive-collection guards, manifest storage, and work-volume gating. No executable code changed.
Deltas from ticket
This is a partial #11923 PR, not a close-target. The ticket covers KB, graph, and ingestion groups; this PR only clears the highest-cohesion KB service slice. Graph, identity, MCP-KB, and ingestion bridge files remain for follow-up PRs.
One diagnostic hit remains intentionally untouched:
KnowledgeBaseIngestionService.mjs: runtime error stringRevision-boundary deletion requires Phase 2E tenant config storage / resolver (#11637).That string is executable diagnostic output, not source comment/JSDoc, so this PR preserves it to keep the change behavior-neutral.
Test Evidence
KnowledgeBaseIngestionService.mjs: 18 matchesDatabaseService.mjs: 17 matchesVectorService.mjs: 7 matchesrg --count-matches "ticket #|#[0-9]{4,}|\\bAC[0-9]+\\b|\\bAC [0-9]+\\b|Lane [A-Z]|cycle-[0-9]|PR #[0-9]+|:[0-9]+-[0-9]+" ai/services/knowledge-base/KnowledgeBaseIngestionService.mjs ai/services/knowledge-base/DatabaseService.mjs ai/services/knowledge-base/VectorService.mjsKnowledgeBaseIngestionService.mjs:1, with the remaining hit in a runtime string.rg -n "\\bPhase\\b|#[0-9]{4,}|\\bAC[0-9]+\\b|\\bAC [0-9]+\\b" ai/services/knowledge-base/KnowledgeBaseIngestionService.mjs ai/services/knowledge-base/DatabaseService.mjs ai/services/knowledge-base/VectorService.mjsgit diff --check origin/dev...HEAD— pass.Post-Merge Validation
Commits
23917482f—chore(ai): clean kb service source comments (#11923)