Pular para conteúdo

AUR-579: the model weighs the deterministic evidence

Measured before (base 19219477)

  • session.Order was resolve, model, evidence, gate, publish: on both --base and --pr the model answered before the security pass, the embedded analysis and Semgrep ran; their findings were appended to the result afterwards and the model never saw them (ReviewContext.Evidence existed since AUR-578 and was never filled by cmd).
  • The repository context reached the model through config.WrapProviderWithWarnings, a decorator that hid the provider's capabilities (separate system/user messages); fixedModelProvider (--limite) did not implement llm.Unwrapper.
  • The model cache and verdict-reuse key (reviewContextCacheKey) carried no evidence digest; the per-file model cache stored issues only.
  • The model had no way to answer per evidence item, and nothing checked an evidence_id against what was offered.

What changed

  • internal/review/session/session.go: Order is resolve, evidence, model, gate, publish.
  • Evidence phase (cmd/aurumcode/review_evidence.go, review_base_analysis.go, review_pr_analysis.go): the security pass, the embedded analysis (staticAnalysisIssues) and SAST run first and touch no review result; each finding gets its engine origin (security, analysis, sast). offerEvidence (review_weighing.go) turns them into prompt.EvidenceItems in a fixed order (origin, file, line, rule) with ids E1..En, the redacted diff line as snippet and the side.
  • Model phase: both sources send ReviewContext.Evidence and ReviewContext.RepositoryContext; the context block is built by config.BuildContextBlockWithWarnings (buildRepositoryContext) and the provider is no longer wrapped. fixedModelProvider implements llm.Unwrapper (it forwards requests unchanged); profileProvider alters the request and deliberately does not. The phase ends with joinEvidence: the model's assessments are copied onto the evidence, the evidence joins the result exactly where it did before (security in the issues on --pr and apart on --base, analysis after the model's issues, SAST last), then the rule config, the verdict snapshot, the SAST notice and the coverage, so stdout/stderr keep their previous order.
  • Answer format: the deterministic_evidence slot of internal/prompt/templates/review.md (rendered only when evidence exists, so the system prompt and every evidence-free prompt keep their bytes) asks for evidence_assessments: evidence_id, status (confirmed|disputed|needs_context), justification, correlates_with, priority, suggestion, and says the engine already counts each item. pkg/types: ReviewResult.EvidenceAssessments, and EvidenceAssessment gains Correlates, Priority, Suggestion. The parser treats a malformed evidence_assessments as an optional section (discarded and recorded, not a degraded parse). internal/review/assessments.go (weighAssessments) keeps only assessments of offered ids with a known status (top-level and issue-level), first per id, normalizes priority, keeps only correlations to other offered ids, and names every refusal in assessment_discard_warning, printed on stderr by both sources. The model never writes Origin (the parser clears it; the engine sets it) and never changes a severity.
  • Sinks: terminal report (printAssessment: origem: <engine> | avaliacao do modelo: <status> [<id>] prioridade ... - <justification>, correlation, suggestion), audit (evidence_assessments[] with origin and assessment; proposed_exceptions), SARIF (properties.assessment beside properties.origin, only for assessed findings, so every other SARIF keeps its bytes).
  • Gate (internal/gate/triage.go, proposed.go, contributors.go): Run.Triage holds the disputed finding keys and the sources whose disputes demote. reviewState.triage (review_gate.go) fills the sources only without a central policy, from gate.triage; under a policy nothing is demoted, including a triage the policy itself declares. The analysis, security (under analysis), SAST (never for a policy-origin section) and repository skills contributors drop demoted findings and record them in Run.Demoted; each demotion is printed and added to the limitations. Every disputed finding that still counts becomes a proposed exception (ProposeExceptions / RenderProposedExceptions): the exceptions YAML with rule, path and the model's reason, and placeholders for owner, expiry and (without a verified identity) the repository. It is text only; nothing reads it back.
  • internal/config/gate.go: gate.triage (map source -> model|none), validated in Parse (unknown source or value is an error); default none.
  • Caches (AC-005): reviewState.contextCacheKey is the old context key when no evidence was offered (evidence-free runs keep their keys) and otherwise cache.RequestKey{PromptDigest: <old key>, EvidenceDigest: DigestOf(evidence)}; it feeds both the per-file cache lookup and the verdict-reuse ContextKey. A review that offered evidence bypasses the per-file model cache (errEvidenceNotCacheable): the cache stores issues, not assessments, so a hit would silently lose the model's verdict on the evidence. Reviewer.RequestCacheKey is not used directly because the cache is per file (its prompt digest covers the whole diff).
  • Offline fixture (internal/review/fixture_cases.go): a fixture whose JSON has the reserved key aurumcode_fixture answers by case (prompt_contains -> response, else default); every other fixture is returned verbatim as before.

How the evidence section is proven not decorative (AC-004)

The gate tutorial's fixtures answer with an assessment only when the prompt contains [E1] origem=analysis regra=analysis/hardcoded-secret, text that only the evidence slot emits; otherwise they answer approve with no assessment. The recorded expected therefore contains the model's assessment and the proposed exception only because the section reached the prompt. TestAUR579EvidenceSectionDecidesTheAnswer asserts the same through the real command, and the AC-004 mutation (renderEvidenceSlot returns "") turns it RED by assertion: without the section the same fixture answers its default, the report has no assessment and the proposed exception disappears, so the expected lines of gate/modelo-pondera would diverge.

Fail-closed choices

  • The model never writes Origin; an assessment with an unknown status, no id, or an id never offered is discarded and named on stderr.
  • The evidence ceiling limits only the prompt: the slot states "N omitidos" and every finding still reaches the gate (the gate reads the passes, never the offered list).
  • Under a central policy the assessment never changes the count; a dispute is only a proposal a human may copy.
  • renderSlot (internal/prompt/slots.go) still panics on a template error: this card did not touch slots.go (pending from AUR-578, left as is).

Proof

All Go commands in the shared container, in series.

Item Command Last line
AC-001 tests/acceptance/AUR-579.sh AC-001 AUR-579/AC-001/pass
AC-002 ... AC-002 AUR-579/AC-002/pass
AC-003 ... AC-003 AUR-579/AC-003/pass
AC-004 ... AC-004 (behavior + expected of the tutorial + section-removal mutation) AUR-579/AC-004/MUT/rejected
AC-005 ... AC-005 AUR-579/AC-005/pass
MUT-001 Order back to model before evidence RED: the prompt sent to the model lacks "[E1] origem=analysis ..."; AUR-579/MUT-001/rejected
MUT-002 the policy guard of reviewState.triage disabled RED: policy: a disputed policy finding must still fail the gate: exit=0; AUR-579/MUT-002/rejected
all tests/acceptance/AUR-579.sh all AUR-579/all/pass

Re-anchored tests of done cards (same semantics): TestRunFollowsTheOneOrder stops at the evidence phase by its position in Order; AUR-578's TestEvidenceSlotRendersRedactsAndDeclaresOmissions raises the evidence ceiling of its "some but not all of 40" check from 220 to 400 tokens because the section's instructions grew; TestModelAssessmentParsedAndOriginIgnored now offers the evidence it assesses (an assessment of evidence never offered is discarded by design); AUR-490's TestAUR490SharedPasses asserts both sources call staticAnalysisIssues (which calls mergeStaticAnalysis). No tests/acceptance script of another card was edited; the anchors of AUR-449, 450, 473, 522, 537, 542, 543, 557, 569 and 576 are unchanged.

Card v2 closings

  • Terminal verdict follows the gate after triage: when gate.triage: model (no central policy) demoted at least one finding and the declared gate passes, the --base report no longer reads "Changes requested" (gateAlignedVerdict, the terminal counterpart of the --pr gateAlignedReviewEvent); without a demotion the verdict is the review's own, so no other tutorial output moved. Proof: TestAUR579VerdictFollowsTheGateAfterTriage.
  • --pr body: each finding carries the terminal's own line (printAssessment: origin beside the model's assessment, correlation, suggestion), and the proposed exceptions are a separate fenced block appended to the published body (appendProposedExceptions), no longer a limitation item. Proof: TestAUR579PublishedBodyCarriesAssessmentAndProposal (render test).
  • A proposed exception exists only when a gate is declared: without a gate nothing counts, so there is nothing to except.
  • Tutorial revisao/modelo-pondera (registered in docs/specs/AUR-561.md): a local review without gate where the model disputes the analysis finding and confirms the security finding on the same line; only the report changes. Tutorial gate/modelo-pondera (registered in docs/specs/AUR-562.md) covers the policy floor, the confirmation and gate.triage.
  • Execucao real: the 15 tutorials were re-recorded with aurum-tutoriais:aur579 built from this tree, each all=0 check=0.

Review closings (after fda0c1c6)

  • P1: the evidence section reports which ids it admitted under its ceiling (renderEvidenceSlot returns them; PromptParts.Meta["evidence_admitted"], not part of the prompt digest). weighAssessments accepts only assessments of admitted ids; an id omitted by the ceiling ("N omitidos") is discarded with the same stderr warning as an id never offered, so the model can never dispute (or, under gate.triage: model, demote) evidence it did not read. The gate still counts every finding. Proof: TestAssessmentOfEvidenceOmittedByTheCeilingIsDiscarded (40 items, ceiling 400 tokens, dispute of the omitted ev-39: discarded, warning names it).
  • P3: Triage.Disputed is keyed by gate.DisputeKey(origin, rule, path, line); each contributor matches with its own origin. Proof: TestTriageMatchesTheDisputeByOrigin.
  • P2: documented in docs/configuration.md that gate.triage.analysis also covers the --seguranca findings (origin security), as gate.sources: analysis does; the vocabulary stays the three source names.