Repository navigation
feat(code-review): pick the focus set by gate level - #512
Merged
Merged
Conversation
Cut sentences, tree labels and Edge Cases cells that repeat text stated elsewhere in the command. No behaviour change. The cuts sit outside the learning arms, so both builds shrink by the same amount.
/code-review now narrows its review focus list by GATE_LEVEL in a new Step 1.2b, once per worktree and before the diff files are written. - full keeps today's sets and reads nothing. - standard drops typescript, and drops dependencies on a code diff unless a listed dependency manifest changed. - lean keeps security, testing and reliability on a code diff, plus compliance on its own trigger. - Security runs at every level and in every class. A new reference, code-review-levels.md, holds the class-by-level table. The command reads it only at standard or lean, once per run, through the sanctioned resolution line. If the read fails, or a row is missing or malformed, the full sets apply for every worktree of the run. The guards accept the sanctioned resolution line as a read root, allow exactly one directory read in the command, and a new test file checks the reference, the block, the unchanged full-level text and the no-second-cycle invariant with known-bad probes. The install goldens follow in the next commit.
The three install-snapshot goldens gain one line each for the new level reference under references/commands/. Regenerated with the golden update script; nothing else changed.
/code-review passes GATE_LEVEL and the planned focus list to the Synthesize agent. The review summary gains a "Gate level" line that lists the focuses and marks each focus with no report. A planned security focus with no report now gives CHANGES REQUESTED, so a lost security review cannot read as a clean merge. The summary reference gives back more bytes than it takes, and the new test checks the spawn inputs, the template line, the merge row and the file set the command writes.
Lower the code-review ceilings (learning-on and learning-off) and the Synthesize mode reference ceiling to the sizes measured after a fresh build. No ceiling rises. Add a ceiling row for the new level reference. /code-review reads it once per run on the default path, so it gets its own measured cap and a probe that reports a file one byte over.
The gates help no longer says that no command reads the level. It now says that /code-review reads it and that standard and lean run fewer review focuses. The constant is renamed to match, and the CLI test checks the new wording.
Describe the level sets in the README, the CLI reference, the commands guide and the CHANGELOG. State the manifest rule for the Dependencies focus, that a lockfile alone does not keep it, that Security runs at every level, and that --full sets the diff range and never the level. The upgrade notes tell a user with no level that the default is now standard and that "devflow gates --set full" keeps today's set. The known limits note that a personal lean level narrows the review even where compliance is declared, with the compliance focus still running on its trigger.
- docs/cli-reference.md: scope the lean set to a code diff, name the Compliance trigger and the reduced classes, so the CLI reference no longer says a docs-only lean run gets Security, Testing and Reliability. - tests/guards/code-review-levels.test.ts: the Gate level order check now reports a template that lost its Date or Merge Recommendation line; known-bad probes now cover every block item, both read-paragraph phrases, the root-after-path order, the stamp order, the CLAUDE_CONFIG_DIR count, the Step 1.2b heading, both Requires lines, the Phase 2 spawn line and the level in the Git spawn.
…line Step 1.2b gets its Produces/Requires annotation. The compliance gate sentence returns as one gated line naming COMPLIANCE_ACTIVE, so the disposition table and the tree agree again. The added bytes are paid by two edge-case and principle cuts; both size ceilings are lowered to the new measured sizes. Task: feat/474-code-review-lens-sets-per-level
The review Merge Rules had no precedence, so "No blocking issues" and "Only MEDIUM in blocking" matched even when a planned security focus had no report. The table now says the first matching row applies and the security row sits before both APPROVED rows. Step 8 defers to the table. The summary guard checks that no APPROVED row precedes the security row and that the precedence sentence exists, with known-bad probes for both. The mode reference ceiling drops to the new size.
Owner
Author
Code Review — Cycle 1Full summary withheld (public repository). Review depth: light review. Focuses that ran: security, regression, testing, consistency.
Full report: .devflow/docs/reviews/feat-474-code-review-lens-sets-per-level/2026-10-15_0510/review-summary.md (not committed; ask the author) Posted by devflow · cycle 1 |
dean0x
added a commit
that referenced
this pull request
Oct 15, 2026
…s after #472 (#514) ## Summary PR #508 cut 76 bytes from each command that carries the settings line. It did not change the command byte ceilings. This PR lowers the ceilings to the measured sizes on main (8875e09). The slack is now 0 for each changed row. Only ceiling values change, in `tests/guards/command-size.test.ts` and `tests/fixtures/numeric-floors.json`. The `description` of each changed fixture row now states the new measured size. No prompt text changes. ## Ceilings | Command | On: old → new | Off: old → new | |---|---:|---:| | bug-analysis | 25065 → 24966 | 21077 → 21056 | | debug | 19967 → 19885 | 17046 → 16966 | | dynamic-plan | 36994 → 36880 | 32557 → 32530 | | dynamic-profile | 16487 → 16389 | 12684 → 12673 | | dynamic-tickets | 41450 → 41336 | 37156 → 37129 | | explore | 12944 → 12866 | 10040 → 9964 | | implement | 49883 → 49807 | 47029 → 46953 | | plan | 38096 → 38018 | 35126 → 35050 | | release | 22903 → 22825 | 19609 (no change, already exact) | | research | 14972 → 14894 | 10838 (no change, already exact) | | resolve | 52422 → 52287 | 49220 → 49087 | | self-review | 15889 → 15691 | 12801 → 12619 | Sizes are in bytes. ### Not changed here - `code-review` (on and off): open PR #512 changes this command and its ceilings. - `dynamic-build` (on and off): open PR #505 changes this command and its ceilings. Those PRs set their own ceilings. Open PR #513 changes no command and no ceiling. ## Evidence - `npm run build` on a clean tree: exit 0, 33 compiled, 0 errors, 0 warnings. - The measured sizes equal the slack table in the #508 PR body. There are no differences. - `npx vitest run tests/guards/command-size.test.ts tests/guards/numeric-floor-manifest.test.ts`: 2 files, 16 tests pass. - `npx tsc --noEmit`: exit 0. - CI: see the checks on this PR. ## Breaking Changes None. The change is in test constants only.
…b data flow - Compliance sentence in Step 1.2 names the `code` class again, so a compliance-active docs-only or tests-only diff cannot spawn the focus at `full` or on the fail-closed path. - Step 1.2 Produces and Requires no longer claim the level work. Step 1.2b declares REVIEW_FOCUS_LIST, DIFF_CLASS, CHANGED_PATHS and GATE_LEVEL, and produces GATE_LEVEL. Phase 3 Requires lists GATE_LEVEL and REVIEW_FOCUS_LIST. Step 0b title names the gate level. - Step 1.2 says the code list starts with the eight core focuses and that Step 1.2b may narrow them, instead of calling them unconditional. - Guard: the compliance class limit, a Step 1.2b slice with its own Produces and Requires checks, the Phase 3 and Step 0b checks and the core sentence, each with a known-bad probe. - Cuts that pay for the added bytes (no behaviour change): the repeated "(max 20 per worktree)" in the agent count line, "(wait for completion before 3b)" in Step 3a, "of that worktree" in Step 0b, "(parallel)" in Step 0c, "(the common case)" in Step 0a and the rationale in Step 0e-i item 5. Task: resolve-512-A
… review The Merge Rules row for a missing security report fired only for a "planned" security focus. The planned list is the optional FOCUSES input, which the same level narrowing produces, so a review whose list lacked security could still give APPROVED. The row is now unconditional in review mode. - Spell the Merge Recommendation slot with the four Merge Rules outcomes. - Restore the "brief" limit on the suggestions guidance. - Trim two duplicate phrases to stay inside the byte ceiling. - Guard: evaluate the Merge Rules for FOCUSES lists with and without security, require the security row below the BLOCK MERGE row, pin the slot spelling and the suggestions limit, each with a known-bad probe. - CHANGELOG: describe the unconditional row.
The review reference fell from 3,813 to 3,805 bytes. The ceiling follows it down so the saved bytes cannot be spent again.
…reference At the standard level a code diff keeps the Dependencies focus only when a listed manifest changes. The list missed real manifests, so a change to go.work, a .NET project file, Package.swift, Podfile, pubspec.yaml, build.sbt, mix.exs or deno.json silently dropped the focus. The list now holds them, with glob forms for Gradle build files and .NET props files. A lockfile alone is still not a manifest change. The reference keeps its byte ceiling: the list is longer, and the intro, the file-name sentence and the compliance rule are shorter. The guard checks each new name, and each has a known-bad probe.
The guard did not pin four load-bearing sentences. A change from "for every class" to "for the code class only" in the gate-level step, or a change to "full keeps it and reads nothing", passed the guard. The same held for the file-name matching sentence and the lockfile sentence of the Manifests line in the level reference. The guard now pins all four, and each has a known-bad probe.
… guard Seven failure branches of the guard had no known-bad probe: a missing or extra class row, a malformed cell, a name outside the focus vocabulary, a drop cell that drops security, a name outside the class's full set, an exception on a cell other than code/standard, and the simulator check for a focus outside the full set. Each now has a seed or a probe run that proves it fires. The same pass probes the compliance check on a non-code class, the lockfile check, and the two rule-1 manifest checks.
The level reference is 1,349 bytes after the manifest list grew and the prose around it shrank. The ceiling follows it down so the saved bytes cannot be spent again.
The level guard copied sectionBetween and tableRows byte for byte, the claude-dir guard held a third sectionBetween, and three guards each held the sanctioned resolution line. Move all three to tests/helpers.ts and import them. The diff-gating guard keeps its own copy. Test counts are unchanged.
… check The check for a restated language list matched parts of words, so a sentence with "untrusted" (rust) or "javascript" (java) tripped it. Match whole names, with backticked `go` only. A new test proves that lookalike words pass and that every real name, plain or in backticks, is reported.
… row Step 1.2b sets the whole run to full when a class row is missing or a cell is malformed. The simulator kept only that class at its full set. It now returns every class at its full set. A new test proves it for a missing row, a malformed cell and a missing row of the only class.
At the default standard gate level the TypeScript focus does not run. The feature blurb named it without that condition. Add a sentence that the gate level narrows the conditional focuses.
The Conditional Activation table says Always and names file triggers for focuses that /code-review now narrows by gate level. Add one line that points to the gate level and its Step 1.2b.
…ead guard The budget KB lacked the level reference ceiling and described the reference-move guard for /implement and one read root only. It now covers the /code-review host, both read roots and the default-path read. The learning KB said command references hold rare branches only; the /code-review level table is read on every non-full run.
…les reader in the level guard
Merge origin/main (04f0a26, #514) into feat/474-code-review-lens-sets-per-level. #514 lowers the command byte ceilings of the other commands in numeric-floors.json and tests/guards/command-size.test.ts. Those rows take main's values. The code-review command ceilings keep this branch's values (34,981 learning-on, 32,236 learning-off) because #514 did not change them.
Each known-bad probe re-ran the whole scenario grid, and every manifest check rebuilt its regular expression. Build the grid once, compile each manifest pattern once, and share the oracle's full and expected sets across probes. No check and no probe changes; the slowest test drops from about 2.9 s to about 0.6 s locally.
Owner
Author
Resolution SummaryFull summary withheld (public repository).
Full report: .devflow/docs/reviews/feat-474-code-review-lens-sets-per-level/2026-10-15_0510/resolution-summary.md (not committed; ask the author) |
Owner
Author
Test Plan Evidence — 194406cVerified 34/39: VERIFIED-CI 32, ATTESTED-LOCAL 2, UNVERIFIED 5, STALE 0, FAILED 0, INDETERMINATE 0
|
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
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.
Summary
The /code-review command runs 11 to 20 lenses on every code diff, at every gate level. #472 added GATE_LEVEL. No command reads it yet. Epic #468 defined the lens sets for each level. This PR makes /code-review apply those sets.
Changes
code-review-levels.md. It has a table of lens classes by gate level. Each cell is akeep:list or adrop:list. Both kinds of list only remove lenses.standard, acodediff keeps the Dependencies focus only when a listed manifest changes. Acodediff that changes a lockfile and no manifest drops it atstandard. This is a founder decision, by design. Atlean, acodediff never keeps the Dependencies focus. Alockfile-onlydiff keeps Dependencies at every level. A lockfile alone is not a manifest change. The manifest list is incode-review-levels.md. It has 22 names, and it includesgo.work,*.csproj,Directory.*.props,Package.swift,Podfile,pubspec.yaml,build.sbt,mix.exs,deno.jsonandbuild.gradle*.**Gate level**:line. A planned focus with no report is marked(no report). The row for a missing security report applies in every review, not only when the planned focus list names security. A review with no security report never gets APPROVED. The Merge Rules now state that the first matching row applies. The row for a missing security report comes before both APPROVED rows.devflow gateshelp text, the README, the CLI reference, the commands doc and the CHANGELOG.Breaking Changes
There is no API change. These behavior changes apply:
devflow init, the defaultstandardlevel drops the TypeScript focus. It also drops the Dependencies focus on acodediff that changes no listed manifest, even when that diff changes a lockfile.leanlevel runs the security, testing and reliability focuses. It also runs compliance on its trigger. A lean tests-only diff drops the consistency focus.devflow gates --set fullkeeps the current focus set.devflow initruns, the installed command keeps the current focus set.leanlevel narrows the review, even where compliance is declared. The compliance focus still runs on its trigger. See feat(gates): decide whether the compliance lens sets a floor on GATE_LEVEL #488 for the compliance floor.Reviewer Focus Areas
keep:anddrop:rules and the manifest list in the reference. Check that compliance appears only as a lens name.command-reference-moveandclaude-dir. The sanctioned resolution line is an accepted read root. Exactly one directory read is allowed.codediff, addcompliance", and a guard checks that sentence. Confirm that no other cut text carried behavior. The complete cut list follows.code-review.mds, Principles: "Clear ownership" and "Auto-discover worktrees".code-review.mds, Step 0a item 3: "(the common case)". Step 0b: "of that worktree". Step 0c: "(parallel)". Step 0e-i item 5: "(valuable for Review agent cross-cycle awareness)".code-review.mds, Phase 2: "(max 20 total per worktree)" on the agent-count line. The agent-overload reason on the multi-worktree line is replaced by "so Review spawns never pile up".code-review.mds, Step 1.1: the--no-renamesreason, which the Edge Cases "moved across classes" row restates, and the tail "they add no focus, and Step 1.2 applies to thecodeclass only".code-review.mds, compliance sentence: "to REVIEW_FOCUS_LIST for this worktree".code-review.mds, Step 3a: "(wait for completion before 3b)". Phase 4: "In multi-worktree mode, report results per worktree."code-review.mds, Edge Cases: the "Concurrent sessions" row. The Docs-only, Tests-only and Lockfile-only rows now read "Its Step 1.1 set, narrowed by level". Also cut: "(docs with tests, tests with a lockfile)", "(fail-safe): the 8 core focuses on the full diff", "The count is printed;", "of about 40 lines" and "the old and the new path".code-review.mds, Architecture: "if FP ratio > 70%" on the Step 0e-ii line (the threshold stays in Step 0e-ii); the eight focus names on the Phase 2codeline; the line that lists consistency, regression, testing and reliability; "D7 marker-dedup" and "REVIEW_PUBLICATION" on the Step 3b line.code-review.mds, Backwards Compatibility: the "Single worktree" bullet. The "Legacy flat layout" bullet is replaced by a "Gate level" bullet.synthesize/review.md: the intro line "Synthesize outputs from multiple Review agents. Apply strict merge rules."; "for cross-referencing recurring vs new issues,"; the sentence on agreement between agents for a compliance and security overlap, which ends "is not corroboration"; "Every bucket is reported for resolution; the bucket decides only whether an issue blocks the merge"; "using Write tool"; "3. Confirm file written in final message"; "Brief descriptions only."synthesize/review.md. Neither word is in the final diff.code-review-levels.md: "full-level set" to "full set"; "the named focuses the set holds" to "the named focuses"; "removes the named focuses" to "removes them"; the file-name sentence to "A path matches by file name."; and "This file never adds it" from the compliance rule.tests/guards/code-review-levels.test.tsnow checks that the missing-security row comes after the BLOCK MERGE row.Testing
Related Issues
Closes #474
Part of epic #468. Builds on #472. Related: #473 (same guard lines and goldens), #475 (Validate lint pack), #480 (later level reader), #488 (compliance floor).
Follow-up (founder decision, not this PR): #515 — the counts-only STUB review comment does not show the gate level.
#473 rebases after this PR merges. It then re-runs
npm run test:golden:update -- install-snapshot, and updates the text that names #473 as a later reader of the level: src/cli/commands/gates.ts, README.md, docs/cli-reference.md and CHANGELOG.md.Test Plan