Run the declared graph matrix over MCP (closes #95, ADR-0026) - #109
Merged
Conversation
ADR-0021 deferred the graph twin explicitly and recorded the condition for revisiting it. That condition is met: the graph CLI and the shared graph.TestProject / result.GraphSuite model exist, so the gap closes by exposing what is there rather than inventing graph or evaluator semantics. experimental_test_graphs takes optional graph_id and supported_extensions and returns exactly the payload the graph project walk emits. It inherits the packs tool's disciplines deliberately: a literal JSON null argument refused rather than decoded into the most expensive run, strict decoding so a misspelled key is an error, presence kept separate from value so a present-but-empty id is refused, project.Present so a configuration that is there and will not load refuses, and an oversized report refused with its size rather than truncated. Writing nothing and consulting no reviewed set are two invariants, not one. Audit nil is what appends no record (ADR-0018); LawCheck nil is what consults no lock (ADR-0019). They are guarded independently in the graph layer, so each has its own test. The one discipline NOT inherited is where the report is bounded, and it is the substance of the change. A graph matrix multiplies where a pack matrix does not — up to 10,000 rows, each re-evaluating up to 64 nodes and retaining every node's canonical disposition — so a suite can reach gigabytes before a check on the marshaled response could see it, which is the shape ADR-0025 already names as a defect. graph.Options.ReportBudget bounds the suite AS it accumulates, refusing at the graph that carried it past. The CLI leaves it unset and streams to a terminal an operator can interrupt. The design was reviewed cross-vendor BEFORE implementation, per the interim review regime and the issue's own first scope item. All five findings accepted; F1 is why ReportBudget exists and F2 is why supported_extensions is decoded strictly rather than as a plain []string, which accepts a null collection and null elements. Two of my own tests were wrong and mutation checking found both. The MCP oversized-report test does not discriminate the accumulating budget — over a small fixture the post-marshal check refuses identically — so the budget is tested at the graph layer, where removing it fails the test. And my strict decoder still accepted [null], because encoding/json treats null into a string as a no-op: the same accidental null acceptance the review found one level out, reproduced one level in. Claim surface grows from seven to eight; CONFORMANCE.md, its mechanical enumeration test, and seven other inventory sites updated together. The author_graph prompt's closing step no longer says "where a terminal is available". Material-decision impact: adds one experimental MCP surface (ADR-0026) and one additive graph option; no graph format, evaluator semantics, disposition comparison or coverage derivation change. Signed-off-by: kikashy <kikashy@hotmail.com>
The design review passed; the implementation did not. Round 2 checked whether
the accepted findings actually landed in code, and three of five had not.
F1 was not fixed. The budget was checked BETWEEN graphs, so a single graph
declaring 10,000 rows still accumulated every one of them before anything
looked — the gigabytes the bound exists to prevent. Worse, ADR-0026 stated the
stronger claim as the substance of its decision. Enforcement moves into the row
loop where the retention happens, the refusal names how many rows were judged
so stopping early is observable, and it escapes as a run failure rather than
being swallowed as that graph's in-band mismatch. The ADR and changelog now say
what the code does, with the rejected first attempt recorded rather than edited
away.
F2 was incomplete. encoding/json matches member names case-insensitively, so
{"GRAPH_ID":"x"} bound to GraphID and passed DisallowUnknownFields against an
advertised additionalProperties:false. Member names are now checked exactly,
with three spelling regressions.
F4 was incomplete: README, the mcp subcommand's Long text, docs/mcp-clients.md
and the tools.go package comment all still said seven surfaces or two
experimental tools, and mcp-clients described graph matrices as instance
matrices run through the packs comparator.
Two claims in the tool description were false. There is no "canonical composite
result" — the composite headline disposition and any named-node dispositions
are canonicalized. And zero rows is not always skipped: a graph or rows
document that cannot be read is a mismatch carrying no rows.
The author_graph prompt recommended experimental_test_graphs at step 6, but the
tool only walks graphs declared in jpack.json and that declaration is proposed
at step 7 — so it could not test the graph just authored. The sequence now says
the CLI takes the document by path on the first pass and the tool closes the
loop afterwards, and ADR-0026's authoring-loop consequence is qualified to
match.
The CLI-parity test never invoked the CLI. It calls graph.TestProject and
supplies this surface's command string, so it is renamed to the layer-parity
test it is.
Both round-2 fixes mutation-checked: reverting either fails its test.
Signed-off-by: kikashy <kikashy@hotmail.com>
Round 3 found F1 closed for rows and still open for coverage: the coverage block is derived per graph and retained across the suite, and it repeats pack-derived probe strings across up to 64 nodes, so a rows-only bound left the multiplying component outside it. ADR-0026 and the changelog claimed the whole report was bounded. Again. Coverage is now charged as it is derived. The regression is built to separate the two bounds deliberately: its budget sits above every row's total and below rows plus coverage, so a rows-only bound does not refuse and this test fails. Mutation-checked by reverting to the round-3 state. The suite envelope — ids, paths, summaries — is deliberately not charged, and the ADR now says so rather than implying everything is. It follows the configuration's size rather than the matrix's, so it cannot multiply, and the caller's check on the finished report is what bounds it. The ADR records both rejected attempts. Each round the code was closer and the document still described the finished thing. Also: README said eight surfaces while enumerating seven, and the renamed layer-parity test still labelled the graph-layer payload "cli" in its failure output. Signed-off-by: kikashy <kikashy@hotmail.com>
…ating Round 4, fourth round on the same claim. Two things were open and both were the claim rather than the mechanism. The entry envelope was uncharged, and an entry's detail echoes values a MATRIX supplies — a rows loader's complaint, a declared matrix version — with no cap. Several graph entries can name one hostile rows file, so that is graph count times matrix-derived content, accumulated before anything looked. The ADR said the envelope could not multiply with the matrix. It could. The envelope is now charged as whatever the finished entry costs beyond the rows and coverage already charged while building it, so every retained component is charged exactly once. Detail is capped at MaxEntryDetailBytes besides, and a truncated detail says so rather than ending cleanly and misleading a reader. The residue is now stated instead of claimed away. Each component is metered AFTER it is composed, so a run can overshoot by at most one component before refusing: one row, one coverage block, or one envelope. Rows abort early, which is where a graph matrix actually multiplies. Coverage metering inside probe construction would eliminate the last of it and is explicitly NOT done, because it reaches into derivation this change is otherwise careful not to touch — the bound there is node count and pack content, not row count. The ADR now records all three rejected attempts. Each described the finished thing before it existed: between graphs, then rows-only, then rows-and-coverage with the envelope loose. Mutation-checked: reverting the envelope charge fails the new regression, which sets a budget covering exactly rows plus coverage. Signed-off-by: kikashy <kikashy@hotmail.com>
Round 5 found the same claim wrong a fifth time, and one part of it was flatly false: coverage derivation retains a witness per row and per expectedNodes entry, so its working memory DOES scale with row count. The ADR said "node count and pack content, not row count". Five rounds of trying to assert completeness is evidence that the completeness claim is the defect, not the mechanism. So the claim is now a mitigation with its gaps named, in the ADR, the changelog and the option's own comment: - components are metered AFTER composition, so a run overshoots by whichever component crosses the line; - the suite's outer envelope is never charged; - coverage derivation's working memory scales with the matrix, and mitigating that means restructuring derivation, which is out of scope and is recorded as an accepted residue rather than left silent. What the budget buys is the one thing that motivated it and the one thing tested: the remaining rows of a large matrix are never evaluated once it trips. The caller's check on the finished report backstops the rest. capDetail stated a cap and then exceeded it, appending its marker after filling the cap, and sliced bytes so it could cut a rune in half. Both fixed; the regression asserts the total fits and the result stays valid UTF-8, and it fails against the previous version. The ADR records all five rejected versions of this paragraph. Every round the code moved closer and the prose still described a finished thing. The prose was the defect. Signed-off-by: kikashy <kikashy@hotmail.com>
Round 6 was asked whether the list of named gaps was complete. It was not, and that was the finding worth having: the row slice is preallocated for the whole declared matrix before the first row is judged, so even a trip on row one costs memory proportional to the declared length. MaxRowsBytes bounds that, not this budget. It is distinct from the coverage witnesses already listed. Three places still claimed more than the code does. The ADR's own TITLE said "bound the report as it accumulates" — five words asserting exactly what five rounds established is not true. The changelog headline said the report "is bounded". And both the ADR and the option comment called the finished-report check a backstop for "everything else", when it backstops the response size and cannot backstop peak working memory: by the time it runs the memory is spent, which is precisely why the row loop needed its own check. The one positive claim is narrowed to what round 6 confirmed is true and tested: the remaining rows are never EVALUATED once the budget trips. That is work avoided, not memory reclaimed. Signed-off-by: kikashy <kikashy@hotmail.com>
The heading survived the rewrite that removed the claim from the body. Signed-off-by: kikashy <kikashy@hotmail.com>
Round 7 found a fifth gap: the rows document is fully decoded into Rows.Cases before the first budget check and stays live through evaluation and coverage, distinct from the preallocated result-row slice found in round 6. That is five rounds spent enumerating exceptions and being told the list is short by one. The enumeration was the defect. A list of gaps is a completeness claim wearing different clothes, and this change has now failed that claim seven times. So the scope is stated as an invariant instead, because an invariant cannot be short by one: ReportBudget constrains only the bytes RETAINED IN THE REPORT as it is composed. Everything a run holds in order to PRODUCE that report is outside it entirely. The second category is bounded by MaxRowsBytes and the carrier limits, and the decoded matrix, the preallocated slice and the coverage witnesses are given as examples explicitly NOT offered as exhaustive. Two limits inside the part it does constrain are still named exactly, because they are exact: metering happens after composition, and the outer envelope is never charged. Round 7 confirmed no other overclaim survives in the five text surfaces. Signed-off-by: kikashy <kikashy@hotmail.com>
Round 8 asked one question -- is the stated invariant true of the code -- and the answer was no, for a real reason rather than a wording one. graph.Test charged its rows and coverage but never its own GraphTest envelope; only TestProject charged an envelope. So a caller using Test directly with a budget, which the option's own comment contemplates, had one that escaped, and the invariant "only the outer suite envelope is uncharged" was false on that path. Test now charges its envelope as whatever the finished report costs beyond the components charged while building it. Inside TestProject the arithmetic self-corrects, because that charges the entry's remaining difference and the guard drops a non-positive remainder. Regression asserts a budget covering exactly rows plus coverage refuses on the direct path, and mutation-checked against the round-8 state. This is the first round in six where the finding was the mechanism rather than the prose. Stating the scope as an invariant did what enumerating gaps could not: it turned "the list is short by one" into a question with a checkable answer, and the answer found a defect. Signed-off-by: kikashy <kikashy@hotmail.com>
Round 9 found the round-8 fix wrong in the opposite direction. Test charged its GraphTest envelope on both paths, but inside TestProject that envelope is discarded — testEntry copies the status, summary, rows and coverage into a GraphSuiteEntry and drops the rest. So production-only bytes were consuming the budget, and the downstream positive-only remainder can neither subtract that nor undo a refusal it had already caused. The envelope is now charged only when reportSpent is nil, which is exactly the direct-Test caller that keeps the report. Inside TestProject the entry envelope is charged instead, as before. The invariant is "charge what is RETAINED", and it fails if either side is over-applied. Both directions are now pinned by their own regression: the composed path must NOT refuse at a budget covering everything the suite retains, and must refuse one byte below it; the direct path must refuse at a budget covering only its rows and coverage. Mutation-checked against both the round-7 state (charged on neither path) and the round-8 state (charged on both). Nine rounds, and this is the second consecutive one whose finding was the mechanism rather than the prose. The invariant did what the gap list could not: it made a wrong answer detectable in both directions instead of only the direction I happened to be looking. Signed-off-by: kikashy <kikashy@hotmail.com>
kikashy
force-pushed
the
mcp-test-graphs
branch
from
August 14, 2026 19:29
b8771e7 to
c2277df
Compare
|
🎉 Thank you for your contribution to Judgment Pack! If you’d like another task, choose a path that interests you: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #95. New ADR-0026.
Material-decision impact: adds one experimental MCP surface and one additive graph option. No graph format, evaluator semantics, disposition comparison, or coverage derivation changes.
ADR-0021 deferred the graph twin explicitly and recorded the condition for revisiting it. That condition is met, so this closes it by exposing
graph.TestProjectrather than inventing anything.Cross-vendor review, before implementation
Per the interim review regime and issue #95's own first scope item, the design was reviewed before any code was written — reviewer: codex-cli 0.145.0 / gpt-5.6-sol, ultra, read-only sandbox, reading the real code rather than the note alone. Verdict: "the wrapper's core semantics are sound, and the proposed call path has no hidden write… I would not implement the design unchanged."
All five findings accepted.
graph.Options.ReportBudgetbounds the suite as it accumulates, refusing at the graph that carried it past. CLI leaves it unset (it streams to an interruptible terminal); MCP sets it.[]stringdoes not enforce the advertised array-of-strings: Go accepts a null collection and null elements.supported_extensionsis decoded strictly viajson.RawMessage. Empty strings and duplicates stay inert, matching the evaluator's set semantics — per the reviewer's answer to my question 1.AuditandLawCheckare separate invariants, guarded independently.CONFORMANCE.mdand its mechanical enumeration test move seven → eight. ADR-0021's dated seven-surface consequence is left as written, per the reviewer.The reviewer also confirmed the thing I could not verify from my own reading: there is no hidden write path — audit append is guarded by
Audit != nil, reviewed-set checking independently byLawCheck != nil, and coverage only reads.Two of my own tests were wrong, and mutation checking found both
ReportBudgetremoved. The budget is therefore tested at the graph layer, where removing it fails the test — and the MCP test is labelled for what it actually proves (refusal, not truncation).[null].encoding/jsontreats null-into-string as a no-op and reports no error — the same accidental null acceptance the review found one level out, reproduced one level in. Now checked explicitly.Both mutation checks initially failed to compile rather than fail, which proves nothing; each was redone as a cleanly compiling original.
Verified end to end over a real MCP session
Tests cover CLI/MCP payload parity (byte-identical against
graph.TestProject), one-graph selection, unknown id, eight bad-argument forms, omitted-vs-empty-vs-duplicated extensions, absent configuration, response bounds, and the two no-write invariants separately.gofmtclean,go vetclean, fullgo test ./...green.The
author_graphprompt's closing step no longer says "where a terminal is available" — which was the point.🤖 Generated with Claude Code