Commit a3493ed
feat(handoff): a feedback arc and its quote are one mechanism — sweep it, and refuse the planned one (#195)
* docs(adr): a feedback arc and its quote are one mechanism
ADR 0028. `feedback: { rerun: R }` and `{{ feedback.<declarer> }}` are two
halves of ADR 0010's repair loop, and until now either half loaded clean
alone: validateFeedback judges the arc, validateFeedbackPlaceholders judges
a token that IS written, LintFeedbackReach judges the arc's aim — none asks
whether both halves are present.
Run 20260816-163759.091162000-1 is the specimen: a correct arc, a build
prompt that branches on "if a FEEDBACK section appears below", and no token
to put one there. The loop re-ran, produced identical output, and failed
identically. Twice the money, one round's worth of information, and `lint`
said nothing.
The decision is a sixth advisory sweep in internal/handoff, warning on the
rerun target and naming the declarer, matched with the runtime's own
placeholder pattern so it holds after fragment splicing. Measured over three
corpora first (docs/measurements/0028-feedback-quote-corpus.md): 11 declarers,
3 hits, all 3 real, 0 noise — and the corpus contains the specimen's own
repair three minutes later, on which the predicate is correctly silent. No
shipped graph fires.
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
* feat(handoff): warn when a feedback loop's repair never quotes the feedback
The sixth advisory sweep, ADR 0028. For every node D declaring
`feedback: { rerun: R }`: if no node in the loop body other than D itself
quotes `{{ feedback.D }}` in its prompt, warn on R, naming D. R is where the
missing line goes, so that is where the line lands.
A middle body node counts — `build → refine → check` quoted at `refine` really
does repair, just not at the loop's first node — and the declarer's own quote
does not: it is the judge, so its re-run repairs nothing, it only re-judges
unchanged artifacts while being reminded of its own findings. Only prompts are
read: a payload on a verify command line is LintVerifyInlining's finding and
one in a cwd is a path.
Matched with the runtime's own placeholderPattern rather than a formatted
string, so what the sweep counts as a quote and what Interpolate splices cannot
drift, and a namespaced id survives fragment splicing — the specimen's real
token was `{{ feedback.qa-a/check }}`. Both shapes are tested, the spliced one
through the real loader over a real fragment.
Wired into the one warnAdvisories helper `lint` and `run --dry-run` share.
Advisory, never a load error: the planner cannot author a `feedback:` at all,
so only a person can write what this condemns.
No runtime behaviour changed.
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
* docs(adr): the planner writes feedback arcs too, so auto mode refuses the blind one
ADR 0028 said twice that `coordinator.validatePlannedNodes` refuses a
planner-authored `feedback:` outright, and concluded from it that only a person
can write what the sweep condemns and that there was "no auto-mode counterpart
to add". Both are false. The coordinator CONSTRAINS a planned arc (max, reach)
and the planner prompt asks for this very pairing in prose — "declare a feedback
arc on the reviewing node … and have the implementing node's prompt read
{{ feedback.<reviewing-node-id> }}" — with only the first half machine-checked.
Measured: 3 of the 11 declarers in the run corpus are planner-authored.
So the escalation the ADR declared unnecessary is taken here, on
validatePlannedFeedbackReach's precedent: validatePlannedFeedbackQuoting refuses
a planned arc whose loop body never quotes the payload, reading the same
predicate through the new handoff.FeedbackQuoteFindings rather than re-deciding
it. Its warrant is weaker than the reach rule's (no planner failure of this shape
is measured) and its price of being wrong is lower than anywhere else in that
validator: one re-plan, and a placeholder that is harmless when ignored because
the namespace resolves to empty on the first pass. The refusal names both ends
and the token, and tells the planner not to make the work conditional on a
feedback section appearing — that "fix" is the specimen written back.
Also handled, all recorded in the ADR:
- the sweep is on `lint`/`run --dry-run` only, never a plain `run`, so the cost
narrative no longer reads as if it stands between the author and the bill —
and DESIGN.md's repo-layout line, which claimed `run` prints the sweeps
against internal/graph/fragment.go's own statement that it does not, is fixed;
- "observe it at run time" is separated from "refuse it at run time" and kept as
a zero-false-positive successor, out of scope rather than rejected;
- "3 hits, 0 noise" carries its caveat on the number: three lanes of ONE graph,
one distinct control;
- a loop FRAGMENT cannot be linted at all (measured: lint on repair-round.yaml
exits 1 on an unbound substitution), so ownership of the warning is inverted
and the corpus's fragment exclusion is blindness, not rigour;
- the topological false negative (ancestry is not data dependency) is named;
- so is the fact that none of the six advisories can be silenced;
- "a citing graph cannot edit a spliced prompt" is narrowed to loop splices,
since substitutions:/with: exist precisely to let it.
TestLintFeedbackQuoting_ShippedGraphsAreClean walks graphs/*.yaml so the "no
shipped graph fires" claim fails in the suite rather than decaying quietly; it
covers the repair-round fragment too, spliced, via adr-driven-dev.yaml.
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
* fix(handoff): a filtered feedback token is not a quote, and the corpus keeps its denominators apart
Three findings from review, none of them changing what the sweep decides on
any graph that can be loaded today:
- bodyQuotesFeedback read groups[1]/groups[2] and ignored the filter group,
so `{{ feedback.D | inline }}` counted as a satisfied quote. The runtime
does the opposite — resolveLocked errors on a filtered feedback token and
graph.Validate refuses it at load — so that was the one seam where the
sweep and the runtime could disagree, calling WIRED a loop the runtime
refuses to run. Guarded, with the load-time invariant it leans on named
and pinned: the new test asserts that graph.Parse still refuses the token,
which is what makes the guard unreachable rather than load-bearing.
- The measurement's headline folded three corpora into one corpus's
denominator ("3 times in 11") while its own table totalled 15 declarers.
11 is the run-snapshot corpus alone; the split is now stated per corpus,
and the three later sentences that meant "run corpus" say so.
- dist/ is an untracked goreleaser artifact directory one `git add -A` away
from being committed. Ignored.
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
* fix(coordinator): the two graph-level refusal families share one prompt budget, so bound it
Review found that the ordering comment in validatePlannedNodes reasons about
"one ~600-char graph-level sentence vs. 83-172-char per-node ones" — arithmetic
written when there was one such family. There are now two, both fire on one arc
independently (ADR 0028 §Failure modes), and both scale per declarer, so they
crowd EACH OTHER rather than being crowded by the per-node refusals the
ordering outranks. Measured on the new twoBrokenLanesSpec fixture: 677 + 677
for reach, 641 for quoting, 1998 bytes against a 2000-byte maxIssuesInPrompt —
this ADR's own three-lane specimen would have blown it alone, and one per-node
slip beside two lanes does.
Past the cut the damage was invisible from both ends: fence.Truncate is
head-only, so the last kept refusal ended mid-sentence and every later one
vanished without trace. The single repair a refused plan buys was then spent on
a prompt that never stated part of the fault.
Three bounds, in the order they matter:
- validatePlannedFeedbackQuoting emits ONE refusal for every blind arc in the
graph, naming each declarer/rerun pair and each token to paste, instead of
repeating a ~500-byte diagnosis per declarer. Four arcs: 753 bytes, from
2272. The same compaction validatePlannedFeedbackReach makes across
producers, one level up.
- maxIssuesInPrompt is sized from the two families rather than picked: 3000
holds three declarers faulty both ways with room for per-node refusals.
- issuesForPrompt replaces the head-only cut — whole refusals from the front,
and the dropped count stated in the prompt. Half a refusal names a node and
stops before the correction, which is an instruction to guess.
Tests for what the review exposed, all of which fail on the prior code:
the two-arcs-both-rules case survives a crowded repair prompt (it loses the
whole quoting family at the old budget); the "reach leads quoting leads
per-node" order is pinned on the refusal list itself, not through a rendered
prompt; the compacted refusal is held to naming both ends of both pairs and to
reading as English in the plural; and issuesForPrompt is pinned on whole-refusal
packing, on a disclosure count that agrees with what was dropped, and on the
one unavoidable cut (a single refusal larger than the whole budget).
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
* docs(changelog): a release section carries one Fixed block, not two
This branch opened a `### Fixed` under `[Unreleased]` without noticing the
one #193 already put there, so the section carried two same-level headings
between `## [Unreleased]` and `## [v0.9.0]`. scripts/release-notes.sh
excerpts a version's section verbatim into the published body, so the next
release page would have shown "Fixed" twice with the entries split across
them — which is exactly the release-notes quality #193 and #194 were fixing.
The three entries are one list; the second heading is gone.
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
* fix(coordinator): one rendering, one measured size, with an address
Three files described the same rendering — two declarers each mis-aimed AND
blind — at three different sizes, and one of them contradicted the others
outright. repair.go said the pair rendered ~2033 bytes and that a family
"was cut"; repair_budget_test.go said the two families render 1998 and FITTED
the old 2000 budget with two bytes to spare (which is why its fixture pads
with five per-node refusals); coordinator.go said 2488. The 2033/2780 pair
were rounded per-family estimates (700x2 + 640), and they were the only
argument on record for maxIssuesInPrompt being 3000.
Re-measured on twoBrokenLanesSpec and its three- and four-lane extensions,
with each figure labelled by which side of the compaction it describes:
reach, per declarer 677
quoting, per arc, uncompacted 592
quoting, compacted 641 (2 arcs) / 701 (3) / 761 (4)
both families joined, 2 lanes 2541 uncompacted, 1997 compacted
both families joined, 3 lanes 2735 compacted
So the two states were never comparable: uncompacted the pair overran the old
2000 budget and a family WAS cut, compacted it fits by three bytes and the
shortest per-node refusal beside it takes it over. Both readings were right
about different renderings and neither said which.
TestGraphLevelRefusalFamiliesRenderTheirMeasuredSize pins every reachable
figure, so a reworded refusal fails a test rather than leaving four comments,
an ADR and a CHANGELOG entry quietly false — which is what happened here. It
also pins the claim the budget rests on: three declarers faulty both ways fit
inside maxIssuesInPrompt.
ADR 0028 Failure modes carried the arithmetic slip that exposed this: it
summed its own 677 + 677 + 641 as 1998 where the join is 1997.
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
* fix(coordinator): the compacted refusal reads in the plural to the end of the sentence
plannedFeedbackQuoteRefusal pluralises where it names the list — the verbs, the
possessives, the target of the paste — but the PLACEMENT clause kept a singular
it never pluralised: "at the end of each of those prompts and after that node's
verdict contract". This is a prompt the one repair call a refused plan buys has
to act on, and that clause is the actionable half; "that node's" beside a plural
"each of those prompts" names a node the planner has to guess at.
%[8]s is plural(n, "that node's", "those nodes'"). Singular output is unchanged
byte for byte. TestPlan_TwoBlindArcsCostOneRefusal already read the plural
rendering for exactly this class of leftover and did not cover this clause; it
now checks every clause that refers back to the list, and that the plural one
names whose verdict contract to paste after.
"those nodes'" is one byte longer than "that node's", which moves every measured
figure for n >= 2 by one — the pin test added in the previous commit caught it
and named all six. Comments, ADR and CHANGELOG follow it: quoting 642/702/762
for two/three/four arcs, and 1998 / 2736 / 3474 joined with the reach family.
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
* docs(handoff): six sweeps, a comment that matches the path it describes, and the sibling return style
Three small corrections, none of them behavioural:
- DESIGN.md's fragment-disclosure paragraph still said "the four *handoff*
sweeps stay lint-only". It has been drifting since the fifth landed; this
branch updated the repo-layout line and lint.go's "five" to six and left this
one. Six: LintPlaceholders, LintSessions, LintVerdicts, LintToolGrants,
LintVerifyInlining, LintFeedbackQuoting.
- TestLintFeedbackQuoting_QuietWhereThereIsNoLoop claimed its second fixture
exercised "an arc whose target is not an ancestor". It does not: a
&graph.Graph{Nodes: ...} literal never runs fromRaw, so byID is nil and
FeedbackBody returns at its FIRST lookup, not at the ancestor check. The
guard under test is the same either way (delete it and the test fails), but
the comment named a branch the case cannot reach — the same struct-construction
trap already corrected once in this branch. It now says which return it takes
and why the ancestor branch is unreachable from any loading path.
- LintFeedbackQuoting normalised its empty case by hand (make+len check+nil).
Its siblings LintVerifyInlining and LintToolGrants declare `var warnings
[]Warning` and get the same nil for free; matching them drops the allocation
and the branch.
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
* chore: this branch is about feedback quoting, not about goreleaser's output
The `/dist/` ignore rule rode in on 912973a with a commit about lint sweeps and
corpus denominators. It is a fine rule and a wrong place: nothing about
feedback quoting touches release artifacts, and mixing them makes the branch's
history unreadable to anyone bisecting either subject.
Moved verbatim to the local branch chore/gitignore-dist, cut from main, so
nothing is lost — it wants its own PR.
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
* docs(coordinator): the pinned sizes carry their own repair instruction
TestGraphLevelRefusalFamiliesRenderTheirMeasuredSize is exact on purpose
— a reworded refusal is EXPECTED to fail it — but the instruction for
what to do about that failure lived only in the comment above the test.
The author who sees it sees a red line in a terminal, not that comment,
so the failure now names the four places the numbers have to be carried
to and the table that has to be updated.
And say, where len(Issues) is defined, that it counts refusals rather
than faults: the reach family emits one per declarer while the quoting
family compacts every blind arc into one, so the two now count
differently in the same number and anything reporting it to a human has
to keep saying "refusal".
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
* fix(coordinator): the truncation that omitted refusals without saying so
Review finding, and it is the failure issuesForPrompt exists to prevent,
reached through its own edge case.
When the first refusal fits inside the budget alone but not alongside the
omission note, the kept-count loop ends at zero and the old code truncated the
JOINED list. The later refusals then vanished AND their count vanished with
them: the planner was told less than it had been refused for, and nothing said
so — a re-plan answering a subset of the reasons it was rejected for.
The note is now reserved first and the first refusal bounded to what remains,
so what the planner loses is the tail of one refusal, which the truncation
marker announces, plus a count of the rest, which the note announces. Both
losses are disclosed; neither is silent.
TestIssuesForPrompt_FirstRefusalTooLongStillDisclosesTheRest pins the boundary.
Mutation-checked: restoring the old branch turns it red.
Two smaller review findings:
The measurement utility decided planner provenance with `run in src`, a
SUBSTRING — any path merely containing the run id would have counted as
planner-authored. A measurement tool looser than what it measures is not
evidence. Re-measured with an exact path comparison and the split is unchanged
(rows=7 planner=3 hand=4, declarers 3 and 8), which is the only reason the
number is still quoted.
Output fences in ADR 0028 and the measurement now carry a language tag.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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 874c9c2 commit a3493ed
13 files changed
Lines changed: 2398 additions & 24 deletions
File tree
- cmd/oh-my-graph
- docs
- adr
- measurements
- internal
- coordinator
- handoff
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
13 | 81 | | |
14 | 82 | | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
15 | 113 | | |
16 | 114 | | |
17 | 115 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
411 | 411 | | |
412 | 412 | | |
413 | 413 | | |
414 | | - | |
| 414 | + | |
415 | 415 | | |
416 | 416 | | |
417 | 417 | | |
| |||
456 | 456 | | |
457 | 457 | | |
458 | 458 | | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
459 | 479 | | |
460 | 480 | | |
461 | 481 | | |
| |||
2345 | 2365 | | |
2346 | 2366 | | |
2347 | 2367 | | |
2348 | | - | |
| 2368 | + | |
2349 | 2369 | | |
2350 | 2370 | | |
2351 | 2371 | | |
| |||
2588 | 2608 | | |
2589 | 2609 | | |
2590 | 2610 | | |
2591 | | - | |
| 2611 | + | |
2592 | 2612 | | |
2593 | 2613 | | |
2594 | 2614 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
58 | | - | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
59 | 62 | | |
60 | 63 | | |
61 | 64 | | |
| |||
101 | 104 | | |
102 | 105 | | |
103 | 106 | | |
104 | | - | |
| 107 | + | |
105 | 108 | | |
106 | 109 | | |
107 | | - | |
108 | | - | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
109 | 113 | | |
110 | 114 | | |
111 | 115 | | |
112 | 116 | | |
113 | 117 | | |
114 | 118 | | |
| 119 | + | |
115 | 120 | | |
116 | 121 | | |
117 | 122 | | |
| |||
0 commit comments