feat(workflows): adversarial-verification mean+veto rewire (V2, #2487) - #2535
Conversation
Declare finalDecision, scoreTablePath, and reviewReportPath without placeholder values that GitHub Code Quality flagged as dead stores. Every loop path assigns them before return. Assistant-model: Grok 4.6
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Type.Record(Type.String(), Type.String()), | ||
| ], { default: DEFAULT_CRITERIA, description: "Criteria record of name-to-description entries, or criteria.md markdown." }), | ||
| accept_mean: Type.Number({ default: 14, description: "Mean score required for acceptance on the 1–20 verification scale." }), | ||
| reask_limit: Type.Integer({ minimum: 0, default: 1, description: "Maximum bounded re-ask waves for invalid criterion reports." }), |
There was a problem hiding this comment.
reask_limit accepts any non-negative integer, and the runner dispatches an additional verifier wave for every retry through that value. Persistently invalid reports therefore let a caller drive arbitrarily many parallel model calls and workflow time; this repeats for each indeterminate repair round. Bound this input to a product-appropriate maximum and defensively clamp it in the runner before dispatching retry waves.
Artifacts
- Authored Bun TypeScript harness invokes the actual workflow with one invalid verifier cell and reask_limit 8, records every parallel wave, and cleans up its temporary workflow files; it is the executable reproduction source.
- Captured output from running the harness in /home/user/repo shows reask_limit 8 passed schema validation and dispatched 18 verifier waves before quorum failure; the uncapped retry behavior is confirmed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/workflows/builtin/adversarial-verification.ts
Line: 22
Comment:
**Uncapped verifier retry waves**
`reask_limit` accepts any non-negative integer, and the runner dispatches an additional verifier wave for every retry through that value. Persistently invalid reports therefore let a caller drive arbitrarily many parallel model calls and workflow time; this repeats for each indeterminate repair round. Bound this input to a product-appropriate maximum and defensively clamp it in the runner before dispatching retry waves.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| function schemaShape(schema: unknown): Record<string, unknown> { | ||
| return schema as Record<string, unknown>; | ||
| } |
There was a problem hiding this comment.
The new schemaShape helper accepts unknown and returns it through an unchecked assertion, removing compile-time validation from the schema tests and violating the repository requirement to use specific types.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: test/unit/builtin-workflows-adversarial-generate.test.ts
Line: 57-59
Comment:
**Schema helper erases concrete types**
The new `schemaShape` helper accepts `unknown` and returns it through an unchecked assertion, removing compile-time validation from the schema tests and violating the repository requirement to use specific types.
**Context Used:** AGENTS.md ([source](https://github.com/bastani-inc/atomic/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Replacement for #2511 after the stacked-PR base branch was deleted. Same branch, now targeting main (V1 already merged).
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Greptile Summary
This PR replaces reducer-based adversarial verification with per-criterion scoring, deterministic acceptance, retry handling for invalid verifier reports, and repair rounds. Runtime testing confirmed that a caller can set
reask_limitto an arbitrarily large value and cause a new verifier wave for every requested retry when reports remain invalid. The schema test helper also removes the concrete type checking required by the repository rule.Confidence Score: 4/5
Do not merge until verifier retry work is bounded; otherwise caller-controlled input can produce unexpectedly long and expensive verification runs.
The retry failure was reproduced against the actual workflow with schema-valid input and invalid verifier output. The remaining finding is a repository-rule violation in the test helper's type signature.
Files Needing Attention: packages/workflows/builtin/adversarial-verification.ts needs an upper bound for
reask_limit, with matching defensive handling in adversarial-verification-runner.ts. test/unit/builtin-workflows-adversarial-generate.test.ts needs a concrete schema type.What T-Rex did
Comments Outside Diff (1)
General comment
reask_limit. For every invalid verifier cell, the workflow dispatches the initial verifier wave plus one additional parallel wave for each requested re-ask. Persistent invalid responses repeat the same process for a second indeterminate round, so worst-case verifier calls grow as2 * (reask_limit + 1) * criterion_count * verifier_countrather than remaining product-bounded.packages/workflows/builtin/adversarial-verification.ts:22hasminimum: 0and default1but no maximum. The runner at line 225 floors/clamps only the lower bound, and its inclusive loop at lines 284-290 uses the unbounded value directly.maximumto the TypeBoxreask_limitinput and defensively clampreaskLimitin the runner to that same constant before the retry loop. Add a focused test asserting values above the limit are rejected or capped and cannot create additional waves.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "merge origin/main" | Re-trigger Greptile
Context used: