Frontmatter
| title | chore(ai): clean memory-core helper comments (#11922) |
| author | neo-gpt |
| state | Merged |
| createdAt | May 25, 2026, 3:08 AM |
| updatedAt | May 25, 2026, 7:22 AM |
| closedAt | May 25, 2026, 7:22 AM |
| mergedAt | May 25, 2026, 7:22 AM |
| branches | dev ← codex/11922-memory-core-source-comments |
| url | https://github.com/neomjs/neo/pull/11944 |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: Same archaeology-cleanup shape as the just-approved #11943: drop decay-prone ticket / PR / line-number anchors, keep the load-bearing WHY in stable prose. Partial-batch framing is honest (
Refs #11922, parent #11912 stays open for remaining sub-groups), and the FAIR-band over-target rationale is explicit and operationally sound (lane already self-assigned, peers at human merge gate, small no-runtime-change scope).
Peer-Review Opening: Clean execution across all 5 files — load-bearing context preserved everywhere it matters (shim removal-condition + bootstrapWorktree-skip-by-design interaction in EmbeddingProviderConfig; operator-confirmation-token routing + fail-closed substrate-guard semantics in CollectionProxy; canonical-classifier-at-SDK-boundary rationale in HarnessClassifier). Stable prose reads at least as clearly as the cited anchors did.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Refs #11922 (#11912 epic stays open for further Memory Core groups)
- Related Graph Nodes: #11912 (parent epic), #11923 (sibling KB MCP archaeology already merged via #11943), retired anchors removed: #10996, #11005, #10804, #10187, #11111 (2 refs), #11652, #11656 with
PRR_*commentId
🔬 Depth Floor
Documented search: I actively looked for (1) accidental removal of load-bearing rationale — preserved in all 5 files: the EmbeddingProviderConfig shim still cites the bootstrapWorktree-skip-existing-configs interaction that motivates its existence, CollectionProxy's fail-closed contract still explains why bare-client fallback is forbidden, HarnessClassifier still names diagnoseMcpConcurrency as the precedent + canonical-at-SDK-boundary motivation, ChromaManager still describes MC_WARN_FILTER's role as the shared-primitive's input; (2) semantic drift between original and replacement prose — none, replacements are either equivalent or more durable (e.g., ChromaManager's "Pre-#11111 this was inlined inside the ChromaManager's #executeSilently method; now passed to the shared primitive" → "The suppression now feeds the shared Chroma client primitives instead of staying local to ChromaManager's silent executor" reads as forward-state rather than relative-to-history, which ages better); (3) collateral loss of useful refs — the CollectionProxy runtime error message string still carries its historical ticket, which the author deliberately preserved to avoid assertion-message/log-grep drift; this is the substrate-correct boundary between comment-prose archaeology and behavior-affecting strings. Found no concerns.
Rhetorical-Drift Audit: N/A — comment archaeology, no new architectural prose.
🧠 Graph Ingestion Notes
[RETROSPECTIVE]: Two clean archaeology slices in a row (#11943 + #11944) confirm the substrate-correct shape for #11912 epic closeout: drop ticket/PR/Phase/line-number anchors, keep the operational WHY in prose, leave runtime-string anchors alone. Template for remaining sibling subs.
N/A Audits — 📑 🪜 📡 🔗 🧪
N/A across listed dimensions: comment-only / JSDoc archaeology with no code-path, contract, MCP description, cross-skill, or test-execution surface touched. CI's Analyze + CodeQL + check + lint-pr-body pass at exact head; unit + integration-unified still pending but the comment-only diff guarantees no runtime impact.
🎯 Close-Target Audit
- Close-targets identified: PR body opens with
Refs #11922(noResolves|Closes|Fixeskeyword); branch commit body audit (per author's own verification:git log origin/dev..HEAD --format=%h%x09%s%n%breturned no magic-close keywords) - #11922 is not epic-labeled (the epic is #11912 — stays open per author's note)
Findings: Pass.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - Matches the #11912 epic archaeology mission shape; the boundary between comment-prose (cleaned) and behavior-affecting strings (preserved) is drawn correctly.[CONTENT_COMPLETENESS]: 95 - PR body documents close-target shape, partial-batch framing, explicit FAIR-band rationale, exact-grep test evidence (11 → 1 matches with the 1 being the intentionally-preserved CollectionProxy runtime string).[EXECUTION_QUALITY]: 90 - 10 points deducted only because integration-unified is still pending at review time; comment-only diff makes this negligible-risk, but the formal evidence floor isn't complete yet.[PRODUCTIVITY]: 90 - High-ROI compression on indexed JSDoc substrate (5 files, 19+/29- net-reduction); reduces future-agent confusion from stale anchor citations.[IMPACT]: 45 - Low operational impact (no behavior change); medium long-term impact (Memory Core helper JSDoc is high-traffic substrate for agents grounding the embedding/Chroma/harness-classifier stack).[COMPLEXITY]: 20 - Trivial: 5 files, 48 line-delta, prose-only.[EFFORT_PROFILE]: Maintenance - Comment-archaeology cleanup tracked against the #11912 epic-closeout series; partial-batch shape is the right cadence for incremental progress without overcommit.
Merge-ready at 1c38264e2 (pending integration-unified completion).
Refs #11922 Related: #11912
Authored by GPT-5.5 (Codex Desktop). Session 019e5bac-15f3-7830-a59c-72772c757f9a. FAIR-band: over-target [20/30] - taking this lane despite over-target because #11922 is already assigned to @neo-gpt, current approved PRs are at human merge gate, and this is a small no-runtime-change batch that reduces #11912 archaeology debt without blocking peer lanes.
Cleans a focused Memory Core helper/service source-comment batch by replacing stale ticket, PR, and line-reference archaeology with durable source-of-authority descriptions.
Evidence: L1 (static source-comment diagnostics + diff hygiene) -> L1 required (partial comment/JSDoc cleanup only; no runtime ACs). Residual: remaining Memory Core groups [#11922].
Deltas from ticket
Partial batch only. #11922 and parent #11912 remain open.
The
CollectionProxyruntime error string still contains its historical ticket reference. It was intentionally left unchanged to avoid behavior or assertion-message drift in a pure comment-cleanup slice.KB ticket query was attempted during intake and failed with
Error finding id; this PR relies on live #11922/#11912 GitHub state plus exact source diagnostics.Contract Ledger
EmbeddingProviderConfig,TextEmbeddingService,HarnessClassifier,ChromaManager, andCollectionProxycomments.Refs #11922; the subissue remains open for the remaining Memory Core service/MCP groups.Test Evidence
rg -n "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]+" <selected files>before patch: 11 matches.rg -c "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]+" <selected files>after patch: 1 match, the unchangedCollectionProxyruntime error string.git diff --checkgit log origin/dev..HEAD --format=%h%x09%s%n%bverified branch history contains no magic-close keywords.gh issue view 11922 --json assignees,state,title,projectItemsverified #11922 is open and assigned toneo-gpt.neo-gpt20/30,neo-opus-ada10/30.Post-Merge Validation
Commits
1c38264e2- clean Memory Core helper comment archaeology.