Correct the Principle-1 over-read in the em-graph header comment (fixes #587)#607
Conversation
#587) Principle 1 bars a second store, not a rebuildable derived index. The header now keeps the Principle-1 bar on an external graph DB, names the freshness-versus-rebuild-cost trade as the real reason for the per-query build, and records that a persisted graph.json is an admissible derived index deferred to RFC-007 Phase 6 rather than forbidden. RFC-007 Part 1 and the deferral rationale no longer assert the comment is uncorrected, and the derived-index count there is corrected from four to five (tokens.json was missing). Comment and prose only: test-em-graph 10/10 and test-em-graph-nodes 28/28 unchanged, RFC registry consistent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lantiscooperdev
left a comment
There was a problem hiding this comment.
Tiered multi-agent pipeline — issue #587 slice, round 1 ACCEPT
Adversarial frozen-diff review of the Principle-1 comment correction, plus the RFC-side passages that would have gone stale the moment the code landed.
Verdict trail
| Round | Scope | Reviewer | Verdict | Artifact |
|---|---|---|---|---|
| Scout ×3 | code surface, governing-artifact grounding, repo-wide same-class sweep | Claude Sonnet (read-only) | maps returned, all claims orchestrator-ground-truthed | in-session |
| 1 | frozen diff vs frozen spec, 6 named attack axes | pi --provider kimi-coding --model k3 |
ACCEPT — 0 blockers, 0 minors, 2 nits | docs/plans/issue-587-em-graph-principle-comment.md §19 |
Review seat was directed by the operator in-session, replacing the standing GLM/neuralwatt reviewer.
What the review actually verified
Clause-by-clause, each with captured output rather than assertion:
| Clause of the new comment | Verdict | Evidence |
|---|---|---|
| "an external graph DB (Neo4j, Memgraph) is barred by Principle 1, which admits no second store" | TRUE | PRINCIPLES.md:13 parenthetical; RFC-007:363 |
"built fresh from index rows + one body pass when cites is selected" |
TRUE | em-graph.mjs cites-gated body pass; only fs call is a readFileSync |
| "always current with zero staleness surface" | TRUE | corroborated verbatim by RFC-007:166 |
| "rebuildable derived index … admissible under Principle 1 and CAPABILITIES.md family 1" | TRUE | CAPABILITIES.md:42-46; identical wording at RFC-007:248 |
| "deferred by RFC-007 to Phase 6 (trigger: rebuild cost per query becomes the binding constraint)" | TRUE | RFC-007:387 Phase 6 row |
| comment does not lie about its own module | PASS | grep -cE "writeFileSync|renameSync|mkdirSync|appendFileSync" scripts/em-graph.mjs → 0 |
| house style preserved | PASS | hexdump confirms e2 80 94 (U+2014); awk confirms 56-77 col widths |
Attack axes that came back clean: SQLite omission is correct (the comment scopes its Principle-1 bar to exactly the pair :363 scopes it to, and an omission cannot be false); "admissible" is RFC-007's own term, not an overreach; RFC-007:220 is internally coherent across its two-pass edit (tense arc formerly → analysis → applied); and a repo-wide sweep found no other site still asserting or quoting the comment as current.
Two orchestrator-side defects caught by independent re-verification
Recorded because the seat reported success on both:
- Under-scoped spec step. The original Step 4 replaced only the trailing sentence of
RFC-007:220, leaving the paragraph opening asserting in the present tense that the comment was still uncorrected, still carrying the decayed:29-31anchor. Fixed by an amended Step 8;grep -c "currently justifies"→0,grep -c "em-graph.mjs:29-31"→0. - Seat stale-green. The builder reported verifies "1a/1b/1c/3/4a/4b/5a/5b/6a" and silently omitted 6b, the one that failed. Per the spec's §A.3 it owed a STOP block. The orchestrator's independent re-run is what caught it.
The seat did find one real bug in the spec: Verify 1d's regex included graph\.json, which the new comment legitimately contains, making the verify unsatisfiable. REQ-6 now greps only the fs call sites.
Test posture (final)
node tests/test-em-graph.mjs→10 passed, 0 failed(unchanged)node tests/test-em-graph-nodes.mjs→28 passed, 0 failed(unchanged)node scripts/em-rfc-validate.mjs→ 14 RFCs consistentnode scripts/em-graph.mjs --hubs --top 3 --scope local→ valid JSON,"status":"ok", exit 0- CI: 8/8 checks green
No test was added. Nothing new is behaviorally testable, and a test asserting comment text would pin prose and make future honest rewording a failure. Stated so the absence is a decision, not an oversight.
Follow-ups filed (Rule 18 step 9)
- #606 —
RFC-007:259-262cites the Alternatives table for Phase 4 and Phase 6 (real rows are:385and:387;:363is the external-graph-DB row). Issue #587's own body and workplan v265 both inherited that bad anchor. All 5 DEFER fields. - #606 comment — nit 1 from this round: RFC-007 attributes the SQLite rejection to Principle 1 at
:18/:335but to the zero-deps pin at:364. Pre-existing at2df117f, not introduced here. - Nit 2 ("Issue #587 tracked that code-side fix" past tense) dispositioned REJECT, no change: the RFC is read in its merged state, at which point #587 is closed and the tense is correct.
What user approval is checking
Per Rule 17, your approval in the UI is the review-required gate. Substantive review is complete and closed ACCEPT with zero blockers. The judgement calls worth your eye are the two accepted deviations: naming documents and sections instead of the line anchors #587 proposed, and adding no Implementation-ledger row. Please do a final read of the diff and approve, or HOLD with findings.
🤖 Posted by lantiscooperdev (bot account) — Claude Opus 5 (1M context).
…al run The plan doc committed with 19 and 18 still reading pending even though the round-1 ACCEPT and the CI run had already happened. Records the kimi k3 verdict, both nit dispositions, the two accepted deviations, the observed CI result, and the bot review id. No code or RFC change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixes #587.
scripts/em-graph.mjs:29-31cited Principle 1 to justify not persisting a derived graph index. That over-reads the principle:PRINCIPLES.md:13bars a second store ("No feature may introduce a second store (queue, socket, sidecar database)"), and a rebuildable derived index is not a store.CAPABILITIES.md:43-46positively sanctions "a derived knowledge-graph index" as a family-1 memory-store concern, and RFC-007 sequences the persistedgraph.jsonas Phase 6 DEFERRED, not rejected.This is the code-side mirror of the RFC correction that shipped in #586 (
8808dd7), whose commit message recorded the obligation, including that the correct Principle-1 bar on an external graph DB must be preserved.What changed
scripts/em-graph.mjsheader comment, 3 lines to 10. It now:graph.jsonwould be a rebuildable derived index, not a second store, admissible and deferred by RFC-007 to Phase 6 with the trigger condition named, not ruled out.docs/rfcs/RFC-007-graph-projection.md, three passages that asserted the comment was still uncorrected. Left unfixed, the RFC would have become false the moment this landed, which is the same doc/code divergence #581 existed to close::218and:220/:251no longer carry theem-graph.mjs:29-33/:29-31line anchors into a comment block, since that is the decay class this PR cleans up;:220and:251flip from "the code comment is to be corrected in a later phase" to the corrected state;:220's derived-index count is corrected from four to five:tokens.jsonwas missing (scripts/em-rebuild-index.mjs:317-318, and enumerated asderivedNamesalongside the other four atscripts/em-backup.mjs:1740). Issue em-graph.mjs:29-31 cites Principle 1 to reject a persisted derived index (over-read) #587's own body carried the same undercount.docs/plans/issue-587-em-graph-principle-comment.md, the frozen build spec, committed with the slice per repo convention.Deliberately not changed
docs/EM_SCRIPTS_GUIDE.md:734-735cites no principle and its "no sidecar DB" claim remains true, so it is not same-class and not drift.docs/plans/RFC-007-P2-rule-rfc-nodes.md:330quotes the old comment as a record of a merged slice. Shipped plan docs are history and are not rewritten.:443-447. A comment fix is not a phase, and a row would need a merge SHA, i.e. a placeholder plus a follow-up PR, which is exactly the Two pre-existing RFC ledger rows cite feature-branch tip SHAs instead of main-merge commits #582/Fill the RFC-007 Phase 2 ledger provenance from git, not a placeholder #605 pattern. The:220rewrite describes the corrected state directly and needs no SHA.Verification
All run by the orchestrator on disk, not accepted from a seat report.
node tests/test-em-graph.mjs10 passed, 0 failednode tests/test-em-graph-nodes.mjs28 passed, 0 failednode scripts/em-rfc-validate.mjs✓ RFC registry consistent: 14 RFCs in _index.json, 14 canonical files, 14 README table rows.grep -cE "writeFileSync|renameSync|mkdirSync|appendFileSync|writeFile\(" scripts/em-graph.mjs0grep -c "graph DB (Principle 1)" scripts/em-graph.mjs0grep -c "barred by Principle 1" scripts/em-graph.mjs1grep -c "em-graph.mjs:29-31" docs/rfcs/RFC-007-graph-projection.md0node scripts/em-graph.mjs --hubs --top 3 --scope local"status":"ok", exit 0A repo-wide same-class sweep over 83 entries under
scripts/, all 25 RFCs, 30 plans,README.md,CAPABILITIES.md,PRINCIPLES.md,CLAUDE.md,instructions/, andtests/found this to be the only site in the repo that over-read Principle 1 against a derived index. Every other Principle-1 citation correctly targets a genuine second store, and no Principle number inscripts/is stale relative toPRINCIPLES.md. No test pins the comment text, so the reword cannot produce a hollow green.Review
Adversarial frozen-diff review by
pi --provider kimi-coding --model k3: ACCEPT, 0 blockers, 0 minors, 2 nits. It verified each clause of the new comment againstPRINCIPLES.md,CAPABILITIES.md, and RFC-007 with captured output, confirmed the U+2014 byte sequence byhexdumpand line widths byawk, independently confirmed the five-index count, and ruled ACCEPT on both flagged deviations (naming documents and sections instead of line anchors; no ledger row).Nit dispositions: the RFC-internal SQLite attribution split (
:18/:335under Principle 1 versus:364under the zero-deps pin) is pre-existing, not introduced here, and is filed as a comment on #606. The "tracked" past tense at:220is rejected as no-change, since the RFC is read in its merged state once #587 is closed.Follow-up filed
#606 —
RFC-007:259-262cites the Alternatives table for Phase 4 and Phase 6. Phase 4 is at:385and Phase 6 at:387, while:361is the Alternatives table header and:363is the external-graph-DB row. Issue #587's body and workplan v265 both inherited that bad anchor. Filed with all 5 DEFER fields.