From 1ad5d16af3b2ea9005e41816f342999e36d5acfa Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 26 Aug 2026 22:13:41 -0400 Subject: [PATCH] Refresh the code-review workflow (cross-target cell packing, 12-check cap) --- .../workflows/code-review/code-review.fabro | 2 +- .../code-review/scripts/code_review.py | 40 +++++++++++-------- .../code-review/specs/report-spec.md | 5 ++- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/.fabro/workflows/code-review/code-review.fabro b/.fabro/workflows/code-review/code-review.fabro index 5120677c8..0a6d56e3d 100644 --- a/.fabro/workflows/code-review/code-review.fabro +++ b/.fabro/workflows/code-review/code-review.fabro @@ -33,7 +33,7 @@ digraph CodeReview { timeout="300s", output_schema="routing", stdin_source="context.internal.run_id", - script="python3 -c \"import hashlib,sys; pairs=list(zip(sys.argv[1::2],sys.argv[2::2])); sys.exit(0 if pairs and all(hashlib.sha256(open(path,'rb').read()).hexdigest()==expected for path,expected in pairs) else 91)\" .fabro/workflows/code-review/scripts/code_review.py b072907e97842df2c7f665614a83655cc92d44a5ff1b7ca053a74d0bfad873a9 .fabro/workflows/code-review/scripts/git_readonly.py bcd4364ba3aca2ee1e12d5909204f645c16bdf22e3753a39d74c79d8d37cf73e .fabro/workflows/code-review/scripts/render_report.py 5b92107f1173a45900933931c4e48cb0378a9c2d72d00889e313f8b0c8ba22b5 .fabro/workflows/code-review/scripts/rule_loader.py f885409c64c631075d7e31fcb6e7a100592430c5eba9a6e989222006061a9f52 .fabro/workflows/code-review/specs/report-spec.md 662dbdcc72a2c28e7336f6b4a3da6c08f1b62501021e5c9f2addb667daad38ea .fabro/workflows/code-review/templates/report.html 64eefc612bcf51d4bdd53282a3feeccc37555b1ef584dd179a254450e3c010c6 .fabro/workflows/code-review/schemas/findings.schema.json 6fdf7c63fb6183c8bef64a874cd56f805d92c2aa3c011a513ee07f07b0797b6d .fabro/workflows/code-review/schemas/verdict.schema.json 4cb752e624f1b88809017ac5db472d59850ffe09ec5a93d3b63ba636364b8b19 .fabro/workflows/code-review/schemas/file-groups.schema.json b53c4e1c0bbd07bbf70e83f4f3b35fd96cb880c621c7c424e95b9aea34e13d7c .fabro/workflows/code-review/prompts/finder.md.j2 86c2e6a032f7c54c1bbab1c12496a8f0d6bf48703abe6017eb175330608cf223 .fabro/workflows/code-review/prompts/verify.md.j2 6528cffd1bf199f27a32c19547aebf6f0c78a546698aaef81c6a89441b80f7da .fabro/workflows/code-review/prompts/sweep.md.j2 e6f89b47b11c57030a6ef7d5896ccb37dbd2a2982e9fa7eb7f2e3df73acab82c .fabro/workflows/code-review/prompts/group-files.md.j2 5b291313a1266d1d658f80ea7989cdefcd8609b6893b7197b17914d553dab041 .fabro/workflows/code-review/prompts/partials/finding-fields.md.j2 b91ddeb86dc7f552323f7ac04823984da9365d013bfe7965c3eb6f51fa8abe0f .fabro/workflows/code-review/prompts/partials/guidance.md.j2 53bc0c40bb917288708bed1f9ba478fbd89b9790c92497762224cc752f40bef5 .fabro/workflows/code-review/prompts/partials/output-schema.md.j2 811994bb357739f2562d84f66dc05075ebe3c7f8d58034f8c25ee1c36bee996b .fabro/workflows/code-review/prompts/partials/read-only-explorer.md.j2 44a0244e7aa62fdb0dbbfdbadcffbfb640af249bae3e96895dedd5c7a33bad10 .fabro/workflows/code-review/prompts/partials/review-target.md.j2 abffeeff0e16b89a0754cd53f1833b3744494cd54ff761798b782a80467446ea .fabro/workflows/code-review/prompts/partials/safe-git-history.md.j2 4ddd8d36d5c51d7e166a6b7f1dff51b72cce0e64108cc7e892002ca909af8b3a .fabro/workflows/code-review/rules/builtin-manifest.json 47e3123fb25c560e36216dc9b8323c86e8301f1868a4fa3219bfd59bda3a533a && python3 .fabro/workflows/code-review/scripts/code_review.py prepare --review-id-stdin --mode {{ inputs.mode }} --effort {{ inputs.effort }} --scope {{ inputs.scope }} --base {{ inputs.base }} --commit {{ inputs.commit }} --range {{ inputs.range }} --model {{ inputs.model }} --guidance {{ inputs.guidance }}" + script="python3 -c \"import hashlib,sys; pairs=list(zip(sys.argv[1::2],sys.argv[2::2])); sys.exit(0 if pairs and all(hashlib.sha256(open(path,'rb').read()).hexdigest()==expected for path,expected in pairs) else 91)\" .fabro/workflows/code-review/scripts/code_review.py 0926622b190a45b2abfd478f17bcdb7c49e3b20615867b8f84d0bb5b067ac663 .fabro/workflows/code-review/scripts/git_readonly.py bcd4364ba3aca2ee1e12d5909204f645c16bdf22e3753a39d74c79d8d37cf73e .fabro/workflows/code-review/scripts/render_report.py 5b92107f1173a45900933931c4e48cb0378a9c2d72d00889e313f8b0c8ba22b5 .fabro/workflows/code-review/scripts/rule_loader.py f885409c64c631075d7e31fcb6e7a100592430c5eba9a6e989222006061a9f52 .fabro/workflows/code-review/specs/report-spec.md 6a6925d3a4f74baec99e6ce26d0a04f64f767ff3a78ef14158b6ba14e8d34d60 .fabro/workflows/code-review/templates/report.html 64eefc612bcf51d4bdd53282a3feeccc37555b1ef584dd179a254450e3c010c6 .fabro/workflows/code-review/schemas/findings.schema.json 6fdf7c63fb6183c8bef64a874cd56f805d92c2aa3c011a513ee07f07b0797b6d .fabro/workflows/code-review/schemas/verdict.schema.json 4cb752e624f1b88809017ac5db472d59850ffe09ec5a93d3b63ba636364b8b19 .fabro/workflows/code-review/schemas/file-groups.schema.json b53c4e1c0bbd07bbf70e83f4f3b35fd96cb880c621c7c424e95b9aea34e13d7c .fabro/workflows/code-review/prompts/finder.md.j2 86c2e6a032f7c54c1bbab1c12496a8f0d6bf48703abe6017eb175330608cf223 .fabro/workflows/code-review/prompts/verify.md.j2 6528cffd1bf199f27a32c19547aebf6f0c78a546698aaef81c6a89441b80f7da .fabro/workflows/code-review/prompts/sweep.md.j2 e6f89b47b11c57030a6ef7d5896ccb37dbd2a2982e9fa7eb7f2e3df73acab82c .fabro/workflows/code-review/prompts/group-files.md.j2 5b291313a1266d1d658f80ea7989cdefcd8609b6893b7197b17914d553dab041 .fabro/workflows/code-review/prompts/partials/finding-fields.md.j2 b91ddeb86dc7f552323f7ac04823984da9365d013bfe7965c3eb6f51fa8abe0f .fabro/workflows/code-review/prompts/partials/guidance.md.j2 53bc0c40bb917288708bed1f9ba478fbd89b9790c92497762224cc752f40bef5 .fabro/workflows/code-review/prompts/partials/output-schema.md.j2 811994bb357739f2562d84f66dc05075ebe3c7f8d58034f8c25ee1c36bee996b .fabro/workflows/code-review/prompts/partials/read-only-explorer.md.j2 44a0244e7aa62fdb0dbbfdbadcffbfb640af249bae3e96895dedd5c7a33bad10 .fabro/workflows/code-review/prompts/partials/review-target.md.j2 abffeeff0e16b89a0754cd53f1833b3744494cd54ff761798b782a80467446ea .fabro/workflows/code-review/prompts/partials/safe-git-history.md.j2 4ddd8d36d5c51d7e166a6b7f1dff51b72cce0e64108cc7e892002ca909af8b3a .fabro/workflows/code-review/rules/builtin-manifest.json 47e3123fb25c560e36216dc9b8323c86e8301f1868a4fa3219bfd59bda3a533a && python3 .fabro/workflows/code-review/scripts/code_review.py prepare --review-id-stdin --mode {{ inputs.mode }} --effort {{ inputs.effort }} --scope {{ inputs.scope }} --base {{ inputs.base }} --commit {{ inputs.commit }} --range {{ inputs.range }} --model {{ inputs.model }} --guidance {{ inputs.guidance }}" ] grouping [ diff --git a/.fabro/workflows/code-review/scripts/code_review.py b/.fabro/workflows/code-review/scripts/code_review.py index e15fa5db3..8b33b7098 100644 --- a/.fabro/workflows/code-review/scripts/code_review.py +++ b/.fabro/workflows/code-review/scripts/code_review.py @@ -66,6 +66,11 @@ MAX_CHANGED_FILES_LISTED = 200 # Rule-mapped review shape (every tier above low). GROUP_MAX_FILES = 10 GROUP_CHAR_BUDGET = 2000 # estimated per-job path payload, in characters +# A cell audits at most this many checks; a larger effective set splits into +# evenly sized cells over the same files. Each cell reports at most +# candidate_cap findings, so unbounded checks would dilute every check's +# share of the cell's attention as repository rules stack up. +MAX_CHECKS_PER_CELL = 12 DISCOVERY_JOB_CEILING = 64 # discovery jobs (local + angle + rule-audit) # A small target at medium collapses to local passes and rule audits only # (no whole-change angles), mirroring the security-review workflow's @@ -1342,27 +1347,30 @@ def build_rule_audit_cells( state: Mapping[str, Any], groups: Sequence[Sequence[str]], ) -> List[Dict[str, Any]]: - """Intersect file groups with per-file effective checks, deterministically. + """Pack files sharing one effective check set into audit cells. - Files inside one semantic group that share the same effective check-ID - set audit together; the ten-file and size caps still apply. + Packing is across the whole target, not within semantic groups: a rule + audit checks each file against the same guidance regardless of its + neighbors, so grouping only multiplied cells (calibration measured + rule cells as 43% of finder agents for 11% of candidates). Cells are + deterministic -- lexical files per check set, the ten-file and size + caps applied -- and ordered by check set, then first file. """ rules_state = state.get("rules") or {} effective: Mapping[str, Sequence[str]] = rules_state.get("effective") or {} - cells: List[Dict[str, Any]] = [] - for group in groups: - by_check_set: Dict[Tuple[str, ...], List[str]] = {} - for path in group: - check_ids = tuple(effective.get(path) or ()) - if not check_ids: - continue + by_check_set: Dict[Tuple[str, ...], List[str]] = {} + for path in sorted(path for group in groups for path in group): + check_ids = tuple(effective.get(path) or ()) + if check_ids: by_check_set.setdefault(check_ids, []).append(path) - for check_ids in sorted(by_check_set): - for chunk in chunk_paths(sorted(by_check_set[check_ids])): - cells.append( - {"files": chunk, "check_ids": list(check_ids)} - ) - cells.sort(key=lambda cell: (cell["files"][0], tuple(cell["check_ids"]))) + cells: List[Dict[str, Any]] = [] + for check_ids in sorted(by_check_set): + slice_count = -(-len(check_ids) // MAX_CHECKS_PER_CELL) + slice_size = -(-len(check_ids) // slice_count) + for start in range(0, len(check_ids), slice_size): + check_slice = check_ids[start : start + slice_size] + for chunk in chunk_paths(by_check_set[check_ids]): + cells.append({"files": chunk, "check_ids": list(check_slice)}) return cells diff --git a/.fabro/workflows/code-review/specs/report-spec.md b/.fabro/workflows/code-review/specs/report-spec.md index c6740a810..561a62efa 100644 --- a/.fabro/workflows/code-review/specs/report-spec.md +++ b/.fabro/workflows/code-review/specs/report-spec.md @@ -72,7 +72,10 @@ Every tier above `low` projects one rule-mapped structure: - One local-correctness finder job per final group, four whole-change angle jobs (behavior preservation, contracts and data flow, design economy, performance and lifetime), and one rule-audit job per non-empty cell of - files sharing the same effective check set. Discovery is capped at 64 + files sharing the same effective check set, packed across the whole + target rather than within groups (at most ten files and twelve checks + per cell; a larger check set splits into evenly sized cells over the + same files). Discovery is capped at 64 jobs; a target that cannot fit fails before dispatch rather than omitting files or checks. A small target at `medium` (at most 5 files and 300 changed lines, or a scope of at most 5 files) collapses the shape to the