Skip to content

Commit 8658ebd

Browse files
jitokimoh-my-graph
andauthored
docs(adr): 0033 — the run is the unit of evidence, not the node (#218) (#233)
* docs(adr): the run is the unit of evidence, not the node (ADR 0033) (#218) #218 read 53 of 73 planned nodes denied a tool call, 49 of those recorded PASS, and 44 of the 49 holding no engine-run check at all. The measurement's own reading of why is the sentence this record is about: only 8 of the 73 declare a `success_check.verify`, and each is the terminal `check` node of its run — "a verify is a per-run gate in these graphs, never a per-node one." The question put to this ADR was whether the engine should require evidence from every node. It answers no, and argues it rather than asserting it. ADR 0030's argument — an unverified run is a choice, not a default — does not instantiate one level down. A run has one sink whose exit code the operator chose in advance; a node's PASS is a model's sentence about its own work, and at the moment the operator would have to author a check for it the node does not exist, because its id comes from the planner. Six candidate suppliers are weighed. One of them — the node checking itself through its own Bash grant — supplies nothing the engine runs, and is rejected on 0213b's measurement rather than on trust. Of the five that would supply an engine-run command, two are shut by guards that already exist: an engine-derived per-node default is forbidden by ADR 0030 §3.5 ("detection may gate a refusal; it may never derive a grant"), and the goal loop choosing the string is the same class `validatePlannedNodeVerify` refuses. That leaves the operator and nobody. A per-node mapping cannot be typed before the plan invents the ids, and the one command an operator can type before the plan exists is one command — `make local` on a documentation node measures source that node never touched, so it is the run's one check, sampled N times, charged N times. Cost, from the corpus rather than guessed, with every figure addressed by line to the evidence brief on disk rather than to a session that has since ended: 81 planned nodes over 18 runs, 78 records carrying $159.27 and 17,244.1 s of model wall-clock. Attaching `make local` to all of them adds 21.6% wall-clock and $0 model spend; `go build ./...` adds 0.73%. The two verify denominators are kept apart and reconciled — 14 of the 81 planned nodes declare one, 8 of the 73 with a readable transcript do, and the six-node gap is itemised by run and node id — so the increment row subtracts 14, not 8. The record says plainly that the cost is not what decided it, so a later reader who finds a cheap command does not read it as a repeal. The measurement's caveats travel with its numbers: self-measurement, no rate (effective n nearer 18 than 73), a corpus that grows while it is read, the 8 nodes excluded from numerator and denominator both, and two undetected denial classes that make every count a floor. Nothing in the engine changes. The planner's reply stays untrusted, the sink filter stays where it is, and no fifth exec seam is proposed — anything required would have gone through verify.ShellVerifier, which already exists. §7 names the coverage census over the 81 nodes that would overturn this. Status: Proposed. Documentation only; the CHANGELOG entry is filed under ### Documented and says what was decided, not what was measured. Signed-off-by: jitokim <pigberger70@gmail.com> Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> * docs(adr): fix two addresses ADR 0033 got wrong (#218) Review found two defects in the record, both of the kind it argues against elsewhere in its own text: an address that says the opposite of the claim resting on it, and a quote marked verbatim that had a clause cut out of it. The spawn address. §1.5 and §2.3 both cited `internal/verify/shell.go:133` as where `ShellVerifier` spawns. That line is `cmd := exec.CommandContext(...)` inside `buildCmd`, which only assembles an `*exec.Cmd`; the comment directly above it, `:126-127`, says so in as many words — "Nothing here spawns; Verify wires it to the OS." The spawn is `:171`, `v.buildCmd(cmdCtx, req).CombinedOutput()`. Both citations now name the build at `:133` and the spawn at `:171`, which matters most in §2.3, where the whole $0-model-spend row rests on what that call does and does not launch. The env-scrub address at `:135` was correct and is unchanged. The error was inherited verbatim from the evidence brief (`read-evidence.out:338`), so it is cut here rather than carried further. The truncated quote. §2.2(d) quoted `0218:467-472` as ending at "…then the uncovered majority needs *some* signal." The source continues: "…, and a prose match as an advisory becomes the primary deliverable rather than a garnish." The clause was dropped without an ellipsis, and it is not decoration — it names a second condition that overturns §2.5, which refuses to build the denial advisory. The quote is restored in full, and §6 gains F5: F1 and F5 are two sides of §7's one census, and a result below F1's one-third threshold leaves §2.1 standing while knocking §2.5 down, obliging the advisory to be built and §2.5's objections to it (`0218:137-145`, `:369-374`) to be answered on their own terms rather than reused to decline again. §7's heading now names both. Documentation only, and the decision is unchanged, so the CHANGELOG entry under [Unreleased] still describes it correctly and is untouched. `make local` green. Signed-off-by: jitokim <pigberger70@gmail.com> Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> * docs: state the run-unit evidence rule where readers meet it (ADR 0033) ADR 0033 §3.1 calls two sentences "the entire deliverable", and until now neither existed outside the ADR: README.md and DESIGN.md were untouched by this branch. README.md carried only the positive half at the --verify-cmd example — "the ENGINE runs your build command at each sink of the plan". The complement now follows the Quickstart block: engine-run evidence is established per RUN, a planned non-sink node carries no engine-run verify, and its PASS is the subprocess exit status and its own sentence. DESIGN.md explained why validatePlannedNodeVerify refuses a planner-authored verify: but never what that refusal costs. The passage now says it — coverage reaches at most the sinks, so an interior planned node's verdict is its own word. Neither addition is a warning without an action: both name the same route, `run` on a hand-written graph with the author's own `verify:`. CHANGELOG.md extends the existing ADR 0033 entry under [Unreleased] with one bullet naming these two sentences. No decision text changed, no code, no flag. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> --------- Signed-off-by: jitokim <pigberger70@gmail.com> Co-authored-by: oh-my-graph <graphs@oh-my-graph.dev>
1 parent a65d518 commit 8658ebd

4 files changed

Lines changed: 683 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,31 @@ oh-my-graph is **alpha software**. The graph YAML schema, the CLI, and the
142142
cancelled context still stop on the first answer — retrying those is
143143
re-rolling a verdict until the loop likes one.
144144

145+
### Documented
146+
147+
- **ADR 0033 — the run, not the node, is the unit of engine-run evidence, and
148+
ADR 0030 is deliberately not extended one level down**
149+
([`docs/adr/0033-the-run-is-the-unit-of-evidence-not-the-node.md`](docs/adr/0033-the-run-is-the-unit-of-evidence-not-the-node.md),
150+
answering [#218](https://github.com/jitokim/oh-my-graph/issues/218)). A
151+
planned node cannot carry its own `success_check.verify` and never will —
152+
`validatePlannedNodeVerify` refuses a planner-authored one — and no supplier
153+
for a per-node command keeps that reply untrusted: an operator cannot type a
154+
mapping keyed on ids the planner has not invented yet, and the one command
155+
they *can* type before the plan exists would measure the same thing at every
156+
node instead of each node's own work. So `--verify-cmd` stays a per-run
157+
command attached to a run's sinks, the deliverable is documentation, and §7
158+
names the coverage census that would overturn it. **Proposed. No behaviour
159+
changed, no flag, no schema field, no new exec seam.**
160+
161+
- That deliverable is now written where a reader meets the gap, not only in
162+
the ADR: the `--verify-cmd` example in [`README.md`](README.md) gains the
163+
complement of its positive half — evidence is established per RUN, so a
164+
planned non-sink node carries no engine-run verify and its `PASS` is the
165+
subprocess exit status and its own sentence — and the passage in
166+
[`DESIGN.md`](DESIGN.md) that explains `validatePlannedNodeVerify`'s
167+
refusal now says what that refusal costs in coverage. Both name the same
168+
route rather than warning without one: `run`, on a hand-written graph, with
169+
your own `verify:` on whichever nodes you mean.
145170
- **Five documentation sentences that v0.11.0 made false**, each corrected
146171
against the code rather than against the surrounding prose. `docs/EXAMPLES.md`
147172
told a reader whose `auto` run depended on an MCP server that it "will stop

DESIGN.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1020,7 +1020,13 @@ planned graph carries no verification a MODEL wrote either —
10201020
`validatePlannedNodeVerify` refuses a planner-authored `verify:`, so the only
10211021
one it can carry is the `--verify-cmd` the user supplied, which is
10221022
advisory-eligible like any other command line but is still the user's own
1023-
string.
1023+
string. What that refusal costs is coverage, and it is worth stating:
1024+
engine-run evidence in a planned graph is established per RUN — the user's
1025+
`--verify-cmd` attaches at the graph's sinks — so an interior planned node's
1026+
verdict is its own word plus the subprocess exit status, by construction and
1027+
not by omission (ADR 0033). The supported route to interior coverage is `run`
1028+
on a hand-written graph, which writes `verify:` on whichever node its author
1029+
means (`DESIGN.md:1589`).
10241030

10251031
```go
10261032
type SuccessCheck struct {

README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,13 @@ mkdir -p /tmp/omg-smoke
7070
oh-my-graph run graphs/haiku-smoke.yaml --input dir=/tmp/omg-smoke
7171
```
7272

73+
Engine-run evidence is established per RUN, not per node: `--verify-cmd`
74+
attaches your command to the sinks of the plan (ADR 0030), and a planned
75+
non-sink node carries no engine-run verify at all — its `PASS` is the
76+
subprocess exit status and its own sentence, by construction and not by
77+
omission. If you want an interior node checked, write your own `verify:` on it
78+
in a hand-written graph and run it with `run` (ADR 0033).
79+
7380
Log in once with the selected CLI (`claude` or `codex login`). No API key is
7481
needed: Anthropic and OpenAI API-key variables are deleted from child process
7582
environments so the CLI uses its saved login. The default remains Claude;

0 commit comments

Comments
 (0)