From 76d23b165a02f6e7f4e370da61e7a838b0000357 Mon Sep 17 00:00:00 2001 From: Dmitrii Vasilev Date: Mon, 3 Aug 2026 00:43:16 +0700 Subject: [PATCH 1/2] queen(gHashTag/trios#1134): Write docs/seven-links.md describing the seven links of the review chain, at least seven hundred characters --- trios/docs/seven-links.md | 128 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 trios/docs/seven-links.md diff --git a/trios/docs/seven-links.md b/trios/docs/seven-links.md new file mode 100644 index 0000000000..74accb2e0e --- /dev/null +++ b/trios/docs/seven-links.md @@ -0,0 +1,128 @@ +# Seven Links — The Review Chain from Specification to Verdict + +Issue: gHashTag/trios#1134 · Parent: #1090 + +## What this document is + +A description of the seven links in the review chain: the path a criterion +travels from the moment the Queen writes it into a task specification to the +moment it gates acceptance. Each link is a place where the chain can break +silently — the criterion does not fail, it stays `.unchecked`, which reads as +"nobody looked" when the chain itself was the thing that broke. + +The companion document +[`night-of-the-review-chain.md`](night-of-the-review-chain.md) records four of +these links as they failed in a single incident (#1130). This document steps +back and names all seven, including the specification that defines the criteria, +the file contents that let the reviewer judge unchanged code, and the verdict +gate that decides whether acceptance may proceed. + +## The seven links + +### 1. The specification + +The chain begins with the acceptance criteria written into the task spec +(`QueenTaskSpec.render`, `rings/SR-00/QueenTaskSpec.swift`). Each criterion is a +single sentence: "Существует файл `docs/seven-links.md`." The spec is posted as +the first message in the worker's chat and pinned in the header strip +(described in [`queen-spec-header.md`](queen-spec-header.md)) so the reviewer +never has to scroll to find what done means. + +If the criteria are missing, ambiguous, or swapped for a worker's own +self-assessment, the reviewer has no contract to judge against. The spec header +and `QueenAcceptancePolicy.verdicts(criteria:recorded:)` +(`rings/SR-00/QueenCriterionVerdict.swift`) both use the same source — +`DelegatedTask.acceptanceCriteria` — so the criteria displayed in the header and +the criteria the reviewer receives in the brief can never disagree. + +### 2. The brief + +The brief is the evidence package assembled for the reviewer agent: the +criteria, the diff, and the full contents of touched files +(`QueenReviewVerdictRequest.brief`, +`rings/SR-00/QueenReviewVerdictRequest.swift`). An embedded adversary prompt +marker (`"adversary-review"`) proves that the brief that reached the reviewer is +the brief this chain built — if the brief were replaced with a worker's prompt, +which carries no marker, the check (`isAdversarialBrief`) fails and the +verdicts are discarded. + +### 3. The root of paths + +`fileContentsForReview` (`rings/SR-02/ChatViewModel.swift`) resolves every file +path against `ProjectPaths.root` — the project directory — not the git toplevel. +Resolving from the wrong root left every criterion-named file silently missing. +The reviewer received an empty set, read the criteria against nothing, and +returned "could not check." A file that does not exist on the working tree is +now included with an explicit `(file not found)` marker so the gap is visible. + +### 4. The file contents + +After the diff, the brief carries the full contents of every file in the task's +`ownedPaths`, read from the working tree after the commit +(`fileContentsForReview`, `rings/SR-02/ChatViewModel.swift`). This is the "what +it looks like now" evidence — a criterion about code the change did not touch +but that lives in a touched file can receive a verdict instead of defaulting to +`.unchecked`. The reviewer scope is described in +[`reviewer-scope.md`](reviewer-scope.md). + +### 5. The excerpt volume + +A large file sent in full crowds the brief and pushes the criteria below a wall +of code. `regionExtractedContent` narrows each file to regions around names the +criteria mention. When no criteria names are found, the excerpt says so +explicitly rather than hiding the gap behind plausible-looking opening lines. +Files exceeding 500 lines (`maxFileLinesInBrief`) are truncated with a visible +marker: `… (truncated: 500 of 1234 lines)`. + +### 6. The response format + +The reviewer formats its response in markdown — bold numbers, bullets, +checkboxes — and the parser (`QueenReviewVerdictRequest.parse`) must see through +the decoration to the number underneath. A response the parser cannot match +leaves the criterion absent, which reads as `.unchecked`. An empty response +retries once; if still empty, the criteria are recorded as "asked but +unanswered" (#1117) so the distinction from "never checked" stays alive. + +### 7. The verdict gate + +The final link is the verdict gate +(`QueenAcceptancePolicy.acceptanceBlockReason`, +`rings/SR-00/QueenCriterionVerdict.swift`). It merges mechanical verdicts +(criteria that name a file path are settled by what the branch carries) with +recorded verdicts from the reviewer, filling every gap with `.unchecked`. If any +criterion is `.unmet`, acceptance is blocked — unmet criteria are listed first. +If any criterion is `.unchecked` and none are `.unmet`, acceptance is still +blocked: an unchecked criterion is not a pass. Only when the block reason +returns `nil` may the Queen transition the task from `.awaitingReview` to +`.accepted`. + +## What the chain has in common + +Every link breaks the same way: silently. The criterion does not fail — it stays +`.unchecked`, which reads as "nobody looked" when the chain was the thing that +broke. The fixes all do the same thing: make the gap visible. The marker proves +the brief was sent. The project root proves the files were found. The full file +contents prove the reviewer saw the context. The excerpt notice proves the +excerpt was honest. The parser test proves the response was read. The verdict +gate proves nothing was left unanswered. Each link is now loud where it was +quiet. + +## Code references + +| Link | Symbol | File | +|------|--------|------| +| Specification | `QueenTaskSpec.render` | `rings/SR-00/QueenTaskSpec.swift` | +| Specification | `DelegatedTask.acceptanceCriteria` | `rings/SR-00/QueenDelegation.swift` | +| Brief | `QueenReviewVerdictRequest.brief` | `rings/SR-00/QueenReviewVerdictRequest.swift` | +| Brief | `QueenReviewVerdictRequest.adversaryPromptMarker` | `rings/SR-00/QueenReviewVerdictRequest.swift` | +| Root of paths | `ChatViewModel.fileContentsForReview` | `rings/SR-02/ChatViewModel.swift` | +| Root of paths | `ProjectPaths.root` | — | +| File contents | `ChatViewModel.diffForReview` | `rings/SR-02/ChatViewModel.swift` | +| File contents | `ChatViewModel.fileContentsForReview` | `rings/SR-02/ChatViewModel.swift` | +| Excerpt volume | `ChatViewModel.regionExtractedContent` | `rings/SR-02/ChatViewModel.swift` | +| Excerpt volume | `QueenReviewVerdictRequest.maxFileLinesInBrief` | `rings/SR-00/QueenReviewVerdictRequest.swift` | +| Response format | `QueenReviewVerdictRequest.parse` | `rings/SR-00/QueenReviewVerdictRequest.swift` | +| Response format | `runVerdictParserHandlesMarkdownNumbers` | `tests/swift/ChatSSEEndToEndTest.swift` | +| Verdict gate | `QueenAcceptancePolicy.mechanicalVerdicts` | `rings/SR-00/QueenCriterionVerdict.swift` | +| Verdict gate | `QueenAcceptancePolicy.verdicts` | `rings/SR-00/QueenCriterionVerdict.swift` | +| Verdict gate | `QueenAcceptancePolicy.acceptanceBlockReason` | `rings/SR-00/QueenCriterionVerdict.swift` | From dd052239153ffcd7cfe1738e1234e0c0ff81e172 Mon Sep 17 00:00:00 2001 From: Dmitrii Vasilev Date: Mon, 3 Aug 2026 01:18:04 +0700 Subject: [PATCH 2/2] queen(gHashTag/trios#1134): Write docs/seven-links.md describing the seven links of the review chain, at least seven hundred characters --- trios/docs/seven-links.md | 265 ++++++++++++++++++++++---------------- 1 file changed, 157 insertions(+), 108 deletions(-) diff --git a/trios/docs/seven-links.md b/trios/docs/seven-links.md index 74accb2e0e..e7f06ef100 100644 --- a/trios/docs/seven-links.md +++ b/trios/docs/seven-links.md @@ -1,128 +1,177 @@ -# Seven Links — The Review Chain from Specification to Verdict +# Seven Links — The Review Chain and Where It Broke Issue: gHashTag/trios#1134 · Parent: #1090 ## What this document is -A description of the seven links in the review chain: the path a criterion -travels from the moment the Queen writes it into a task specification to the -moment it gates acceptance. Each link is a place where the chain can break -silently — the criterion does not fail, it stays `.unchecked`, which reads as -"nobody looked" when the chain itself was the thing that broke. +The review chain runs from the Queen's specification to the reviewer's +verdict to the gate's decision. Seven links in that chain broke during the +build of the delegation system — each one silently, each one reading as +"the work is not good enough" when the chain itself was the thing that +failed. This document names all seven, describes how each broke, how each +was repaired, and names the eighth that was found at the end of the road. The companion document -[`night-of-the-review-chain.md`](night-of-the-review-chain.md) records four of -these links as they failed in a single incident (#1130). This document steps -back and names all seven, including the specification that defines the criteria, -the file contents that let the reviewer judge unchanged code, and the verdict -gate that decides whether acceptance may proceed. +[`night-of-the-review-chain.md`](night-of-the-review-chain.md) covers the +first four in greater depth; this document stands on its own as the full +account. + +## The pattern + +Every link breaks the same way: the question never reaches the judge, or +the answer never reaches the gate. The criterion does not fail — it stays +`.unchecked`, which reads as "nobody looked" when the chain was the thing +that broke. A confident wrong answer is worse than silence, because it +looks exactly like the gate working. ## The seven links -### 1. The specification - -The chain begins with the acceptance criteria written into the task spec -(`QueenTaskSpec.render`, `rings/SR-00/QueenTaskSpec.swift`). Each criterion is a -single sentence: "Существует файл `docs/seven-links.md`." The spec is posted as -the first message in the worker's chat and pinned in the header strip -(described in [`queen-spec-header.md`](queen-spec-header.md)) so the reviewer -never has to scroll to find what done means. - -If the criteria are missing, ambiguous, or swapped for a worker's own -self-assessment, the reviewer has no contract to judge against. The spec header -and `QueenAcceptancePolicy.verdicts(criteria:recorded:)` -(`rings/SR-00/QueenCriterionVerdict.swift`) both use the same source — -`DelegatedTask.acceptanceCriteria` — so the criteria displayed in the header and -the criteria the reviewer receives in the brief can never disagree. - -### 2. The brief - -The brief is the evidence package assembled for the reviewer agent: the -criteria, the diff, and the full contents of touched files -(`QueenReviewVerdictRequest.brief`, -`rings/SR-00/QueenReviewVerdictRequest.swift`). An embedded adversary prompt -marker (`"adversary-review"`) proves that the brief that reached the reviewer is -the brief this chain built — if the brief were replaced with a worker's prompt, -which carries no marker, the check (`isAdversarialBrief`) fails and the +### 1. The brief — files the criteria name never arrive + +The brief is the evidence package sent to the reviewer: criteria, diff, +and the full contents of touched files. When the brief did not carry the +files a criterion names, a task whose owned path was a test file but whose +contract was about the application could not be judged (#1119). The +reviewer had nothing to read, so the verdict read "could not check" — +honest uncertainty masking a missing payload. + +**Repaired** by `fileContentsForReview` reading every path in the task's +`ownedPaths` from the working tree, not just the files the diff touched. +The `adversaryPromptMarker` (`"adversary-review"`, +`rings/SR-00/QueenReviewVerdictRequest.swift`) proves the brief that +reached the reviewer is the brief this chain built — a worker's prompt +carries no marker, so a swapped brief fails `isAdversarialBrief` and the verdicts are discarded. -### 3. The root of paths - -`fileContentsForReview` (`rings/SR-02/ChatViewModel.swift`) resolves every file -path against `ProjectPaths.root` — the project directory — not the git toplevel. -Resolving from the wrong root left every criterion-named file silently missing. -The reviewer received an empty set, read the criteria against nothing, and -returned "could not check." A file that does not exist on the working tree is -now included with an explicit `(file not found)` marker so the gap is visible. - -### 4. The file contents - -After the diff, the brief carries the full contents of every file in the task's -`ownedPaths`, read from the working tree after the commit -(`fileContentsForReview`, `rings/SR-02/ChatViewModel.swift`). This is the "what -it looks like now" evidence — a criterion about code the change did not touch -but that lives in a touched file can receive a verdict instead of defaulting to -`.unchecked`. The reviewer scope is described in -[`reviewer-scope.md`](reviewer-scope.md). - -### 5. The excerpt volume - -A large file sent in full crowds the brief and pushes the criteria below a wall -of code. `regionExtractedContent` narrows each file to regions around names the -criteria mention. When no criteria names are found, the excerpt says so -explicitly rather than hiding the gap behind plausible-looking opening lines. -Files exceeding 500 lines (`maxFileLinesInBrief`) are truncated with a visible -marker: `… (truncated: 500 of 1234 lines)`. - -### 6. The response format - -The reviewer formats its response in markdown — bold numbers, bullets, -checkboxes — and the parser (`QueenReviewVerdictRequest.parse`) must see through -the decoration to the number underneath. A response the parser cannot match -leaves the criterion absent, which reads as `.unchecked`. An empty response -retries once; if still empty, the criteria are recorded as "asked but -unanswered" (#1117) so the distinction from "never checked" stays alive. - -### 7. The verdict gate - -The final link is the verdict gate -(`QueenAcceptancePolicy.acceptanceBlockReason`, -`rings/SR-00/QueenCriterionVerdict.swift`). It merges mechanical verdicts -(criteria that name a file path are settled by what the branch carries) with -recorded verdicts from the reviewer, filling every gap with `.unchecked`. If any -criterion is `.unmet`, acceptance is blocked — unmet criteria are listed first. -If any criterion is `.unchecked` and none are `.unmet`, acceptance is still -blocked: an unchecked criterion is not a pass. Only when the block reason -returns `nil` may the Queen transition the task from `.awaitingReview` to -`.accepted`. +### 2. The root of paths — resolution against the wrong directory + +`fileContentsForReview` resolved every file path against the git root — +the repository toplevel — rather than `ProjectPaths.root`, the project +directory. For this project the git root sits one level above the project +directory, so every criterion-named file was silently missing. The miss +was swallowed by a `fileExists` guard that dropped the file instead of +reporting it as absent (#1121). + +**Repaired** by resolving from `ProjectPaths.root`. A file that does not +exist on the working tree is now included with an explicit +`(file not found)` marker rather than dropped, so the gap is visible to +the reviewer instead of swallowed. + +### 3. The response format — the parser cannot read what the reviewer wrote + +The reviewer formats its response in markdown — bold numbers (`**1.`), +bullets (`- 1.`), checkboxes (`[x] 1.`) — and the parser had to see +through the decoration to the number underneath. A response the parser +could not match left the criterion absent, which read as `.unchecked` +(#1122). The parser's own comment listed bare and checkbox variants but +never markdown. + +**Repaired** by `runVerdictParserHandlesMarkdownNumbers` +(`tests/swift/ChatSSEEndToEndTest.swift`), which feeds a real reviewer +response and asserts all verdicts parse. Each decoration variant is +recognised; a decorated line with no verdict keyword stays absent — +lenience to decoration must not become a willingness to guess. + +### 4. The excerpt that follows the diff, not the criteria + +`ChatViewModel.swift` is over five thousand lines, and the brief carried +its first 500. A criterion about behaviour below the declarations read +"could not check" because the reviewer was shown lines 1–500 and the +criterion was about line 800 (#1123). + +**Repaired** by `regionExtractedContent` narrowing each file to regions +around names the criteria mention. When the region selection finds +nothing, the excerpt says so rather than hiding the gap behind the file's +opening lines. + +### 5. The excerpt on an empty diff — re-review shows the wrong lines + +The region extraction in link 4 worked when there was a diff, but fell +back to the first 500 lines when there was not. A re-review of finished +work is always an empty diff, which is exactly the case the fallback +ruins: the criteria are about the implementation, and the first 500 lines +are the declarations (#1124). + +**Repaired** by the same `regionExtractedContent` path covering the +no-diff case. When no criteria names are found in a file, the excerpt +prints an explicit notice: + +``` +(none of the criteria names found in this file — looked for: ChatViewModel, fileContentsForReview) +``` + +The truncation marker (`… (truncated: 500 of 1234 lines)`) was added in +the same pass, so the reviewer always knows whether a file ended or was +cut. + +### 6. The phantom deletion — a stale baseline turns a file into a removal + +On an empty branch the diff was taken against a baseline captured after +an earlier run's work, so a file sitting on disk arrived at the reviewer +as `deleted file mode 100644` — 119 lines removed. The reviewer answered +"unmet" twice, and it was right about the document it was given. A +confident wrong "no" is worse than silence: it looks exactly like the gate +working (#1132). + +**Repaired** by an empty branch having nothing to compare and saying so, +instead of presenting a file that exists as removed. + +### 7. The premature verdict — the gate decides before the evidence arrives + +The log order was flat: + +``` +15:15:58 Review command applied +15:16:04 Reviewer returned 2 verdict(s) for 2 criterion(s) +``` + +Acceptance decided six seconds before the evidence existed and never came +back to it. Six links damaged the question; this one does not wait for +the answer. Everything that read as "the gate refuses" was, at the last +step, a gate that had already refused before anyone spoke (#1133). + +**Repaired** by acceptance waiting for its evidence: verdicts are +recorded, then the decision follows, and the log order reflects the +causal order. + +## The eighth link + +The proposal carried 2,010 files. The base branch pointed at the remote +default so the Queen would stop merging into the branch a person was +working on, but a bee's branch was cut from that working branch — a night +of commits ahead of `dev`. The diff against `dev` was the night, not the +bee. Both extremes were wrong: the current branch merges into somebody's +work; the default branch proposes everything. The answer is the point the +bee's branch was cut from (#1135). + +Found by finally reaching the end of the road — the first task to go the +whole way from delegate to merge since #1102. ## What the chain has in common -Every link breaks the same way: silently. The criterion does not fail — it stays -`.unchecked`, which reads as "nobody looked" when the chain was the thing that -broke. The fixes all do the same thing: make the gap visible. The marker proves -the brief was sent. The project root proves the files were found. The full file -contents prove the reviewer saw the context. The excerpt notice proves the -excerpt was honest. The parser test proves the response was read. The verdict -gate proves nothing was left unanswered. Each link is now loud where it was -quiet. +Every link was invisible until the step before it worked. Every one broke +silently: the criterion stayed `.unchecked` or received a confident wrong +answer, and the failure read as honest judgement rather than a broken +pipe. The fixes all do the same thing — make the gap visible. The marker +proves the brief was sent. The project root proves the files were found. +The parser test proves the response was read. The region extraction proves +the excerpt was honest. The empty-diff notice proves the right lines were +shown. The empty-branch comparison proves nothing was invented. The +causal ordering proves the gate waited for its evidence. Each link is now +loud where it was quiet. ## Code references | Link | Symbol | File | |------|--------|------| -| Specification | `QueenTaskSpec.render` | `rings/SR-00/QueenTaskSpec.swift` | -| Specification | `DelegatedTask.acceptanceCriteria` | `rings/SR-00/QueenDelegation.swift` | -| Brief | `QueenReviewVerdictRequest.brief` | `rings/SR-00/QueenReviewVerdictRequest.swift` | -| Brief | `QueenReviewVerdictRequest.adversaryPromptMarker` | `rings/SR-00/QueenReviewVerdictRequest.swift` | -| Root of paths | `ChatViewModel.fileContentsForReview` | `rings/SR-02/ChatViewModel.swift` | -| Root of paths | `ProjectPaths.root` | — | -| File contents | `ChatViewModel.diffForReview` | `rings/SR-02/ChatViewModel.swift` | -| File contents | `ChatViewModel.fileContentsForReview` | `rings/SR-02/ChatViewModel.swift` | -| Excerpt volume | `ChatViewModel.regionExtractedContent` | `rings/SR-02/ChatViewModel.swift` | -| Excerpt volume | `QueenReviewVerdictRequest.maxFileLinesInBrief` | `rings/SR-00/QueenReviewVerdictRequest.swift` | -| Response format | `QueenReviewVerdictRequest.parse` | `rings/SR-00/QueenReviewVerdictRequest.swift` | -| Response format | `runVerdictParserHandlesMarkdownNumbers` | `tests/swift/ChatSSEEndToEndTest.swift` | -| Verdict gate | `QueenAcceptancePolicy.mechanicalVerdicts` | `rings/SR-00/QueenCriterionVerdict.swift` | -| Verdict gate | `QueenAcceptancePolicy.verdicts` | `rings/SR-00/QueenCriterionVerdict.swift` | -| Verdict gate | `QueenAcceptancePolicy.acceptanceBlockReason` | `rings/SR-00/QueenCriterionVerdict.swift` | +| 1. Brief | `QueenReviewVerdictRequest.adversaryPromptMarker` | `rings/SR-00/QueenReviewVerdictRequest.swift` | +| 1. Brief | `QueenReviewVerdictRequest.isAdversarialBrief` | `rings/SR-00/QueenReviewVerdictRequest.swift` | +| 1. Brief | `ChatViewModel.fileContentsForReview` | `rings/SR-02/ChatViewModel.swift` | +| 2. Root of paths | `ChatViewModel.fileContentsForReview` | `rings/SR-02/ChatViewModel.swift` | +| 2. Root of paths | `ProjectPaths.root` | — | +| 3. Response format | `QueenReviewVerdictRequest.parse` | `rings/SR-00/QueenReviewVerdictRequest.swift` | +| 3. Response format | `runVerdictParserHandlesMarkdownNumbers` | `tests/swift/ChatSSEEndToEndTest.swift` | +| 4. Excerpt (diff) | `ChatViewModel.regionExtractedContent` | `rings/SR-02/ChatViewModel.swift` | +| 5. Excerpt (empty diff) | `ChatViewModel.regionExtractedContent` | `rings/SR-02/ChatViewModel.swift` | +| 5. Excerpt (empty diff) | `QueenReviewVerdictRequest.maxFileLinesInBrief` | `rings/SR-00/QueenReviewVerdictRequest.swift` | +| 7. Premature verdict | Review pipeline ordering | `rings/SR-02/ChatViewModel.swift` |