feat(super-checker): iterative multi-critic review-revision loop - #245
Open
marcpickett1 wants to merge 2 commits into
Open
feat(super-checker): iterative multi-critic review-revision loop#245marcpickett1 wants to merge 2 commits into
marcpickett1 wants to merge 2 commits into
Conversation
Adds gpd:super-checker, a general-purpose command that runs an iterative review-revision loop on any physics task or artifact: 1. Agent A (gpd-result-solver) produces an initial result 2. N independent critics (gpd-result-critic) review it in parallel 3. The meta-critic (gpd-meta-critic) synthesizes the reviews, adjudicates each critique, and decides whether revision is needed 4. If converged (no substantive criticisms), the loop exits early 5. Otherwise Agent A revises based on the complete meta-review 6. Repeats up to --loops times (default 5) Usage: gpd:super-checker "Derive the one-loop beta function for QCD" gpd:super-checker "Check the results in GPD/phases/03/RESULTS.md" --critics 5 gpd:super-checker "Verify the ground-state energy calculation" --loops 3 Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
|
📝 WalkthroughWalkthroughThis PR adds the gpd:super-checker command and workflow: Agent A (result-solver) produces results, N parallel result-critics review them, a meta-critic synthesizes and decides convergence, and Agent A revises until STATUS: CONVERGED or the loop cap is reached. It also adds agent specs, CLI docs, workflow docs, and test/baseline updates. ChangesGPD Super-Checker Multi-Agent Workflow
Sequence DiagramsequenceDiagram
participant User
participant Workflow
participant AgentA as ResultSolver
participant Critics as N_ResultCritics
participant MetaCritic
User->>Workflow: run gpd:super-checker TASK --critics N --loops M
Workflow->>AgentA: spawn (initial) with TASK
AgentA-->>Workflow: return <result> (CURRENT_RESULT)
loop per-LOOP
Workflow->>Critics: spawn N critics in parallel with CURRENT_RESULT
Critics-->>Workflow: return concatenated <critique>
Workflow->>MetaCritic: spawn with CRITICS_TEXT + CURRENT_RESULT
MetaCritic-->>Workflow: return meta_review with STATUS
alt STATUS == CONVERGED
Workflow-->>User: final result, loop, CONVERGED
else STATUS == NEEDS_REVISION
Workflow->>AgentA: spawn (revision) with CURRENT_RESULT + LAST_META_REVIEW
AgentA-->>Workflow: return revised <result>
end
end
Workflow-->>User: present final CURRENT_RESULT, LOOP, CONVERGED, REMAINING_CONCERNS
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/gpd/agents/gpd-meta-critic.md (1)
36-69: ⚡ Quick winAdd a language to this fenced output-format block.
Using
```texthere keeps formatting consistent and resolves MD040.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/gpd/agents/gpd-meta-critic.md` around lines 36 - 69, The fenced code block containing the <meta_review> content is missing a language hint; change the opening fence from ``` to ```text so the block is explicitly marked (e.g., update the fenced block around <meta_review> in src/gpd/agents/gpd-meta-critic.md) to satisfy MD040 and keep consistent formatting.src/gpd/agents/gpd-result-solver.md (1)
37-45: ⚡ Quick winAdd language identifiers to fenced template blocks.
Use
```text(or```xml) for these output templates to satisfy markdown lint and keep docs CI clean.Also applies to: 53-67
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/gpd/agents/gpd-result-solver.md` around lines 37 - 45, The markdown fenced template blocks around the <result> and <caveats> sections are missing language identifiers; update the triple-backtick fences that wrap those templates (the ``` blocks surrounding <result> and <caveats>) to include a language tag such as ```text or ```xml so lint/CI accepts them, and apply the same change to the other similar fenced blocks noted (the second occurrence covering the other template).src/gpd/specs/workflows/super-checker.md (1)
36-39: ⚡ Quick winAdd language identifiers to fenced blocks in this workflow spec.
Please mark these fences with
bashortextas appropriate to satisfy markdown lint and keep docs checks stable.Also applies to: 51-58, 64-93, 110-114, 125-158, 166-180, 182-219, 234-236, 246-248, 250-288, 298-304
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/gpd/specs/workflows/super-checker.md` around lines 36 - 39, The fenced code blocks in the workflow spec (e.g., the block showing "ERROR: super-checker requires a task description or artifact path." and the other code snippets at the listed ranges) lack language identifiers; update each triple-backtick fence to include an appropriate language tag (use `bash` for shell/usage examples and `text` for plain output/error blocks) so that markdown lint passes; search for the fenced blocks in src/gpd/specs/workflows/super-checker.md (including the ranges called out: lines ~36-39, 51-58, 64-93, 110-114, 125-158, 166-180, 182-219, 234-236, 246-248, 250-288, 298-304) and add `bash` or `text` after the opening ``` for each block.src/gpd/agents/gpd-result-critic.md (1)
42-62: ⚡ Quick winSpecify fence languages for critique templates.
Please tag these code fences (e.g.,
text) to avoid markdownlint MD040 warnings.Also applies to: 66-79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/gpd/agents/gpd-result-critic.md` around lines 42 - 62, The fenced critique templates (the ``` blocks containing <critique> ... </critique>) are missing explicit fence languages which triggers markdownlint MD040; update those fences to include a language tag (e.g., change ``` to ```text) for the blocks around the <critique> template and the other similar block at lines 66-79 so the markdown parser/linter recognizes the content type and the MD040 warning is resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/gpd/commands/super-checker.md`:
- Line 67: The convergence token string used in the super-checker documentation
is inconsistent: replace the lowercase token `status: converged` with the
canonical uppercase `STATUS: CONVERGED` so it matches the contract used by the
meta-critic and workflow parsers (see symbols STATUS: CONVERGED in
src/gpd/agents/gpd-meta-critic.md and src/gpd/specs/workflows/super-checker.md);
ensure any references or checks that look for the convergence token in code or
docs use the exact `STATUS: CONVERGED` token to avoid cross-layer mismatches.
In `@src/gpd/specs/workflows/super-checker.md`:
- Around line 14-16: Validate the numeric flags by checking prev_token is empty
and that N_CRITICS and MAX_LOOPS contain only positive integers before
proceeding to model resolution and the main loop: add a guard that errors out if
prev_token is set (e.g., "Missing value for $prev_token"), then validate
N_CRITICS and MAX_LOOPS with a positive-integer pattern (reject zero, negatives,
or non-digits) and exit with clear messages like "ERROR: --critics must be a
positive integer." and "ERROR: --loops must be a positive integer."; update the
flag-parsing section that sets N_CRITICS/MAX_LOOPS and the pre-model-resolution
checks to fail-fast on invalid values.
---
Nitpick comments:
In `@src/gpd/agents/gpd-meta-critic.md`:
- Around line 36-69: The fenced code block containing the <meta_review> content
is missing a language hint; change the opening fence from ``` to ```text so the
block is explicitly marked (e.g., update the fenced block around <meta_review>
in src/gpd/agents/gpd-meta-critic.md) to satisfy MD040 and keep consistent
formatting.
In `@src/gpd/agents/gpd-result-critic.md`:
- Around line 42-62: The fenced critique templates (the ``` blocks containing
<critique> ... </critique>) are missing explicit fence languages which triggers
markdownlint MD040; update those fences to include a language tag (e.g., change
``` to ```text) for the blocks around the <critique> template and the other
similar block at lines 66-79 so the markdown parser/linter recognizes the
content type and the MD040 warning is resolved.
In `@src/gpd/agents/gpd-result-solver.md`:
- Around line 37-45: The markdown fenced template blocks around the <result> and
<caveats> sections are missing language identifiers; update the triple-backtick
fences that wrap those templates (the ``` blocks surrounding <result> and
<caveats>) to include a language tag such as ```text or ```xml so lint/CI
accepts them, and apply the same change to the other similar fenced blocks noted
(the second occurrence covering the other template).
In `@src/gpd/specs/workflows/super-checker.md`:
- Around line 36-39: The fenced code blocks in the workflow spec (e.g., the
block showing "ERROR: super-checker requires a task description or artifact
path." and the other code snippets at the listed ranges) lack language
identifiers; update each triple-backtick fence to include an appropriate
language tag (use `bash` for shell/usage examples and `text` for plain
output/error blocks) so that markdown lint passes; search for the fenced blocks
in src/gpd/specs/workflows/super-checker.md (including the ranges called out:
lines ~36-39, 51-58, 64-93, 110-114, 125-158, 166-180, 182-219, 234-236,
246-248, 250-288, 298-304) and add `bash` or `text` after the opening ``` for
each block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c795dee8-febf-4800-ba48-0570aadd02d4
📒 Files selected for processing (5)
src/gpd/agents/gpd-meta-critic.mdsrc/gpd/agents/gpd-result-critic.mdsrc/gpd/agents/gpd-result-solver.mdsrc/gpd/commands/super-checker.mdsrc/gpd/specs/workflows/super-checker.md
| - Critics are spawned in parallel as `subagent_type="gpd-result-critic"`. | ||
| - The meta-critic is spawned as `subagent_type="gpd-meta-critic"`. | ||
| - Model resolution: `gpd resolve-model gpd-result-solver`, `gpd-result-critic`, `gpd-meta-critic`. | ||
| - Convergence: exit the loop when the meta-critic returns `status: converged`. |
There was a problem hiding this comment.
Unify convergence token contract with workflow/meta-critic.
Line 67 uses status: converged, but src/gpd/agents/gpd-meta-critic.md and src/gpd/specs/workflows/super-checker.md define/parse STATUS: CONVERGED. This mismatch is a cross-layer contract break and can lead to incorrect convergence checks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gpd/commands/super-checker.md` at line 67, The convergence token string
used in the super-checker documentation is inconsistent: replace the lowercase
token `status: converged` with the canonical uppercase `STATUS: CONVERGED` so it
matches the contract used by the meta-critic and workflow parsers (see symbols
STATUS: CONVERGED in src/gpd/agents/gpd-meta-critic.md and
src/gpd/specs/workflows/super-checker.md); ensure any references or checks that
look for the convergence token in code or docs use the exact `STATUS: CONVERGED`
token to avoid cross-layer mismatches.
Comment on lines
+14
to
+16
| - `N_CRITICS` — value after `--critics` (default: 3). | ||
| - `MAX_LOOPS` — value after `--loops` (default: 5). | ||
|
|
There was a problem hiding this comment.
Validate --critics and --loops as positive integers before running the loop.
Right now non-numeric or non-positive values are accepted, which can yield invalid behavior (e.g., zero critics, skipped looping semantics, or malformed orchestration). Add explicit checks and fail fast with a clear error.
Suggested validation patch
TASK=""
N_CRITICS=3
MAX_LOOPS=5
+prev_token=""
@@
done
Validate: if `TASK` is empty, stop with:
@@
Usage: gpd:super-checker <task or artifact> [--critics N] [--loops N]+Also validate numeric flags before model resolution:
+
+```bash
+if [[ -n "$prev_token" ]]; then
- echo "ERROR: Missing value for $prev_token"
- exit 1
+fi
+if ! [[ "$N_CRITICS" =~ ^[1-9][0-9]*$ ]]; then - echo "ERROR: --critics must be a positive integer."
- exit 1
+fi
+if ! [[ "$MAX_LOOPS" =~ ^[1-9][0-9]*$ ]]; then - echo "ERROR: --loops must be a positive integer."
- exit 1
+fi
+```
</details>
Also applies to: 34-39, 104-119
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @src/gpd/specs/workflows/super-checker.md around lines 14 - 16, Validate the
numeric flags by checking prev_token is empty and that N_CRITICS and MAX_LOOPS
contain only positive integers before proceeding to model resolution and the
main loop: add a guard that errors out if prev_token is set (e.g., "Missing
value for $prev_token"), then validate N_CRITICS and MAX_LOOPS with a
positive-integer pattern (reject zero, negatives, or non-digits) and exit with
clear messages like "ERROR: --critics must be a positive integer." and "ERROR:
--loops must be a positive integer."; update the flag-parsing section that sets
N_CRITICS/MAX_LOOPS and the pre-model-resolution checks to fail-fast on invalid
values.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
…budgets The super-checker PR broke test collection (pytest exit 2) because gpd-result-solver declared an invalid role_family 'execution'; the agent registry rejects anything outside worker/analysis/verification/review/ coordination, which aborted command/agent discovery during collection. Root fixes: - gpd-result-solver: role_family execution -> worker. - The three new agents (solver/critic/meta-critic) now use scoped_write artifact authority with the canonical internal-specialist boundary pointer, and the critic/meta-critic gain file_write tools to match. - super-checker workflow: replace the malformed inline 'read_only' spawn contracts (empty paths, embedded closing tag) with canonical scoped_write contracts that persist each loop's result, critiques, and meta-review under a per-run GPD workspace directory. Each spawn now declares quoted placeholder paths and uses readonly=false per the workflow spawn convention. Budget/coverage registration for the new surfaces: - command/workflow prompt budget tables gain super-checker baselines. - agent prompt budget table gains the three new agents; per-kind and total agent-prompt ceilings raised to admit the added agents. - spawn-contract inventory registers super-checker (4 contracts). - codex/copilot-cli/opencode bridge-occurrence advisory budgets raised for the new command's runtime bridge snippets. Regenerated artifacts: help surface, detailed command reference, repo-graph contract, tests/README node counts.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/gpd/specs/references/help/detailed-command-reference.md`:
- Around line 193-197: Update the "Notes" section in
detailed-command-reference.md to include the missing required fields so the
final-response note matches the super-checker command contract: explicitly list
"loop count" and "convergence status" alongside the existing defaults and
behavior, and ensure the description of "final response" states it contains the
converged result plus the loop_count and convergence_status fields as defined by
the super-checker contract (refer to the super-checker command contract for
exact field names).
In `@src/gpd/specs/workflows/help.md`:
- Line 235: Update the final-response documentation in help.md to fully list all
required output fields per the gpd:super-checker contract: add explicit entries
for "loop count" (the number of critic loops executed, default N=5) and
"convergence status" (boolean or enum indicating whether the meta-critic closed
early or the result converged), and ensure the paragraph describing the final
response mentions these fields alongside the converged result and any remaining
concerns so the expected payload schema is complete.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fadfa4fb-5e42-43e8-826f-c8f475d3397c
📒 Files selected for processing (13)
src/gpd/agents/gpd-meta-critic.mdsrc/gpd/agents/gpd-result-critic.mdsrc/gpd/agents/gpd-result-solver.mdsrc/gpd/specs/references/help/detailed-command-reference.mdsrc/gpd/specs/workflows/help.mdsrc/gpd/specs/workflows/super-checker.mdtests/README.mdtests/adapters/test_runtime_projection_diagnostics_budget.pytests/core/test_agent_prompt_budget.pytests/core/test_command_prompt_budget.pytests/core/test_prompt_surface_diagnostics_budget.pytests/core/test_spawn_contract_inventory.pytests/repo_graph_contract.json
✅ Files skipped from review due to trivial changes (5)
- tests/core/test_prompt_surface_diagnostics_budget.py
- tests/core/test_spawn_contract_inventory.py
- tests/README.md
- src/gpd/agents/gpd-result-critic.md
- src/gpd/agents/gpd-result-solver.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/gpd/agents/gpd-meta-critic.md
- src/gpd/specs/workflows/super-checker.md
Comment on lines
+193
to
+197
| Notes: | ||
| - Default critics N=3, default loops N=5. | ||
| - The loop exits early when the meta-critic finds no substantive criticisms. | ||
| - The final response includes the converged result and any remaining concerns. | ||
|
|
There was a problem hiding this comment.
Align final-output note with the command contract.
Line 196 omits required fields from the command contract (loop count and convergence status). Please update this note to match src/gpd/commands/super-checker.md so the public reference reflects the full expected final response shape.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gpd/specs/references/help/detailed-command-reference.md` around lines 193
- 197, Update the "Notes" section in detailed-command-reference.md to include
the missing required fields so the final-response note matches the super-checker
command contract: explicitly list "loop count" and "convergence status"
alongside the existing defaults and behavior, and ensure the description of
"final response" states it contains the converged result plus the loop_count and
convergence_status fields as defined by the super-checker contract (refer to the
super-checker command contract for exact field names).
| **`gpd:super-checker <task description or artifact> [--critics N] [--loops N]`** | ||
| Iterative multi-critic review loop — Agent A produces a result, N independent critics fact-check it in parallel, a meta-critic synthesizes the critiques into a complete review, Agent A revises, and the loop repeats until convergence or the loop limit is reached | ||
| Usage: `gpd:super-checker "Derive the ground-state energy of the quantum harmonic oscillator"`; `gpd:super-checker "Verify the one-loop beta function for QCD" --critics 5`; `gpd:super-checker "Check the numerical result in GPD/phases/03-simulation/RESULTS.md" --loops 3` | ||
| Notes: Default critics N=3, default loops N=5. The loop exits early when the meta-critic finds no substantive criticisms. The final response includes the converged result and any remaining concerns. |
There was a problem hiding this comment.
Document all required final response fields.
Line 235 should also mention loop count and convergence status to match the gpd:super-checker contract. Right now it under-documents the expected output payload.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gpd/specs/workflows/help.md` at line 235, Update the final-response
documentation in help.md to fully list all required output fields per the
gpd:super-checker contract: add explicit entries for "loop count" (the number of
critic loops executed, default N=5) and "convergence status" (boolean or enum
indicating whether the meta-critic closed early or the result converged), and
ensure the paragraph describing the final response mentions these fields
alongside the converged result and any remaining concerns so the expected
payload schema is complete.
Collaborator
Author
|
🤖 RoastBot: Built by an AI, for an AI, to check an AI's work. Marctar The Mad has achieved full detachment from the labor market. |
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
3 participants
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
gpd:super-checker, a general-purpose command that wraps any physics task in an iterative review-revision loop--loopsrounds (default 5)New files
src/gpd/commands/super-checker.mdsrc/gpd/specs/workflows/super-checker.mdsrc/gpd/agents/gpd-result-solver.mdsrc/gpd/agents/gpd-result-critic.mdsrc/gpd/agents/gpd-meta-critic.mdUsage
Test plan
gpd:super-checkerappears ingpd helpoutputSTATUS: CONVERGED→ loop exits, result presentedSTATUS: NEEDS_REVISION→ solver revises, loop continues--loopscap, presents result with remaining concerns--criticsand--loopsflags parsed correctly; defaults (3, 5) applied when absent🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests