fix(evidence): rank questions by the ceiling, not the inferred floor (#419) - #421
fix(evidence): rank questions by the ceiling, not the inferred floor (#419)#421pengfei-threemoonslab wants to merge 2 commits into
Conversation
…419) The declaration questionnaire promised an order — "by how much answering can move the verdict" — and delivered the inverse of it. Each question was ranked by `effect_evidence_rank(conservative_effect)`, the effect the scan had *already inferred* for the action, and a pre-filled proposal is offered on exactly the same condition (#416). The two mechanisms ran off one signal, so every question that arrived with a draft answer outranked every question that arrived blank: the cheapest questions first, the most valuable ones last. On the fifth `adk-samples#1745` walk that put three already-drafted mail tools at Q2-Q4 and `create_sap_sales_order` — the single question that produces both `critical` blockers the moment it is answered `financial_write` — at Q6, behind three drafts a reader working top to bottom confirms first. The ranking was faithful to observed risk. Observed risk is not the quantity the header names. A question is now ranked by the **ceiling** of what its answer can establish: where the scan measured a side effect that measurement bounds the answer from below and the old rank still applies, and where it measured nothing there is no bound at all, so the question sorts above every measured one. An action nothing was observed about is not a low-risk action; it is an unmeasured one, and it is exactly where a human answer carries new information. On the same walk the money question moves Q6 to Q3 and all three drafts move to the end. "Measured" and "could be drafted" are one fact, so they are one predicate: `effect_is_measured` is the gate `propose_effect_declaration` already applied, now read from both ends. A heuristic reading of `read` is deliberately not a measurement — this resolver refuses to establish a read-only action from a heuristic (#357), so such an action is as unproven as one with no reading. Within the unmeasured group nothing was observed, so the tiebreaker is the shape of the action's name in three coarse bands, read with the keyword vocabulary the scanner already owns rather than a second one. That vocabulary is deliberately gated for *evidence* on most source types — a Python function called `create_sap_sales_order` is not proof that it writes anything — and ordering is given no more trust than it needs: the band is inert for every measured action, so it cannot reorder anything the scan did read, and a source-level test pins its call sites so it can never reach a claim, an issue, or a verdict. Two rendering changes follow. The header states the order the file actually uses, and a test renders the file and checks the two against each other. And a blank with no reading to print now says the scan read nothing about that action: the header explains that the top of the file is the unread half, and a block that printed nothing let its silence read as "nothing to see here". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pengfei-threemoonslab
left a comment
There was a problem hiding this comment.
I found three correctness/contract issues and one backward-compatibility rendering issue that should be addressed before merge.
- [P1] Keep authoritative read bounds separate from proposal eligibility.
_reachuseseffect_is_measured, but that predicate intentionally returns false for every read-only reading sopropose_effect_declarationnever pre-fillseffect: read. That proposal-safety rule is not a valid whole-question reach rule: OpenAPIGET, MCPreadOnlyHint: true, and an already reviewedeffect: readare bounded reads. Their remaining authority questions nevertheless receive rank 9 plus the repository-controlled name band. A structural GET nameddelete_accounttherefore sorts before a genuinely unknownget_statuseffect question:
r authority rank=9 shape=1
u effect rank=9 shape=-1
u authority rank=9 shape=-1
That reverses this PRs unread-first promise and lets names reorder structural or declared effects. Please separate “may propose a non-read effect” from “effect is bounded for ordering”; declared/structural effects should retain their evidence rank with an inert shape, while only unresolved effects receive the ceiling. Add GET, readOnlyHint, and partially answered declaration regressions.
-
[P2] Update the published ordering contract.
DeclarationQuestionRowstill says “Ordered strongest-acting action first,” andagent-contract-current.mdstill saysopen_questions[]is “highest-acting action first.” The model docstring is emitted verbatim into the current report, packet, and verifier schemas, so machine-facing documentation now contradicts runtime behavior. Update the source docstring and contract text, then regenerate the current schemas. -
[P2] The new header still disagrees with the heuristic-read tier. The header says the first actions are ones the scan “could read nothing about.” A heuristic-only
readobservation is deliberately assigned to that tier, yet its block printsWhat this scan read this action effect as: read. The rendered acceptance test covers empty readings and non-read proposals, but not this third case. Describe the tier as “could not bound/establish” and add a heuristic-read fixture, or change the classification. -
[P3] Do not assert relative placement without question coverage.
_reading_linesalways says an empty-reading question “comes before” measured questions.build_declaration_scaffold(..., questions=None)is explicitly supported for older reports and preserves gap emission order, so a measured gap followed by a blank gap renders in that order while the second block claims it came first. Remove the relative sentence or emit it only when this block is actually numbered by coverage.
Validation: all GitHub checks are green; I also ran the questionnaire, semantic, declaration, effect-coverage, and schema-roundtrip suites plus Ruff and git diff --check. The in-tree Shipgate v24 verifier returned passed / mergeable, and its receipt reproduced as valid. Those checks do not exercise the cases above.
Four findings from review of the ordering change. **[P1] `effect_is_measured` was the wrong predicate to rank with.** It is a proposal-safety rule: it returns False for every read-only reading, however authoritative, because a pre-filled `effect: read` is the one direction where a confirmed guess loses safety (#357). An OpenAPI `GET`, a trusted `readOnlyHint: true`, and a reviewed `effect: read` are all *proven* reads, and all three read as "nothing to propose" — so ranking with it sent them to the ceiling and let the name band break the tie among them. A structural `GET` named `delete_account` led the questionnaire ahead of a genuinely unknown effect: this issue's own defect inverted, with a repository-chosen name ordering something the scan had established. Ordering now asks `effect_is_bounded`, which is two clauses because the resolver records the two cases differently: an effect status of `declared`, `structural`, or `conflicting` — which is what catches a reviewed `effect: read`, since a declaration leaves no reading behind — or an observed side effect, which is the `inferred` action whose keyword hint reads `external_communication`. A heuristic reading of `read` is in neither: this resolver may not act on it, so the answer stays open. Three regressions cover it, all named to look as mutating as the band can score, and all three fail against the old predicate. `_PendingQuestion` is now seeded from its first action rather than from a zero floor, which a bounded `read` reaches exactly. **[P2] The published ordering contract said the old thing.** `DeclarationQuestionRow`'s docstring is emitted verbatim into the report, packet, and verifier schemas, and it — with `docs/agent-contract-current.md` — still said "highest-acting action first". Both now describe the ranking and state that position is not severity: the action at the top is the one *least* is known about. Schemas and `llms-full.txt` regenerated. No field shapes change. **[P2] The header still excluded one member of its own tier.** "could read nothing about" is false for an action whose only reading is a heuristic `read` — it prints that reading above its block and is still unbounded. The header now describes the tier as what nothing has pinned down, "no effect evidence at all, or only a reading this scan is not allowed to act on", and the rendered acceptance test carries all three shapes: nothing read, a protocol default, and a heuristic read. **[P3] The block note claimed a position it may not have.** `build_declaration_scaffold(..., questions=None)` is supported for older reports and preserves gap emission order, so a blank can follow a bounded question. The sentence claiming placement is now emitted only when the block is actually numbered by coverage; the rest of the note, which says what the silence means, is unconditional. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
All four addressed in 4055234. Thank you — finding 1 was a real inversion of this PR's own thesis, and none of my fixtures could have caught it. 1 [P1] — bounded is not the same test as draftable. You are right, and the reasoning that produced the bug is worth stating: I argued "measured" and "could be drafted" are one fact, so they should be one predicate. They are one fact — which is exactly why Ordering now asks
A heuristic Three regressions added — OpenAPI 2 [P2] — contract updated and regenerated. 3 [P2] — header reworded, third fixture added. Reworded rather than reclassified, for the reason above. The tier is now "the actions nothing has pinned down: no effect evidence at all, or only a reading this scan is not allowed to act on." The rendered acceptance test carries all three shapes — nothing read ( 4 [P3] — placement claim gated on numbering. Split the note: what the silence means is unconditional, and "It is asked before the ones the scan could read for itself" is emitted only when the block is numbered by coverage. The regression builds the same gaps twice, with and without coverage, and asserts emission order really does put the drafted block first in the uncovered case — so the test would fail if the premise ever stopped holding. One note on your validation: the in-tree verifier returning Full gate green again locally; reference walk order and the two |
Closes #419.
The defect
The declaration questionnaire promised an order — "ordered by how much answering can move the verdict, so the first questions are the ones about money, outward communication, and destruction" — and delivered the inverse of it.
Each question was ranked by
effect_evidence_rank(conservative_effect): the effect the scan had already inferred. A pre-filled proposal is offered on exactly the same condition (#416). So the two mechanisms ran off one signal, and every question that arrived with a draft answer outranked every question that arrived blank — the cheapest questions first, the most valuable ones last.send_emailrisk_hint:keyword→ external_communicationcreate_sap_sales_orderwritefallback)On the fifth
adk-samples#1745walk that put three already-drafted mail tools at Q2–Q4 andcreate_sap_sales_order— the single question that produces bothcriticalblockers the moment it is answeredfinancial_write— at Q6, behind three drafts a reader working top to bottom confirms first.Rank by the ceiling, not the floor
The ranking was faithful to observed risk. Observed risk is not the quantity the header names. An action nothing was observed about is not a low-risk action; it is an unmeasured one, its answer can still turn out to be
destructive, and it is exactly where a human answer carries new information.A question is now ranked by the ceiling of what its answer can establish. Where the scan measured a side effect, that measurement bounds the answer from below and the old rank still applies; where it measured nothing, there is no bound at all, so the question sorts above every measured one (
UNMEASURED_EFFECT_RANK)."Measured" and "could be drafted" are one fact, so they are one predicate:
effect_is_measuredis the gatepropose_effect_declarationalready applied, now read from both ends. A heuristic reading ofreadis deliberately not a measurement — this resolver refuses to establish a read-only action from a heuristic (#357), so such an action is as unproven as one with no reading at all.Reference walk, before → after:
Answering the money question still yields
SHIP-POLICY-APPROVAL-MISSING+SHIP-ACTION-FINANCIAL-WRITE-CONTROL-MISSINGandblocked; it is now reached three questions earlier.A name may order what it may not judge
Within the unmeasured group nothing was observed, so alphabetical order is no order at all — and that group is the whole questionnaire on a repository that has just adopted. The tiebreaker is the shape of the action's name, in three coarse bands, read with the keyword vocabulary the scanner already owns rather than a second one.
That vocabulary is deliberately gated for evidence on most source types — a Python function called
create_sap_sales_orderis not proof that it writes anything, andgoogle_adkis not in_KEYWORD_GATED_SOURCE_TYPESfor exactly that reason. Ordering is given no more trust than it needs:0) for every measured action, so it cannot reorder anything the scan did read, even by accident;Getting a band wrong costs a reader one place in a list they must finish either way.
What "never precedes" can and cannot mean
Acceptance bullet 2 reads "a question carrying a proposal never precedes a blank question of equal or lower inferred rank." Taken universally that means all blanks sort first — and enforcing it literally breaks two things worth more:
authorityquestion always arrives blank and follows that action'seffectquestion by design. They are oneaction_surface.actionsrow and one block; sorting every blank first separates them, and the block that answers both gets announced as "Questions 1 and 7" with no block numbered 7 anywhere in the flow.What is enforced is the statement the mechanism actually has, which is the same claim with the ambiguity removed: every question the scan could draft an answer for sorts after every question about an action it measured nothing about. Since drafting and measuring are one condition, no effect question carrying a proposal precedes a blank effect question.
Two rendering changes
Acceptance
create_salesforce_quote, Q2).test_a_drafted_question_never_precedes_an_unmeasured_one.Tests
Eight new tests. Five of them fail against the old ranking (verified by reverting
_reachand re-running). Plus a negative control that a name cannot reorder a measured action, a source-level guard on the band's call sites, and an exhaustive check that the proposal gate and the ordering gate are one predicate.Full gate green:
pytest -n auto -m "not perf",tests/test_adapter_static_only.py,tests/test_latency_budget.py -m perf,ruff check .. No report-schema change, no sample-golden churn (every shipped sample reports0open declaration questions).🤖 Generated with Claude Code