Add a prioritized run mode - #200
Conversation
An AutoProver run does the same amount of work regardless of how much the answer is worth. Property inference fans out per component, and every property it invents then gets the full CVL authoring, prover and counterexample treatment. Formalization is usually the dominant cost, and it scales with (components x properties). This adds a second run mode. `comprehensive` is today's behaviour and stays the default. `prioritized` ranks every candidate property across every component once, then spends the run on the highest-contribution property plus the properties needed to state or discharge it. The mode rides on `PipelineRun`, which the driver and the CVL author both already hold, so nothing else needed a new parameter. `--run-mode` sets it, and so does `AUTOPROVER_RUN_MODE`, which means the cloud can turn it on by adding one variable to the Batch job. That is the same route `AUTOPROVER_GLOBAL_PROVER_TIMEOUT` already takes. Ranking is a single structured call between extraction and formalization, cached under the properties namespace. Its output is validated before anything acts on it: every candidate ranked exactly once, primary and supporting all from one component, supporting capped and deduplicated. An unusable ranking raises rather than falling back to the whole set, since a silent widening would spend exactly what the mode exists to save. The author's exits tighten around the focus. `record_skip` refuses the focus properties, and `give_up` rejects an exhausted stop until the prover has actually been run. Two escapes are deliberate. A `sort='environment'` stop is never gated, because a missing certoraRun produces no prover run at all and a bare floor would be unsatisfiable. And the budget monitor's wrap-up lifts the skip protection, since the order it gives requires the skips that protection forbids. The give-up floor counts verify_spec tool calls rather than prover_history. That list only grows when the prover returns a report, and a spec that will not type-check never gets one. The output says what it did not look at. report.json carries run_mode and the deprioritized properties with their scores and rationales, coverage carries the deprioritized count, the full ranking goes to property_ranking.json, and job_info.json records the mode on every path. Scope is the prover flow. The foundry and rust-app entry points do not expose the flag, since the focus protections live in the CVL author. Also forks FINAL_PROPERTIES_KEY on the mode: a prioritized run would otherwise overwrite a comprehensive run's record for the same component with its pruned batch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four fixes from review, and one deliberate non-change. The ranking model no longer states which property the run pursues. Nothing forced `primary` to be the top of `ranked`, so property_ranking.json could rank A first while the run went after D, with nothing to catch it. Rather than define a priority formula, tell the model to sort by it and then check the arithmetic, the formula lives in code and code does the picking. `primary` and `supporting` come off the schema; each ranked property carries `depends_on` instead, so the winner's dependencies are available without a second call. The artifact and the run's actual subject are now the same thing by construction. Validation got smaller in the process. A cache hit could smuggle a retired focus past every protection. FORMALIZATION_KEY is derived from the property list, so a pruned batch usually forks the cache, but not when the focus is the component's whole list, which a single-property component always is. A replay never runs the author's tools, so the gated give_up and the skip protection do not apply to it, and a prioritized run could return a comprehensive result that had skipped the one property it existed to establish. A hit that does not map every focus property, or that retired one, is now treated as a miss. cache-autoprove could not find prioritized results, and was already wrong twice over. It rebuilt the formalization key from the raw property-inference output with no plugins argument, while the driver writes it from the post-plugin batch plus the contributing plugin ids. Pruning was a third way to disagree. It now reads FinalProperties, which was written for exactly this and had no reader, and falls back to the old path for records from before that entry existed. Three docstrings pointed at composer.meta.run as the offline walker; that module does not exist, so they now name the one that does. Component labels could still collide. Numbering repeats breaks when a component is itself named `Vault (2)`, and the colliding label resolves a ranked property onto the wrong batch. Labels are now suffixed until actually unused, and asserted. MAX_SUPPORTING drops from 4 to 2. Review argued the supporting properties should be author context rather than batch members; they stay first-class, since a dependency that is only suggested is never verified and the primary's proof could then rest on an unchecked assumption. But a batch of five was more than "one property plus what it rests on" ought to mean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude answers: Four of the five are right and are fixed in b89217b. One I have pushed back on, with the part of it that was fair taken. 1. The selected primary is not derived from the ranking. Agreed, fixed, though not quite as proposed. The bug is real: nothing forced The recommendation was a combined priority score, validated ordering, and CRITICAL_MATCH_BONUS = 15
def priority(rp): return rp.score + (CRITICAL_MATCH_BONUS if rp.critical_match else 0)
2. Cache hits should be checked for focus compatibility. Agreed, and this was worse than P2. Right, and the consequence is sharper than the finding says. A replay never calls Implemented as recommended. 3. Cache inspection cannot locate prioritized results. Agreed, and it is broader than reported.
Fixed exactly as recommended: 4. Five properties instead of one. Pushing back on the reframing, taking the magnitude. This one re-litigates a decision that was made deliberately. The design question was put explicitly and "top-1 plus its supporting cluster" was chosen over "exactly one property", with the batch specified as 1..k. So the behaviour matches intent rather than drifting from it. On the merits: a dependency handed to the author as context is never verified, so the primary's proof could rest on an unchecked assumption. Keeping the supporting properties first-class means the judge demands rules for them and they are skip-protected, which is the point of pulling them in at all. What is fair in the finding is the magnitude. "One property plus what it rests on" should not quietly mean five, so 5. Generated component labels can collide. Agreed, fixed. Confirmed by construction:
Finding 3 is the one the suite cannot really cover, since it walks a live LangGraph store. Verifying that one by hand against a prioritized run and an existing comprehensive run. |
| return f"Giving up on {label}: {p['reason']}" | ||
|
|
||
|
|
||
| def _verify_attempts(state: dict) -> int: |
There was a problem hiding this comment.
you can do better on this type I think
There was a problem hiding this comment.
Claude answers: done
| @tool_family(GiveUpParams) | ||
| class GatedGiveUp( | ||
| WithInjectedId, | ||
| WithInjectedState[dict], |
There was a problem hiding this comment.
ibid, this is a weak sauce type bound.
There was a problem hiding this comment.
Claude answers: done
| return tool_state_update( | ||
| self.tool_call_id, | ||
| f"Rejected: you have run the prover {ran} time(s) on this task, and this run " | ||
| f"requires at least {floor} before a property may be abandoned. This is the " | ||
| "one property the run exists to establish. Decompose it into lemmas, add or " | ||
| "strengthen a harness, or formalize a weaker core of it and state the gap — " | ||
| "then verify. If the toolchain itself is broken, stop with " | ||
| "sort='environment' instead.", | ||
| ) |
There was a problem hiding this comment.
for this, its my preference to make the return type of the tool be str | Command. And then do return "Error message" on the error path. It's more explicit about which branches are updating state and which are "oopsie didn't work"
There was a problem hiding this comment.
Claude answers: done
| def gated_give_up_tool( | ||
| *, | ||
| name: str, | ||
| description: str, | ||
| label: str, | ||
| min_attempts: int, | ||
| reason_description: str = "The reason for giving up on your task", | ||
| ) -> BaseTool: |
There was a problem hiding this comment.
tioli, I prefer to make the dependencies of the state of a tool like this explicit. You can do this with:
class GatedGiveUpDeps(TypedDict, MessagesState):
result: ReadOnly[str]
failed: ReadOnly[bool]
class gated_give_up_tool[T: GatedGiveUpToo](
*,
...,
state_ty: type[T]
) -> BaseTool:
return GatedGiveUp[state_ty].with_template(...)
There was a problem hiding this comment.
Claude answers: not done as sketched. The state type is now GatedGiveUpState rather than dict, but the generic parameter does not survive the tool_family decorator: it returns an already-specialized _TemplatedTool, so GatedGiveUp[state_ty] is a pyright error.
There was a problem hiding this comment.
I think GatedGaveUp.with_template(...)[t] might work? I honestly forget; but I think eric used parametricity and tool families successfully.
There was a problem hiding this comment.
Claude answers: done. You were right, the subscript just has to come after with_template rather than before it. The state type is SourceCVLGenerationState now, so failed turned out to be bool | None and required.
| threat_model_digest: str | None, | ||
| with_refinement: bool, | ||
| extra_context_digest: str | None = None, | ||
| run_mode: str = "comprehensive", |
There was a problem hiding this comment.
literal type, remove the default.
There was a problem hiding this comment.
Claude answers: done
| - `score`: 0-100, how much *proving it* would raise confidence in the overall correctness of the | ||
| system. Score importance, not difficulty. |
There was a problem hiding this comment.
There is absolutely no scale provided here. Any ordering within 0-100 provides information for relative positioning I suppose, but what does a score 100 look like? 5? 25? Are we fine just leaving that up to the agent?
There was a problem hiding this comment.
Claude answers: done. Anchored with a scored example at each level.
| - `depends_on`: the properties from the SAME component that would be needed to state or discharge | ||
| this one — the lemmas it assumes, the invariants its argument leans on. Only the run's chosen | ||
| property has its dependencies pursued, and only the first {{ max_supporting }} of them, so put | ||
| the ones the proof genuinely cannot do without first. A property that is merely *related* is | ||
| not a dependency; leave it out. |
There was a problem hiding this comment.
see my comments elsewhere about my dubiousness around the utility of what we're asking for here. There is a sort mismatch here in what we're telling it to find within "depends on".
There was a problem hiding this comment.
Claude answers: done
| W1. Its violation is a real loss — funds, access, or an accounting identity — not a cosmetic | ||
| deviation. |
There was a problem hiding this comment.
"real loss"
It is at this point, I realize we've never even told the agent this is for a Web3/Defi protocol. So it has no idea what we're talking about here.
There was a problem hiding this comment.
with how tied we are to solidity I don't think this is not sensible
There was a problem hiding this comment.
Claude answers: done
| WHAT MAKES A PROPERTY WORTH THE RUN: | ||
| W1. Its violation is a real loss — funds, access, or an accounting identity — not a cosmetic | ||
| deviation. | ||
| W2. It constrains behaviour across many entry points rather than one narrow path. |
There was a problem hiding this comment.
Commounweaulth speulling jump scaure.
There was a problem hiding this comment.
But also, what does this mean? This sounds like you're asking for "cross cutting", but I'd argue that's not a really important metric. A rebalanceShares function that is called in one entry point is probably the most important in the whole contract.
There was a problem hiding this comment.
Claude answers: done
There was a problem hiding this comment.
Claude answers: done. Dropped.
| not a dependency; leave it out. | ||
| - `rationale`: one or two sentences. Say what breaks if the property does not hold. | ||
|
|
||
| WHAT MAKES A PROPERTY WORTH THE RUN: |
There was a problem hiding this comment.
what run! this is the first time you've mentioned a "run"
There was a problem hiding this comment.
Claude answers: done
The ranking read nothing that pre-formalization produces, but it waited for all of it anyway. On a RiverV1 run that meant four hours of autosetup and structural invariant proving before the run made the one decision the mode exists to make, with a single invariant prover job accounting for two of those hours. The extracted batches and the guidance documents are what the ranking needs, and both are in hand the moment extraction lands. Moving it ahead of the await also puts the focus in hand before the setup finishes, which is what a setup step would need to aim at it. The empty-batch check stays where it was, after the await. Raising it earlier would let "no properties extracted" mask a setup that failed, which two of the overlap tests rightly caught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review found that the "supporting lemmas" idea was not one. A property that follows from another is not something this pipeline models, the property agent is told not to write them, and what the field would actually have collected is "these are important too". So the ranker now groups candidates into claims and the run pursues the group holding the highest-scoring property. Grouping is a shape the report already asks a model for and gets, unlike a dependency graph with no cycle check and nothing to say about indirect dependencies. The ranking prompt is rewritten against its sibling, property_analysis_system_prompt.j2. It never said the subject was a Web3 protocol, never explained what a run or a verification budget was, and asked for scores on a 0-100 scale it never anchored, which also left the flagged-concern bonus measured against nothing. It now opens on the domain, says where the work sits, and anchors the scale with scored examples. Two bits of advice come out of the author prompt. Telling a stuck agent it may add a `require` that rules out unreachable states reads as licence to assume away the states where the property is interesting, which is the opposite of the training this thing has had. What remains is decompose or weaken honestly, with weakening tied to the rule that a property saying nothing is a failure rather than a result. The focus protection read an in-memory flag that the budget monitor flipped. It was not checkpointed, so a resumed session could have the flag and the state disagreeing about whether the run was wrapping up. It reads `budget_curtailed` from the state now, which is where that was recorded all along. The field is optional on the shared state: only the prover and foundry authors have a budget, and natspec and rustapp share the tool. Also: the retry replays the rejected ranking, without which the model was asked to fix something it had no memory of producing; validation reports every problem at once rather than one per attempt; component labels carry the unit index, so two components sharing a name need no invented suffix; and the give-up gate takes a real state type instead of `dict`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| def gated_give_up_tool( | ||
| *, | ||
| name: str, | ||
| description: str, | ||
| label: str, | ||
| min_attempts: int, | ||
| reason_description: str = "The reason for giving up on your task", | ||
| ) -> BaseTool: |
There was a problem hiding this comment.
I think GatedGaveUp.with_template(...)[t] might work? I honestly forget; but I think eric used parametricity and tool families successfully.
| if batches and run.run_mode is RunMode.PRIORITIZED: | ||
| with named_budget_or_nop("property_extraction"): | ||
| batches, deprioritized = await _prioritize( | ||
| backend.analysis_spec.properties_key, backend.artifact_store, | ||
| batches, run, phases["extraction"], threat_model, extra_context, | ||
| ) | ||
|
|
||
| staged = await staged_task | ||
| if not batches: | ||
| raise ValueError("No properties extracted from any component.") |
There was a problem hiding this comment.
Claude answers: not done. It already overlaps: staged_task is a live task started before extraction, so the setup keeps running while the ranking waits on its model call. Gathering them explicitly reads better but changes nothing at runtime, and the fuller version breaks two tests that pin this pair as overlapped-but-not-gated.
The tool family does take a type parameter, with the subscript after the render rather than before it. GatedGiveUp is generic in its state again, and the CVL author names SourceCVLGenerationState at the bind site, so the fields the gate reads and the surrender records are checked against the state it actually runs on instead of a bound that merely happened to be satisfied. Matching the real state meant `failed` is `bool | None` and required, not an optional `bool`. The test state was seeding neither, which the bound now catches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Why
Today there is nono guaranteed correlation between the effort put by the agent and the value of the output. Property inference fans out per
component, every property it invents gets the full CVL authoring + prover + counterexample
treatment, and
budget_integration.mdalready namesformalizationthe dominant cost. That phasescales with (components x properties), so an inference agent that thinks of forty properties bills
for forty.
What
Today's behaviour becomes run mode
comprehensiveand stays the default.prioritizedranks everycandidate property across every component once, then spends the run on the highest-contribution
property plus the properties needed to state or discharge it. One property, pursued properly,
instead of everything pursued shallowly.
Concretely, a prioritized batch is the primary property plus at most two dependencies drawn from
its own component. All three are first-class: the feedback judge demands rules for each, they count
in the report, and all of them are skip-protected. The model does not pick the primary. It scores
every candidate and records what each rests on;
priority = score + 15 if critical_matchis appliedin code, so
property_ranking.jsonand the property the run actually spends itself on are the samething by construction.
The insertion point is one seam in
composer/pipeline/core.py, between_extract_allandstaged.begin. That is the first and only point where every component's candidates exist together:inference never sees more than the unit it was given, and the per-component plugin hook cannot
compare across units either.
How it is turned on
--run-mode prioritized, orAUTOPROVER_RUN_MODE=prioritized.The value rides on
PipelineRun, which the driver and the CVL author both already hold, sorun_pipelineand the backend protocols needed no new parameter.The ranking is not trusted
A single structured call, cached under the properties namespace, validated in Python before anything
acts on it: every candidate ranked exactly once, no invented keys, and every
depends_onnaming aproperty of its own component. An unusable ranking raises rather than falling back to the whole
candidate set, because a silent widening would spend exactly what this mode exists to save. The
failure is cheap anyway, since nothing has been formalized yet.
The author may not walk away
Two escape hatches would each be a total loss when the run is one property, so both tighten:
record_skiprefuses the focus properties, andgive_uprejects anexhaustedstop until theprover has actually been run a few times, with the attempts enumerated.
Two escapes are deliberate, and both are the difference between a gate and a deadlock:
sort='environment'is never gated. A missingcertoraRunorsolcproduces no prover run atall, so a bare attempt floor would be unsatisfiable and would hold the agent against its recursion
limit instead of letting it report a broken toolchain.
skip the rest, publish", which is exactly what the protection forbids.
The floor counts
verify_spectool calls rather thanprover_history.prover_historyonly growswhen the prover returns a report, and
verify_specreturns early with a bare string when the specwill not type-check, which is the failure most worth escalating through.
The output says what it did not look at
A prioritized
report.jsonwould otherwise be indistinguishable from a contract with threeproperties. So
report.jsoncarriesrun_modeand the deprioritized properties with their scoresand rationales,
coveragecarries adeprioritized_countand a warning, the full ranking is writtento
property_ranking.json, andjob_info.jsonrecords the mode on every path including a crash.What does not get cheaper
prepare_formalizationis untouched, so the harness lift,AutoSetup, custom summaries, structural invariants and the invariant CVL/prover loop cost exactly
what they cost today, as does per-component property inference (it has to run before anything can be
ranked). The saving is the formalization fan-out, which is the dominant cost. Two more caveats: in
--cloudmode components already verify in parallel, so the wall-clock win is smaller than the moneywin, and a
GaveUpis never cached, so a prioritized run against a genuinely unprovable property isthe worst case for this design.
Scope
Prover flow only. The foundry and rust-app entry points keep their own parsers and do not expose the
flag, because the focus protections live in the CVL author: a prioritized foundry run would get the
narrowing without the discipline, which is the worst of both.
Also in here
FINAL_PROPERTIES_KEYnow forks on the mode. Its leaf was fixed apart from the threat-model,interactive and extra-context parameters, so a prioritized run would have overwritten a
comprehensive run's record for the same component with its one-property batch, and an offline walker
would have read the pruned list as if it were everything inference found.
Testing
tests/test_prioritize.pycovers the validation and the cut, including that a supporting propertyfrom another component is rejected and that the selection can never return an empty batch list (the
driver raises on that).
tests/test_focus_exits.pycovers the gate and the skip protection onboth binding paths, since the author reaches its skip tools either through
skip_toolsdirectlyor through
property_tools, and a protection on only one is silently bypassable. Driver-level testsin
tests/test_pipeline_staged_formalizer.pyprove one batch reaches formalization, thatbeginstill runs once beforehand, and that the ranker never runs in comprehensive mode.
pytest -m "not expensive": 1126 passed.pyright: 0 errors. The 21 errors in the suite are a localpgvectortestcontainer that will not stay up; they are intest_rag_db/test_indexed_toolandpredate this branch.
Not validated here
The ranker's judgement. The mechanism is tested; whether it picks the right property is what the
first real runs are for, and
property_ranking.jsonis the artifact to read when checking that.