diff --git a/README.md b/README.md index 51a97d5..7b6e0a6 100644 --- a/README.md +++ b/README.md @@ -91,7 +91,7 @@ jobs: with: ref: ${{ github.event.pull_request.base.sha }} # trusted base; PR head is fetched as review data fetch-depth: 0 - - uses: 0xPolygon/codegenie@v0.5.3 + - uses: 0xPolygon/codegenie@v0.5.4 with: model: "anthropic/claude-opus-5:high" llm-api-key: ${{ secrets.LLM_API_KEY }} diff --git a/examples/workflows/codegenie-review-comment.yml b/examples/workflows/codegenie-review-comment.yml index 89dd70c..81ca342 100644 --- a/examples/workflows/codegenie-review-comment.yml +++ b/examples/workflows/codegenie-review-comment.yml @@ -34,7 +34,7 @@ jobs: with: fetch-depth: 0 - - uses: 0xPolygon/codegenie@v0.5.3 + - uses: 0xPolygon/codegenie@v0.5.4 with: model: "anthropic/claude-opus-5:high" llm-api-key: ${{ secrets.LLM_API_KEY }} diff --git a/examples/workflows/codegenie-review-pr.yml b/examples/workflows/codegenie-review-pr.yml index 7caf3dc..d6c6486 100644 --- a/examples/workflows/codegenie-review-pr.yml +++ b/examples/workflows/codegenie-review-pr.yml @@ -34,7 +34,7 @@ jobs: ref: ${{ github.event.pull_request.base.sha }} fetch-depth: 0 - - uses: 0xPolygon/codegenie@v0.5.3 + - uses: 0xPolygon/codegenie@v0.5.4 with: model: "anthropic/claude-opus-5:high" llm-api-key: ${{ secrets.LLM_API_KEY }} diff --git a/package.json b/package.json index 93db3b9..fc8a531 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@0xsequence/codegenie", - "version": "0.5.3", + "version": "0.5.4", "description": "High-signal AI code review agent", "type": "module", "bin": { diff --git a/specs/plans/106-issue-106-verifier-revision-payload-contract.md b/specs/plans/106-issue-106-verifier-revision-payload-contract.md new file mode 100644 index 0000000..cbfc313 --- /dev/null +++ b/specs/plans/106-issue-106-verifier-revision-payload-contract.md @@ -0,0 +1,215 @@ +# Issue 106: Enforce Meaningful Verifier Revisions and Calibrate Confidence + +Status: PENDING +Planned from: trails-api eval `49f4645b`, especially runs 55 and 57, 2026-08-04 +Planned at: commit `1824056` (branch `master`) +Recommended priority: immediate. Stage 9 proved the expected predicate but +lost it through malformed revision semantics and uncalibrated confidence. + +> **Executor instructions**: Follow this plan step by step. Run every +> verification command and confirm the expected result before moving on. Stop +> on any condition below; do not improvise. Update this plan's row in +> `specs/plans/README.md` when complete. +> +> **Drift check (run first)**: +> `git diff --stat 1824056..HEAD -- src/llm/schemas.ts src/pipeline/verifier.ts src/skills/prompt-builder.ts src/evals/eval-scoring.ts tests/phase4-llm.test.ts tests/verifier.test.ts tests/pipeline-phase5.test.ts tests/evals.test.ts tests/shared-utils.test.ts specs/project/components/review_pipeline.md specs/project/components/evals.md` +> STOP if the verdict schema, one-repair seam, or verification artifact shape +> no longer matches Current state. + +## Execution metadata + +- **Priority**: P1 +- **Effort**: M +- **Risk**: MED +- **Depends on**: none +- **Category**: bug +- **Planned at**: commit `1824056`, 2026-08-04 + +## Why this matters + +Run 57's verifier proved the exact-output under-delivery chain, then submitted +`verdict: "revise"` without `finalFinding` or `revisedAnchor`. Both fields are +optional today, so the harness completed the revision while retaining the +original low-confidence candidate; composition then withheld its gate-only +anchor and suppressed it. + +Run 55 exposed the sibling contract problem: the verifier kept the same proven +predicate at low confidence solely because a secondary lookup hit budget. +Run 53 published when the verifier instead returned calibrated medium +confidence. Stage 9 needs an explicit rule: keep means unchanged; any +structured change uses revise; confidence follows decisive verified evidence, +not inherited generation confidence or secondary tool pressure. + +## Current state + +- `SubmitVerificationVerdictSchema` at `src/llm/schemas.ts:250-262` is one + object with optional `finalFinding` and `revisedAnchor` for every verdict. +- `normalizeSubmittedVerdict` accepts every evidence-backed non-reject and + does not reject an empty revise. +- `runVerifierStructured` already provides exactly one compact schema-repair + attempt and maps persistent failure to an incomplete verdict. Reuse it. +- `verifyCandidate` applies `finalFinding` whenever present, even on `keep`; + historical provider output therefore includes keep-with-payload records. +- Stage 9 prompt `p9.6` explains when to revise but not the payload or + confidence contract. +- `verificationOutcome` reports all incomplete records as + `verification-incomplete`. + +## Commands you will need + +| Purpose | Command | Expected on success | +|---------|---------|---------------------| +| Install | `pnpm install --frozen-lockfile` | exit 0 | +| Focused tests | `pnpm exec vitest run tests/phase4-llm.test.ts tests/verifier.test.ts tests/pipeline-phase5.test.ts tests/evals.test.ts tests/shared-utils.test.ts` | all selected tests pass | +| Checks | `pnpm run check` | exit 0 | +| Full tests | `pnpm test` | all tests pass | +| Build | `pnpm build` | exit 0 | +| Provider/case validation | run `pnpm dev eval --eval-dir --no-cache` for `49f4645b`, `0c4d5213`, and `relay-wc` under `/home/peter/Dev/0xPolygon/codegenie-private-evals/trails-api` | provider accepts the schema; required and `should_not_find` guards hold | + +## Scope + +**In scope**: + +- `src/llm/schemas.ts`, `src/pipeline/verifier.ts`, + `src/skills/prompt-builder.ts`, `src/evals/eval-scoring.ts`. +- `tests/phase4-llm.test.ts`, `tests/verifier.test.ts`, + `tests/pipeline-phase5.test.ts`, `tests/evals.test.ts`, and + `tests/shared-utils.test.ts` only for the why-ledger guard. +- `specs/project/components/review_pipeline.md`, + `specs/project/components/evals.md`, and the plan status row. + +**Out of scope**: + +- Composer thresholds, representative-anchor publication, promotion policy, + additional repair attempts, private eval configuration, or historical + artifact rewrites. +- Making an exact anchor mandatory for a real unanchorable finding. + +## Git workflow + +- Branch: `fix/verifier-revision-contract` +- Suggested commit: `fix(verifier): enforce meaningful revisions` +- Do not push or open a PR unless asked. + +## Steps + +### Step 1: Encode the non-empty revision contract + +Replace the permissive schema with an object union: + +1. `keep | reject`, retaining today's optional payload properties for provider + and historical compatibility; +2. `revise` with required `finalFinding` and optional `revisedAnchor`; +3. `revise` with required `revisedAnchor` and optional `finalFinding`. + +Both revise forms use `additionalProperties: false`; both payloads together +remain valid. Bump `SCHEMA_VERSIONS.submit_verdict` from 1 to 2. Through the +real `validateToolCall` path, test minimal keep/reject, legacy keep-with-finding, +both valid revise forms, revise-with-both, empty revise, and an extra property. + +After local tests, the operator must run a real configured-provider Stage-9 +smoke before further implementation is considered mergeable. Local validation +does not prove that the provider accepts a root object-union tool schema. If it +rejects the schema, STOP and replace the schema strategy in a fresh reviewed +plan; do not add provider-specific rewriting here. + +**Verify**: +`pnpm exec vitest run tests/phase4-llm.test.ts` -> all schema cases pass, then +the provider smoke reaches Stage 9 without a tool-schema rejection. + +### Step 2: Make verdict and confidence semantics explicit + +Bump Stage 9 prompt `p9.6` to `p9.7` and add one compact contract: + +- bare `keep` means confidence, severity, evidence, wording, and placement are + publishable unchanged; +- any structured change uses `revise`; revise without `finalFinding` or + `revisedAnchor` is invalid, and prose in `reason` changes nothing; +- when a low-confidence promoted predicate is confirmed, return a complete + `finalFinding` with calibrated confidence/evidence; add `revisedAnchor` only + when exact changed-line placement is proven; +- medium is appropriate when decisive changed-code evidence and the failure + mode are confirmed even if a narrow secondary check remains unresolved; +- tool refusal, truncation, or budget pressure on a secondary check must not + hold confidence low; if the decisive predicate is unconfirmed, reject or set + `requiredEvidencePresent: false`; +- low remains appropriate for speculative reachability, ambiguous intent, or + weak path matching. + +Add generic why-ledger entries for the run-57 empty revision and run-55 +secondary-budget confidence cap. Test key phrases, not the full prompt. + +**Verify**: +`pnpm exec vitest run tests/pipeline-phase5.test.ts tests/shared-utils.test.ts` +-> all tests pass. + +### Step 3: Repair empty revisions and canonicalize legacy keeps + +Extend `VerifierSchemaInvalidKind` with +`revise_without_revision_payload`. Classify a submit call with `verdict: +"revise"` and neither non-null payload before generic invalid-arguments +classification. Its compact repair prompt must require one valid payload or a +change to keep/reject. Preserve the existing one-attempt repair, counters, and +budget behavior. + +In `normalizeSubmittedVerdict`, before the evidence check: + +- canonicalize `keep` with either payload to `revise`, preserve the payload, + and emit `verification_keep_payload_canonicalized` with candidate id and + payload kinds; bare keep remains unchanged; +- map an empty revise from a non-validating adapter/test double to incomplete, + emit `verification_semantic_invalid` with the stable reason, and never treat + it as verifier rejection. + +Tests must cover successful repair, repair failure, budget-exhausted repair, +legacy keep canonicalization, bare keep, and the non-validating empty-revise +defense. A completed empty revise must never enter `verified`. + +**Verify**: +`pnpm exec vitest run tests/verifier.test.ts tests/pipeline-phase5.test.ts` -> +all cases pass. + +### Step 4: Attribute the unrecovered loss and validate broadly + +In `verificationOutcome`, map an incomplete reason containing +`revise_without_revision_payload` to stable subreason `empty-revision`; retain +`verification-incomplete` for every other incomplete cause. Add both eval +tests and document the schema, repair, canonicalization, telemetry, and loss +contract. + +Run all gates and the three eval cases. Across at least 10 repeats of +`49f4645b`, require zero completed empty revisions. Inspect low-confidence +keep/revise records: none may cite only secondary tool pressure while claiming +the decisive predicate is confirmed. Any new `should_not_find` violation or +material publication inflation on the two quiet cases is a stop-ship signal +for the confidence wording, not a reason to tune the cases. + +**Verify**: all commands exit 0; `git diff --check` is silent; only Scope files +and the plan status row changed. + +## Done criteria + +- [ ] Empty revise is schema-invalid, repaired once, or persisted incomplete. +- [ ] Bare keep means unchanged; keep-with-payload canonicalizes to revise. +- [ ] A real configured provider accepts the schema before merge. +- [ ] Stage 9 carries the revision and confidence contract at `p9.7` (or the + next legitimate version) with why-ledger coverage. +- [ ] Eval scoring distinguishes `empty-revision` from other incomplete work. +- [ ] Revised-anchor-only and unanchored complete-finalFinding behavior remains + valid. +- [ ] Focused, full, build, repeat, and cross-case guards pass. +- [ ] Only Scope files and the plan status row changed. + +## STOP conditions + +Stop if local or real-provider schema validation rejects a valid verdict; the +runner no longer owns one repair attempt; revised-anchor-only or unanchored +complete findings regress; the fix requires changing publication thresholds or +publishing a gate-only anchor; cross-case guards regress; or focused tests fail +twice after a reasonable correction. + +## Maintenance notes + +Verdict meaning belongs in structured state, never inferred from prose. Keep +the adapter defense even after provider validation. Any future requirement that +every confirmed promotion return `finalFinding` is a separate schema migration. diff --git a/specs/plans/107-issue-107-related-promotion-signals.md b/specs/plans/107-issue-107-related-promotion-signals.md new file mode 100644 index 0000000..785cc24 --- /dev/null +++ b/specs/plans/107-issue-107-related-promotion-signals.md @@ -0,0 +1,212 @@ +# Issue 107: Carry Related Promotion Signals Without Changing Selection + +Status: PENDING +Planned from: trails-api eval `49f4645b`, repeated promotion behavior in runs 52-57, 2026-08-04; redesigned after overfit review +Planned at: commit `1824056` (branch `master`) +Recommended priority: after Issue 106. This improves the evidence available to +Stage 9 without changing which candidates receive the scarce verifier slots. + +> **Executor instructions**: Follow this plan step by step. Run every +> verification command and confirm the expected result before moving on. Stop +> on any condition below; do not improvise. Update this plan's row in +> `specs/plans/README.md` when complete. +> +> **Drift check (run first)**: +> `git diff --stat 1824056..HEAD -- src/types.ts src/util/text-similarity.ts src/pipeline/uncertainty-promotion.ts tests/uncertainty-promotion.test.ts tests/verifier.test.ts specs/project/components/review_pipeline.md specs/project/components/evals.md` +> If promotion admission, ranking, selection, candidate construction, or +> provenance changed, compare live behavior with Current state and STOP on a +> semantic mismatch. + +## Execution metadata + +- **Priority**: P2 +- **Effort**: M +- **Risk**: MED +- **Depends on**: none +- **Category**: bug +- **Planned at**: commit `1824056`, 2026-08-04 + +## Why this matters + +Promotion currently discards every eligible source beyond the small verifier +cap. In run 57, the cap correctly selected one EXACT_INPUT and one EXACT_OUTPUT +predicate, but three additional EXACT_OUTPUT framings from other packets were +marked lane-limited and disappeared before Stage 9. Those signals are useful +context, but they are not independent proof and should not change confidence, +rank, or slot allocation by themselves. + +This plan therefore leaves admission, ordering, caps, selected sources, and +candidate ids unchanged. It attaches strongly related **unselected** sources +to the already-selected candidate as bounded, explicitly non-authoritative +provenance. A false association may add a noisy lead, but cannot merge two +selected predicates, erase a verifier slot, or raise confidence. + +## Current state + +- `src/pipeline/uncertainty-promotion.ts:75-128` admits and ranks sources, + selects at most `promotionLimit(...)`, labels every other eligible source + `promotion_lane_limited`, and builds each candidate from one selected source. +- `src/pipeline/uncertainty-promotion.ts:227-247` owns selection and the local + behavior-delta reserve. It must remain behaviorally unchanged. +- `CandidateFindingProvenance` at `src/types.ts:764-772` holds only the primary + promoted source. +- Shared normalizers already exist in `src/util/text-similarity.ts`, but the + broad human-attention grouping rule is intentionally a display-dedup rule, + not predicate identity. Do not use it to group or select promotions. +- Run 57 selected the EXACT_INPUT and EXACT_OUTPUT candidates that should have + remained distinct. The defect is loss of the remaining related signals, not + the identity of the two selected candidates. + +## Commands you will need + +| Purpose | Command | Expected on success | +|---------|---------|---------------------| +| Install | `pnpm install --frozen-lockfile` | exit 0 | +| Focused tests | `pnpm exec vitest run tests/uncertainty-promotion.test.ts tests/verifier.test.ts` | all selected tests pass | +| Checks | `pnpm run check` | exit 0 | +| Full tests | `pnpm test` | all tests pass | +| Build | `pnpm build` | exit 0 | +| Owner live eval | `pnpm dev eval --eval-dir /home/peter/Dev/0xPolygon/codegenie-private-evals/trails-api/49f4645b --no-cache` | case completes; selected count/calls do not increase | +| Cross-case guard | run the same command for `trails-api/0c4d5213` and `trails-api/relay-wc` | cases complete; `should_not_find` guards hold | + +## Scope + +**In scope**: + +- `src/types.ts` — optional bounded related-signal provenance. +- `src/pipeline/uncertainty-promotion.ts` — post-selection association, + decisions, summary counters, and artifact/event data. +- `tests/uncertainty-promotion.test.ts`, `tests/verifier.test.ts`. +- `specs/project/components/review_pipeline.md`, + `specs/project/components/evals.md`, and the plan status row. + +**Out of scope**: + +- Any change to admission, rank, selected-source order, promotion caps, + candidate ids, or the local behavior-delta reserve. +- Grouping sources before selection, consensus bonuses, base-confidence + changes, or treating repeated model outputs as proof. +- New LLM calls, new similarity/tokenization helpers, or changes to + human-attention grouping. +- Merging related-source code excerpts into candidate evidence. The primary + selected source remains the candidate's evidence owner. + +## Git workflow + +- Branch: `fix/promotion-related-signal-provenance` +- Suggested commit: `fix(promotion): retain related lane-limited provenance` +- Do not push or open a PR unless asked. + +## Steps + +### Step 1: Associate related unselected sources after selection + +Do not change `selectPromotionSources`. After selection, compare each +unselected eligible source with selected sources. It may be associated only +when all of these hold: + +1. `riskProfile(...).category` matches. +2. `promotionClass` matches. +3. At least one normalized file and one normalized symbol overlap. +4. The existing normalized attention terms have either an exact normalized + question match, or satisfy the existing broad related-question convention + (`sharedTerms >= 3` or Jaccard `>= 0.24`). These values classify a source as + related context only; they do not assert predicate identity. + +Assign a source to at most one selected candidate. Choose deterministically by +exact-question match, then shared-term count, Jaccard, selected rank, question, +and packet id. Selected sources are never eligible to become another +candidate's related signal. + +If no selected candidate matches, retain today's +`promotion_lane_limited` decision. If matched, emit a decision with +`promoted: false`, reason `represented_as_related_signal`, and the selected +candidate id. Such a source is represented, not lane-limited. + +Do not move this association before selection. Add tests proving selected +sources, order, candidate ids, and model-call count are identical before and +after association. + +**Verify**: +`pnpm exec vitest run tests/uncertainty-promotion.test.ts` -> all tests pass. + +### Step 2: Carry bounded, explicitly non-authoritative provenance + +Extend `CandidateFindingProvenance` with optional fields: + +```ts +relatedSignals?: Array<{ + packetId: string; + sourceKind: "uncertainty" | "follow_up_hint"; + question: string; + files: string[]; + symbols: string[]; +}>; +crossPacketRelatedCount?: number; +``` + +For each selected candidate, attach at most eight associated signals, sorted +deterministically. `crossPacketRelatedCount` counts unique related-signal +packet ids excluding the primary source packet. Do not call it +`independentSupportCount`; packet calls may share context and model behavior. +Do not merge related-signal prose or code into `evidence`, and do not alter +confidence. + +Add summary counters `representedRelatedSignals` and +`unrepresentedLaneLimited`; preserve `promoted` as candidate count. Document +the adjusted `laneLimited` meaning as unselected signals that reached neither +a candidate nor related provenance. + +Add a verifier handoff test proving these fields appear only inside the +untrusted candidate JSON and do not change tool budgets, projected skills, or +the primary provenance question. + +**Verify**: +`pnpm exec vitest run tests/uncertainty-promotion.test.ts tests/verifier.test.ts` +-> all tests pass. + +### Step 3: Add the run-57 regression and cross-case guard + +Create the run-57-shaped fixture: selected EXACT_INPUT and EXACT_OUTPUT +sources, three related unselected EXACT_OUTPUT framings, and the normal cap. +Assert: + +- the same EXACT_INPUT and EXACT_OUTPUT sources are selected in the same order + and keep their existing candidate ids; +- selected sources never become one another's related signals; +- the related EXACT_OUTPUT framings are attached to the selected EXACT_OUTPUT + candidate, bounded and deterministically ordered; +- no rank, confidence, evidence, cap, or model-call count changes; +- unmatched unselected sources remain `promotion_lane_limited`. + +Then update the component docs and run all commands in the table. On live +cross-case validation, any new final-finding `should_not_find` violation, +selected-candidate change, or verifier-call increase is a stop-ship signal. + +**Verify**: all commands exit 0; `git diff --check` is silent. + +## Done criteria + +- [ ] Promotion selection, order, caps, candidate ids, and call count are + unchanged. +- [ ] Related unselected signals reach exactly one selected candidate as + bounded, optional, non-authoritative provenance. +- [ ] Related repetition does not change rank, evidence, or confidence. +- [ ] Decisions distinguish represented signals from truly lane-limited ones. +- [ ] Run-57 and cross-case guards pass. +- [ ] `pnpm run check`, `pnpm test`, and `pnpm build` exit 0. +- [ ] Only Scope files and the plan status row changed. + +## STOP conditions + +Stop and report if association requires changing selection, candidate ids, +caps, prompt versions, confidence, or primary evidence; if a source cannot be +assigned deterministically to at most one selected candidate; if cross-case +guards regress; or if focused tests fail twice after a reasonable correction. + +## Maintenance notes + +Related signals are leads, not votes. If telemetry later proves that repeated +wordings waste slots, design a separate selection experiment against a broad +eval corpus. Do not turn this provenance path into fuzzy pre-cap clustering or +a confidence bonus without that evidence. diff --git a/specs/plans/108-issue-108-verifier-severity-observability.md b/specs/plans/108-issue-108-verifier-severity-observability.md new file mode 100644 index 0000000..2ca3c69 --- /dev/null +++ b/specs/plans/108-issue-108-verifier-severity-observability.md @@ -0,0 +1,193 @@ +# Issue 108: Add a Verifier Severity Rubric and Revision Telemetry + +Status: PENDING +Planned from: trails-api eval `49f4645b`, run 56 severity inflation, 2026-08-04; reduced to measurement-first scope after overfit review +Planned at: commit `1824056` (branch `master`) +Recommended priority: after Issue 106. This shares the Stage-9 prompt cadence +but deliberately adds no deterministic severity cap. + +> **Executor instructions**: Follow this plan step by step. Run every +> verification command and confirm the expected result before moving on. Stop +> on any condition below; do not improvise. Update this plan's row in +> `specs/plans/README.md` when complete. +> +> **Drift check (run first)**: +> `git diff --stat 1824056..HEAD -- src/types.ts src/pipeline/verifier.ts src/pipeline/severity-policy.ts src/skills/prompt-builder.ts tests/verifier.test.ts tests/pipeline-phase5.test.ts tests/shared-utils.test.ts specs/project/components/review_pipeline.md` +> Issue 106 is expected to change verifier and prompt files. Rebase the line +> references and prompt version onto its landed result. STOP if severity no +> longer flows through `revisedFinding` and `applySeverityPolicy` as described. + +## Execution metadata + +- **Priority**: P2 +- **Effort**: S +- **Risk**: LOW +- **Depends on**: + `specs/plans/106-issue-106-verifier-revision-payload-contract.md` +- **Category**: bug +- **Planned at**: commit `1824056`, 2026-08-04 + +## Why this matters + +Run 56 published a bounded rounding issue as high severity after one verifier +raised a low candidate by two levels, while the actual impact was below one +origin base unit. Severity inflation erodes reviewer trust, but one observed +incident is not enough evidence for permanent schema and capping machinery. + +The generic response is a clear Stage-9 rubric plus passive audit data. This +plan changes model guidance and observability only. It does not cap, rewrite, +or otherwise alter submitted severity in deterministic code. + +## Current state + +- `src/pipeline/verifier.ts:880-918` accepts the submitted final-finding + severity and then applies only the existing behavior-change policy. +- `src/pipeline/severity-policy.ts` caps high/critical only for + `intentional_needs_confirmation` and preserves `severityBeforeCap` for the + existing never-hide guarantee. This plan must not change that behavior. +- No verdict metadata or event records original, submitted, and applied + severity together. +- Stage 9 has detailed confidence and false-positive guidance but no compact + magnitude/reach severity rubric. + +## Commands you will need + +| Purpose | Command | Expected on success | +|---------|---------|---------------------| +| Install | `pnpm install --frozen-lockfile` | exit 0 | +| Focused tests | `pnpm exec vitest run tests/verifier.test.ts tests/pipeline-phase5.test.ts tests/shared-utils.test.ts` | all selected tests pass | +| Checks | `pnpm run check` | exit 0 | +| Full tests | `pnpm test` | all tests pass | +| Build | `pnpm build` | exit 0 | +| Owner live eval | run `pnpm dev eval --eval-dir --no-cache` for `49f4645b`, `0c4d5213`, and `relay-wc` under `/home/peter/Dev/0xPolygon/codegenie-private-evals/trails-api` | cases complete; required and `should_not_find` guards hold | + +## Scope + +**In scope**: + +- `src/skills/prompt-builder.ts` — rubric, next Stage-9 version, why ledger. +- `src/pipeline/verifier.ts` — passive revision metadata and telemetry. +- `src/types.ts` — optional persisted audit record. +- `tests/verifier.test.ts`, `tests/pipeline-phase5.test.ts`, and + `tests/shared-utils.test.ts` only for the why-ledger guard. +- `specs/project/components/review_pipeline.md` and the plan status row. + +**Out of scope**: + +- Any deterministic severity cap, new verdict schema field, composer change, + or `severityBeforeCap` change. +- Parsing impact prose, category-specific exceptions, base-severity changes, + confidence policy, or historical artifact rewrites. +- Writing a future cap design before telemetry establishes the problem rate. + +## Git workflow + +- Branch: `fix/verifier-severity-observability` +- Suggested commit: `feat(verifier): add severity rubric and revision telemetry` +- Do not push or open a PR unless asked. + +## Steps + +### Step 1: Add the generic severity rubric + +In `src/skills/prompt-builder.ts`, bump Stage 9 from Issue 106's landed prompt +version to the next version and add one compact rule: + +- low: bounded or localized impact; +- medium: material but limited impact; +- high: broad or serious user/system impact; +- critical: catastrophic impact or a security-boundary compromise; +- severity measures magnitude and reach, not merely whether a correctness + invariant is technically violated; +- a change of more than one level from the input candidate must quantify the + concrete impact bound in verification text. + +Add a why-ledger entry citing the run-56 low-to-high inconsistency without +case-specific files or symbols. Add narrow prompt assertions; do not snapshot +the full prompt. + +**Verify**: +`pnpm exec vitest run tests/pipeline-phase5.test.ts tests/shared-utils.test.ts` +-> all tests pass. + +### Step 2: Audit original, submitted, and applied severity + +In `verifyCandidate`, when a complete submitted `finalFinding` produces a +revised candidate, compare: + +- `original`: `candidate.severity`; +- `submitted`: the model's `finalFinding.severity` before policy; +- `applied`: the revised candidate severity after existing policy. + +Attach this optional verdict record: + +```ts +severityRevision?: { + original: Severity; + submitted: Severity; + applied: Severity; + deltaLevels: number; // signed: submitted rank minus original rank +}; +``` + +Emit `verification_severity_revision` with the same bounded fields plus +candidate id, category, and behavior change. Use info level for +`deltaLevels >= 2` and debug otherwise. Do not include long evidence text and +do not alter applied severity. + +Add tests for a decrease, no change, a one-level increase, a two-level +increase, and an existing behavior-change cap. Assert signed deltas, applied +severity, event level, backward-compatible optional metadata, and no change to +`severityBeforeCap` behavior. + +**Verify**: +`pnpm exec vitest run tests/verifier.test.ts tests/pipeline-phase5.test.ts` +-> all tests pass. + +### Step 3: Document, validate broadly, and finish + +Document the rubric, record, event, and measurement rule below. Run all gates +and the three owner eval cases. Any new required-expectation loss, +`should_not_find` violation, or unexplained publication change attributable to +the prompt is a stop-ship regression; revise the generic wording, never the +eval cases. + +**Verify**: all commands exit 0; `git diff --check` is silent; only Scope files +and the plan status row changed. + +## Measurement rule for any future cap + +After landing, collect at least 20 runs spanning all three named eval cases. +Open a **new plan against the then-current HEAD** only if review finds either: + +- at least three multi-level increases (`deltaLevels >= 2`) whose verification + text does not quantify a commensurate impact; or +- one operator-confirmed severe calibration failure with material user-facing + consequences. + +The future plan must use that corpus to define its policy, schema, and tests. +Do not preserve or implement a speculative cap in this plan. + +## Done criteria + +- [ ] Stage 9 has the generic magnitude/reach rubric and why-ledger entry. +- [ ] Complete final-finding submissions persist and emit original, + submitted, applied, and signed-delta severity data. +- [ ] Deterministic applied-severity behavior is unchanged. +- [ ] Decrease, unchanged, one-level, multi-level, and behavior-cap tests pass. +- [ ] All three cross-case eval guards pass. +- [ ] `pnpm run check`, `pnpm test`, and `pnpm build` exit 0. +- [ ] Only Scope files and the plan status row changed. + +## STOP conditions + +Stop if Issue 106 has not landed; if audit metadata requires changing applied +severity; if the existing behavior-change guarantee regresses; if cross-case +guards regress; or if focused tests fail twice after a reasonable correction. + +## Maintenance notes + +The telemetry is the policy input, not a pretext for a cap. Repeated model +severity changes are hypotheses until source evidence and human review show a +real calibration failure. If the measurement rule fires, write a fresh, +self-contained plan rather than extending this one in place. diff --git a/specs/plans/109-issue-109-verified-low-confidence-publication.md b/specs/plans/109-issue-109-verified-low-confidence-publication.md new file mode 100644 index 0000000..4db6a41 --- /dev/null +++ b/specs/plans/109-issue-109-verified-low-confidence-publication.md @@ -0,0 +1,191 @@ +# Issue 109: Restore Summary-Only Publication for Verified Low-Confidence Deltas + +Status: PENDING +Planned from: trails-api eval `49f4645b`, runs 52/55/57 and earlier hatch-published runs, 2026-08-04 +Planned at: commit `1824056` (branch `master`) +Recommended priority: immediately after Issue 106. Issue 106 improves +calibration; this plan preserves visibility when a valid finding remains low. + +> **Executor instructions**: Follow this plan step by step. Run every +> verification command and confirm the expected result before moving on. Stop +> on any condition below; do not improvise. Update this plan's row in +> `specs/plans/README.md` when complete. +> +> **Drift check (run first)**: +> `git diff --stat 1824056..HEAD -- src/pipeline/composer.ts tests/pipeline-phase5.test.ts specs/project/components/review_pipeline.md` +> STOP if `applyCaps`, `withholdRepresentativeAnchor`, final representative +> ids, or the low-confidence hatch differ semantically from Current state. + +## Execution metadata + +- **Priority**: P1 +- **Effort**: M +- **Risk**: MED +- **Depends on**: + `specs/plans/106-issue-106-verifier-revision-payload-contract.md` +- **Category**: bug +- **Planned at**: commit `1824056`, 2026-08-04 + +## Why this matters + +The low-confidence escape hatch is intended to publish verifier-confirmed, +evidence-backed behavior deltas. It is unreachable for promotion findings: +their gate-only `backfill_packet_representative` anchor is correctly stripped +before composition, but the hatch then requires an anchor and changed line. +The finding is suppressed instead of routed summary-only. + +Run 55 demonstrates the result: an evidence-backed, medium-risk keep on the +expected behavior delta reached composition at low confidence, lost its +untrusted placement, and became “No credible findings.” Removing an unsafe +inline location must not remove an otherwise qualified summary finding. + +## Current state + +- `withholdRepresentativeAnchor` at `src/pipeline/composer.ts:640-658` removes + gate-only representative anchors and sets `changedLine: false`; this safety + invariant must remain intact. +- `lowConfidencePublishableCandidateIds` currently allows every keep/revise, + without evidence, risk, or completeness checks. +- `isPublishableLowConfidenceBehaviorDelta` requires allowlist membership, + concrete behavior-delta evidence/text, a confirmation path, and a surviving + anchor/changed line. The final condition makes the hatch unreachable after + representative-anchor withholding. +- `FinalFinding.id` is the canonical representative candidate id at the + `applyCaps` boundary; `mergedCandidateIds` may include siblings with different + verdict quality. + +## Commands you will need + +| Purpose | Command | Expected on success | +|---------|---------|---------------------| +| Install | `pnpm install --frozen-lockfile` | exit 0 | +| Focused tests | `pnpm exec vitest run tests/pipeline-phase5.test.ts tests/verifier.test.ts` | all selected tests pass | +| Checks | `pnpm run check` | exit 0 | +| Full tests | `pnpm test` | all tests pass | +| Build | `pnpm build` | exit 0 | +| Owner validation | run `pnpm dev eval --eval-dir --no-cache` for `49f4645b`, `0c4d5213`, and `relay-wc` under `/home/peter/Dev/0xPolygon/codegenie-private-evals/trails-api` | required and `should_not_find` guards hold; no untrusted inline anchor publishes | + +## Scope + +**In scope**: + +- `src/pipeline/composer.ts` — verdict gate and two-outcome hatch. +- `tests/pipeline-phase5.test.ts` — positive, negative, merged-member, and cap + regressions. +- `specs/project/components/review_pipeline.md` and the plan status row. + +**Out of scope**: + +- Publishing representative anchors inline; changing confidence/severity + thresholds or caps; weakening concrete-text/confirmation requirements; + verifier, promotion, or human-attention behavior; private eval edits. + +## Git workflow + +- Branch: `fix/verified-low-confidence-publication` +- Suggested commit: `fix(composer): publish verified anchorless deltas summary-only` +- Do not push or open a PR unless asked. + +## Steps + +### Step 1: Make allowlist membership carry verdict quality + +A candidate id qualifies only when its verdict is keep/revise, +`requiredEvidencePresent === true`, `falsePositiveRisk !== "high"`, and +`verificationIncomplete !== true`. Test each rejection condition and valid +keep/revise controls. + +Qualification is representative-local at publication: the final +`finding.id`, not any arbitrary `mergedCandidateId`, must be allowlisted. A +non-representative sibling cannot lend hatch eligibility; a non-qualifying +sibling cannot revoke an otherwise qualifying representative. + +Add a two-member matrix covering both directions. STOP if `finding.id` is not +the canonical representative at `applyCaps`; that would invalidate this rule. + +**Verify**: +`pnpm exec vitest run tests/pipeline-phase5.test.ts` -> all matrix cases pass. + +### Step 2: Split the hatch into anchored and anchorless outcomes + +Replace the boolean helper with `publish | publish_summary_only | suppress`. +Keep every existing requirement except the anchor check: + +- low confidence; +- qualifying representative id; +- pre-cap publication not already suppressed; +- behavior-delta category; +- concrete changed code, non-empty related code, failure mode, + why-this-matters, and confirmation path. + +Return `publish` when a trusted model or merged-recovered anchor survives with +`changedLine: true`. Return `publish_summary_only` when no anchor survives. +Any inconsistent anchor/changed-line shape or failed requirement suppresses. + +`toFinalFinding` normally derives `changedLine` from anchor presence, so the +inconsistent shape is not reachable through ordinary composition. Keep the +defensive suppress branch because the helper accepts a structural +`FinalFinding`, but do not export private `applyCaps` or add an artificial +pipeline fixture solely to exercise that branch. The anchored, anchorless, and +failed-requirement integration controls are the required tests. + +In `applyCaps`, force the anchorless outcome to `summary-only`, never inline. +Record downgrade reason `low-confidence-anchorless` when applicable. Preserve +the ordinary `maxFindings` report cap and all high/critical guarantee behavior. +Extend `low_confidence_verified_delta_published` with `anchorless` and applied +publication; keep the event name. + +**Verify**: +`pnpm exec vitest run tests/pipeline-phase5.test.ts` -> existing anchored and +broad-suppression controls plus both new outcomes pass. + +### Step 3: Prove the run-55 path and negative boundaries + +Build a promotion-shaped composer input with a valid gate-only representative +anchor, low confidence, concrete logic-bug evidence, and a qualifying medium- +risk keep. Let composition itself withhold the anchor. Assert: + +- anchor withholding still fires; +- the finding publishes summary-only with no inline location; +- the hatch event records `anchorless: true`; +- selection has no confidence-threshold suppression; +- the review is not the no-findings fallback. + +Negative controls: vague failure mode, evidence-absent verdict, high-risk +verdict, incomplete verdict, reject, pretrim suppression, non-qualifying +representative with qualifying sibling, and report-cap overflow. Preserve the +existing trusted-anchor inline control. + +Document the contract, run all gates, then run at least 10 repeats of +`49f4645b` plus both quiet cross-cases. A qualifying representative must never +end with `confidence-threshold`; no representative anchor may publish inline; +and any new `should_not_find` violation is stop-ship evidence that the verdict +gate is too loose, not a reason to tune the eval. + +**Verify**: all commands exit 0; `git diff --check` is silent; only Scope files +and the plan status row changed. + +## Done criteria + +- [ ] Allowlist membership requires evidence-backed, non-high-risk, complete + keep/revise. +- [ ] Hatch eligibility belongs to the final representative id. +- [ ] Trusted anchored findings retain current publication; qualified + anchorless findings publish summary-only and never inline. +- [ ] Anchor withholding, negative boundaries, merged-member matrix, and + report cap remain correct. +- [ ] Focused, full, build, repeat, and cross-case guards pass. +- [ ] Only Scope files and the plan status row changed. + +## STOP conditions + +Stop if the fix requires publishing a representative anchor, changing a cap +or threshold, weakening concrete-text checks, touching verifier/promotion/note +code, or if `finding.id` is not the representative. Also stop on cross-case +regression or two focused-test failures after a reasonable correction. + +## Maintenance notes + +Issue 106 reduces how often this path is needed; this plan guarantees the +remaining floor. If summary-only noise rises, tighten the structured verdict +gate—not the placement invariant and not the eval cases. diff --git a/specs/plans/110-issue-110-publication-aware-note-adjudication.md b/specs/plans/110-issue-110-publication-aware-note-adjudication.md new file mode 100644 index 0000000..ded048b --- /dev/null +++ b/specs/plans/110-issue-110-publication-aware-note-adjudication.md @@ -0,0 +1,240 @@ +# Issue 110: Make Note Fallback Publication-Aware and Score Rendered Notes + +Status: PENDING +Planned from: trails-api eval `49f4645b`, runs 52/55/57 and the run-52 `surfacedAsNote` misreport, 2026-08-04; reduced after overfit review +Planned at: commit `1824056` (branch `master`) +Recommended priority: after Issues 106 and 109. This closes the remaining +visibility and measurement gaps without adding member-snapshot machinery. + +> **Executor instructions**: Follow this plan step by step. Run every +> verification command and confirm the expected result before moving on. Stop +> on any condition below; do not improvise. Update this plan's row in +> `specs/plans/README.md` when complete. +> +> **Drift check (run first)**: +> `git diff --stat 1824056..HEAD -- src/types.ts src/pipeline/composer.ts src/pipeline/human-attention.ts src/evals/eval-artifacts.ts src/evals/eval-scoring.ts tests/human-attention-adjudication.test.ts tests/pipeline-phase5.test.ts tests/evals.test.ts specs/project/components/review_pipeline.md specs/project/components/evals.md` +> Issue 109 changes the composer hatch, not these source regions. STOP if +> output selection no longer receives already-published findings or if the +> resolution/matching flow differs semantically from Current state. + +## Execution metadata + +- **Priority**: P1 +- **Effort**: M +- **Risk**: MED +- **Depends on**: + `specs/plans/106-issue-106-verifier-revision-payload-contract.md`, + `specs/plans/109-issue-109-verified-low-confidence-publication.md` +- **Category**: bug +- **Planned at**: commit `1824056`, 2026-08-04 + +## Why this matters + +Output note suppression assumes every verifier keep/revise becomes visible as +a finding. Runs 52, 55, and 57 disproved that assumption: composition +suppressed the finding after its matching note group had already been removed, +and the review rendered “Everything looks good.” The scorer then compounded +the bug by treating an internal group as a user-visible note. + +The generic invariant is simple: an unpublished keep/revise cannot suppress +its fallback note, and fallback notes must outrank ordinary notes within the +existing output cap. Separately, eval scoring must inspect `outputNotes`, not +internal groups. Evidence-backed rejects remain authoritative. + +## Current state + +- `src/pipeline/composer.ts:220-228` passes only non-suppressed findings but all + verification resolutions into `selectHumanAttentionForOutput`. +- `src/pipeline/human-attention.ts:232-243` suppresses available groups using + those unfiltered resolutions, then calls `selectHumanAttentionGroups`. +- `selectHumanAttentionGroups` takes the first five ranked groups. Merely + restoring eligibility does not guarantee a fallback group survives that cap. +- `suppressAttentionGroupsResolvedByVerification` suppresses a whole group on + the first matching resolution. This plan leaves reject behavior unchanged; + residual cross-predicate reject suppression is measured for a separate plan. +- `src/evals/eval-artifacts.ts` normalizes artifact `groups`, while the schema + has carried rendered `outputNotes` since version 2. +- `EvalArtifacts` and `EvalLossDetail` live in `src/types.ts`; new scorer fields + must be added there as optional, backward-compatible fields. +- `should_not_find` currently evaluates reported final findings, not notes. + Live overfit validation must therefore inspect rendered notes separately. + +## Commands you will need + +| Purpose | Command | Expected on success | +|---------|---------|---------------------| +| Install | `pnpm install --frozen-lockfile` | exit 0 | +| Focused tests | `pnpm exec vitest run tests/human-attention-adjudication.test.ts tests/pipeline-phase5.test.ts tests/evals.test.ts` | all selected tests pass | +| Checks | `pnpm run check` | exit 0 | +| Full tests | `pnpm test` | all tests pass | +| Build | `pnpm build` | exit 0 | +| Owner live eval | run `pnpm dev eval --eval-dir --no-cache` for `49f4645b`, `0c4d5213`, and `relay-wc` under `/home/peter/Dev/0xPolygon/codegenie-private-evals/trails-api` | cases complete; final-finding guards hold and rendered fallback notes are audited separately | + +## Scope + +**In scope**: + +- `src/pipeline/human-attention.ts` — publication-aware resolution filtering, + fallback-priority selection, bounded telemetry/artifact fields. +- `src/evals/eval-artifacts.ts`, `src/evals/eval-scoring.ts`, `src/types.ts` — + rendered-note parsing and truthful loss diagnostics. +- `tests/human-attention-adjudication.test.ts`, + `tests/pipeline-phase5.test.ts`, `tests/evals.test.ts`. +- `specs/project/components/review_pipeline.md`, + `specs/project/components/evals.md`, and the plan status row. + +**Out of scope**: + +- Note-group merge thresholds, note wording, or raising the five-note cap. +- Member snapshots, truncated/opaque-group behavior, survivor reconstruction, + or changing reject-resolution semantics. +- Composer confidence/publication policy, verifier policy, promotion policy, + or historical artifact rewrites. + +## Git workflow + +- Branch: `fix/publication-aware-note-fallback` +- Suggested commit: `fix(attention): preserve unpublished verified predicates as notes` +- Do not push or open a PR unless asked. + +## Steps + +### Step 1: Filter keep/revise resolutions by actual publication + +Inside `selectHumanAttentionForOutput`, partition resolutions before applying +verification suppression: + +- every `reject` remains active; +- a keep/revise remains active only if its `candidateId` equals a published + finding id or appears in that finding's `mergedCandidateIds`; +- all other keep/revise resolutions become **publication-fallback + resolutions** and do not suppress groups. + +Do not change the pre-composer reject-only suppression path. Add tests proving +an unpublished keep/revise leaves its matching group available, while a +published keep/revise and every reject behave exactly as today. + +**Verify**: +`pnpm exec vitest run tests/human-attention-adjudication.test.ts tests/pipeline-phase5.test.ts` +-> all tests pass. + +### Step 2: Protect fallback groups inside the existing note cap + +Using the existing resolution/group matcher, identify available groups matched +by publication-fallback resolutions after active reject suppression. Select +output groups in two stable classes: + +1. matching publication-fallback groups, in existing rank order; +2. all remaining groups, in existing rank order. + +Take at most the existing `MAX_HUMAN_ATTENTION_NOTES`. Do not raise the cap. +This guarantees that no ordinary note displaces a verifier-kept predicate; +when more fallback groups exist than the cap, render the highest-ranked five +and record the overflow rather than claiming every predicate was shown. + +Extend bounded telemetry/artifacts with fallback group ids/count and +`omittedFallbackCount`. Add regressions for: + +- a run-55-derived **unpublished keep**: low-confidence candidate, + `requiredEvidencePresent: true`, `falsePositiveRisk: "medium"`, and a + deliberately vague `failureMode` below Plan 109's concrete-text threshold. + Assert Plan 109 leaves it unpublished with `confidence-threshold`, then this + plan selects its matching fallback note instead of the no-findings output; +- the fully concrete run-55 control: same evidence-backed medium-risk keep, + but with concrete behavior-delta text satisfying Plan 109. Assert it + publishes summary-only and its matching note is suppressed as redundant; +- published inline and summary-only controls -> matching note stays + suppressed; +- six available groups with the fallback ranked sixth -> fallback is selected + without increasing total notes; +- evidence-backed reject matching the group -> reject still suppresses it; +- more than five fallback groups -> deterministic top five plus overflow. + +Do not use `requiredEvidencePresent: false` for the unpublished-keep fixture: +normal verifier normalization converts that shape to reject, and an active +reject should suppress the note. The fixture must remain a completed keep that +fails publication quality, not verification truth. + +**Verify**: +`pnpm exec vitest run tests/human-attention-adjudication.test.ts tests/pipeline-phase5.test.ts` +-> all cases pass. + +### Step 3: Score only rendered notes + +Read the human-attention artifact once in `src/evals/eval-artifacts.ts`. +Preserve today's internal-group normalization as `humanAttentionNotes` and add +optional `humanAttentionOutputNotes` from `outputNotes`; leave it undefined for +old artifacts that lack the field. + +In `src/types.ts`, add optional +`EvalArtifacts.humanAttentionOutputNotes` and +`EvalLossDetail.noteGroupExisted`. In scoring: + +- set `surfacedAsNote` only from `humanAttentionOutputNotes`; +- fall back to internal groups only when the output field is absent, preserving + old-artifact replay behavior; +- set `noteGroupExisted` from the internal-group match; +- keep aggregate `noteSurfaced` tied to the corrected field. + +Test an internal-only run-52 shape, a rendered note, an explicit empty +`outputNotes` array, and a legacy artifact without the field. + +**Verify**: +`pnpm exec vitest run tests/evals.test.ts` -> all cases pass. + +### Step 4: Document and validate the visibility/noise tradeoff + +Document publication-aware suppression, fallback priority, overflow telemetry, +and truthful scoring. Run all gates and the three owner eval cases. + +`should_not_find` does not inspect notes. For `0c4d5213` and `relay-wc`, also +inspect `human-attention-notes.json.outputNotes` using the same path/text +matching semantics as `expectationMatchesNote`; no banned predicate may +resurface only as a note. Every newly rendered note must trace to a dropped +keep/revise candidate id in fallback telemetry. Do not weaken the eval cases +to accommodate note noise. + +**Verify**: all commands exit 0; `git diff --check` is silent; only Scope files +and the plan status row changed. + +## Measurement rule for member-level reject adjudication + +After Issues 106, 109, and this plan land, collect at least 20 runs spanning +`49f4645b` and at least one other case. Open a **new plan against current HEAD** +only if at least two runs lose a required predicate because a reject resolution +for one raw note suppresses a merged group containing a different predicate. + +That future design must adjudicate raw notes before regrouping survivors, or +use an equivalent lossless raw-note lookup. It must never suppress an opaque or +truncated group wholesale. If no residual failures occur, do nothing. + +## Done criteria + +- [ ] Unpublished keep/revise resolutions cannot suppress their note groups; + published keep/revise and reject behavior remain intact. +- [ ] The vague unpublished-keep fixture renders a fallback note, while the + fully concrete Plan-109-qualified run-55 control publishes summary-only + and suppresses its redundant note. +- [ ] Fallback groups outrank ordinary notes inside the unchanged five-note + cap, with deterministic overflow accounting. +- [ ] `surfacedAsNote` reflects rendered output; `noteGroupExisted` remains a + diagnostic only; legacy artifacts score as before. +- [ ] Cross-case final-finding guards pass and rendered notes contain no banned + predicates. +- [ ] `pnpm run check`, `pnpm test`, and `pnpm build` exit 0. +- [ ] Only Scope files and the plan status row changed. + +## STOP conditions + +Stop if publication awareness requires moving resolution-index construction; +if reject behavior changes; if fallback priority requires raising the cap; if +old artifacts change score beyond the documented correction; if cross-case +notes leak banned predicates; or if focused tests fail twice after a reasonable +correction. + +## Maintenance notes + +Fallback priority is a visibility invariant, not recall credit: the finding +still missed. If residual cross-predicate reject suppression is measured, write +the raw-note-first follow-up then; do not reintroduce member snapshots or +fail-closed opaque groups. diff --git a/specs/plans/README.md b/specs/plans/README.md index cd876cc..6edca87 100644 --- a/specs/plans/README.md +++ b/specs/plans/README.md @@ -106,3 +106,17 @@ This directory tracks implementation plans for confirmed improvements. Status va | 100 | COMPLETE | Issue 100: Short Hunk IDs — Planner Coverage Survival and Dispatch Resilience | | 101 | IN PROGRESS (paid semantic A/B pending) | Issue 101: Exact Skill Provenance and Evidence-Gated Bundled-Skill Revision | | 102 | PENDING | Issue 102: Same-File Packet Packing | +| 104 | PENDING | Issue 104: Delegate Mode — Run the Harness Without a Provider API Key | +| 105 | PENDING | Issue 105: Competitive Benchmark — Measuring codegenie Against Other Review Harnesses | +| 106 | COMPLETE | Issue 106: Enforce Meaningful Verifier Revisions and Calibrate Confidence | +| 107 | COMPLETE | Issue 107: Carry Related Promotion Signals Without Changing Selection | +| 108 | COMPLETE | Issue 108: Add a Verifier Severity Rubric and Revision Telemetry | +| 109 | COMPLETE | Issue 109: Restore Summary-Only Publication for Verified Low-Confidence Deltas | +| 110 | COMPLETE | Issue 110: Make Note Fallback Publication-Aware and Score Rendered Notes | + +## Recommended order for 106-110 + +Land the terminal visibility chain first: **106 -> 109 -> 110**. Plans 107 and +108 are lower-risk supporting improvements; both follow 106 and may land after +the visibility chain. Plan 108 must rebase its Stage-9 prompt version onto the +landed 106 version. diff --git a/specs/project/components/evals.md b/specs/project/components/evals.md index 846ddc4..447bf92 100644 --- a/specs/project/components/evals.md +++ b/specs/project/components/evals.md @@ -540,7 +540,7 @@ Scoring and attribution read the following artifacts. The expectation-bearing fo | Artifact | Evals reads | Writer (owner) | | --- | --- | --- | -| `candidate-findings.json` | `CandidateFinding[]`: every structurally valid candidate from Stage 7, with `id`, matching fields, `producedBy`, `clusterId?`, `duplicateOf?` | `components/review_pipeline.md` | +| `candidate-findings.json` | `CandidateFinding[]`: every structurally valid candidate from Stage 7 plus bounded Stage-9 uncertainty promotions, with `id`, matching fields, `producedBy`, `clusterId?`, `duplicateOf?`; promoted provenance may carry up to eight explicitly non-authoritative `relatedSignals` and `crossPacketRelatedCount`, which do not change expectation matching, confidence, or evidence | `components/review_pipeline.md` | | `verification.json` | Per candidate id: either a pre-verification-gate record `{ candidateId, gate: "suppressed", gateReason }` or `{ candidateId, gate: "passed", verdict: VerificationVerdict }`; revised findings carry `verdict.finalFinding`. Pre-clustered duplicate members carry no record of their own — the reader resolves them through the candidate's `duplicateOf` chain to the representative's record | `components/review_pipeline.md` | | `final-selection.json` | Per verified-kept finding: `{ findingId, decision: "published" \| "merged" \| "suppressed", reason, mergedIntoFingerprint? }` — the telemetry requirement "final-selection decisions and reasons for omitted verified findings" in artifact form | `components/review_pipeline.md` | | `final-findings.json` | `FinalFinding[]` including suppressed entries, with `publication`, `fingerprint`, `mergedCandidateIds` | `components/review_pipeline.md` | @@ -621,12 +621,14 @@ For a missed expectation `E`: 1. **Suppressed final?** Match `E` against final findings with `publication: "suppressed"`. On match → `lost-at-composition`. `subReason` comes from `final-selection.json` (`report-cap`, `soft-comment-cap`, `confidence-threshold`, `severity-threshold`, `composer-pre-trim`, `composer-suppressed`) or the finding's selection record; omissions caused by confidence/severity thresholds are this label (the functional spec's "lost at composition — deduped, merged, or capped" maps here). If `final-selection.json` is absent or has no record, the label still applies with `subReason: "unrecorded"` plus a warning note. 2. **Merged into a non-matching final?** Match `E` against the verified-kept set (verdicts `keep`/`revise`, post-revision values). For each match `c`, check whether any final finding covers `c` (`c.id ∈ F.mergedCandidateIds` or `c.id === F.id`). If a covering final exists but did not satisfy `E` (otherwise the expectation would have passed) → `lost-at-composition` with `subReason: "merged-deduped-away"`; the detail names the absorbing final's fingerprint, title, and which fields of `E` it fails. A verified-kept match with no covering final and no selection record is also `lost-at-composition`, `subReason: "unrecorded"`. -3. **Lost at verification?** Match `E` against the remaining candidates. A match whose resolved verification outcome (following `duplicateOf` to the cluster representative) is a verifier `reject`, a pre-verification-gate suppression, `verificationIncomplete`, or absent entirely → `lost-at-verification`. `subReason` distinguishes `verifier-rejected` (detail carries the verifier's `reason` and `falsePositiveRisk`), `low-confidence-suppressed`, `invalid-anchor`, `no-evidence`, `no-failure-mode`, `verification-incomplete`, `budget-exhausted`, and `unrecorded`. +3. **Lost at verification?** Match `E` against the remaining candidates. A match whose resolved verification outcome (following `duplicateOf` to the cluster representative) is a verifier `reject`, a pre-verification-gate suppression, `verificationIncomplete`, or absent entirely → `lost-at-verification`. `subReason` distinguishes `verifier-rejected` (detail carries the verifier's `reason` and `falsePositiveRisk`), `low-confidence-suppressed`, `invalid-anchor`, `no-evidence`, `no-failure-mode`, `empty-revision` (an unrecovered `revise_without_revision_payload`), `verification-incomplete` (all other incomplete causes), `budget-exhausted`, and `unrecorded`. 4. **Partial match?** Match `E`'s `path` field alone (exact-or-glob) against all candidates and all final findings. If any instance is in the right file but fails other fields → `partial-match`. The detail carries the closest instances (fewest failed fields; ties by artifact order) with full per-field mismatch records — e.g. `category: expected security, actual logic_bug`, `lineRange: expected 80–90, actual 120`. An expectation without a `path` field skips this rung. 5. **Otherwise** → `missed-before-candidate-generation`. Hint detail, applied at every rung: hint events (follow-up hints and structured uncertainties) from `events.jsonl` are searched with **diagnostic-grade reduced matching** — hints carry no severity/category/anchor, so only these fields participate: `path` matches any entry of the hint's `files` (exact-or-glob); `titlePattern`/`failureModePattern` test deterministically over the hint's text and symbols (the exact reduction is pinned by the implementation); `lineRange`, `category`, and `severityAtLeast` are ignored. Matches are recorded in `EvalLossDetail.matchingHints` with the hint's confidence — supporting detail showing a reviewer articulated the question (most useful on `missed-before-candidate-generation` losses), never a pass and never a label. An expectation whose only present fields are unmatchable against hints (e.g. category + severity only) records no hint detail. Reduced matching is acceptable here because hint detail is forensic. +Rendered-note diagnostics use the same reduced path/text predicate but read `human-attention-notes.json.outputNotes`, not internal groups. A rendered match sets `EvalLossDetail.surfacedAsNote` and contributes to repeat `noteSurfaced`; a match that existed only in `groups` sets diagnostic-only `noteGroupExisted`. Artifacts predating `outputNotes` fall back to internal groups for replay compatibility, while an explicit empty `outputNotes` array truthfully means no note was shown. + #### Missed-Before-Candidate-Generation Sub-Reasons When the walk bottoms out at rung 5, attribution sub-diagnoses why nothing was ever produced, using enrichment artifacts when present: diff --git a/specs/project/components/review_pipeline.md b/specs/project/components/review_pipeline.md index ff58b34..54853aa 100644 --- a/specs/project/components/review_pipeline.md +++ b/specs/project/components/review_pipeline.md @@ -598,6 +598,8 @@ Behavior: Stage 9 is the false-positive control. Its candidate pool is every validated candidate from Stage 7 — v1 findings are always packet-produced; static signals are prompt hints only and never enter the pool as findings of their own. +Before verifier gating, the bounded uncertainty-promotion lane admits, ranks, and selects unresolved concrete predicates without extra model calls. Selection, the local-behavior-delta reserve, candidate order/ids, confidence, and primary evidence are independent of related-signal handling. After selection only, an eligible unselected source may be attached to exactly one selected candidate as non-authoritative `provenance.relatedSignals` when category and promotion class match, normalized file and symbol scopes overlap, and normalized questions are exact or meet the display-related convention (at least three shared attention terms or Jaccard at least 0.24). The deterministic association chooses exact question, shared terms, similarity, selected rank, question, then packet id; each candidate carries at most eight signals and `crossPacketRelatedCount` counts unique non-primary packets. Selected sources never become one another's related signal, and related signals never alter evidence or confidence. `uncertainty-promotion.json`/telemetry distinguish `representedRelatedSignals` from `unrepresentedLaneLimited`; `laneLimited` means an unselected signal reached neither a candidate nor bounded related provenance. + Deterministic pre-verification gates, in order, each recording the candidate id and gate decision: 1. Schema validity — defensive re-check of required `CandidateFinding` fields. @@ -611,16 +613,20 @@ Verifier dispatch: one candidate per call through the worker runner (stage 9, `r Verdict handling (`VerificationVerdict` per candidate): -- `keep` → the candidate (or `finalFinding` when provided) enters the verified set unchanged in identity. +- `keep` → a bare keep means confidence, severity, evidence, wording, and placement are publishable unchanged. Historical/provider keep verdicts carrying `finalFinding` or `revisedAnchor` are canonicalized to `revise`, with `verification_keep_payload_canonicalized` telemetry naming the payload kinds. - `reject` → recorded with reason; excluded. -- `revise` → `finalFinding` must preserve the candidate id (contract); a `revisedAnchor` is accepted only if it validates against a changed diff line, otherwise the original validated anchor is preserved; a real-but-unanchorable issue keeps no anchor and proceeds as summary-only. Severity/confidence/wording/fix/test narrowing is accepted as submitted; lineage is preserved. +- `revise` → the provider-safe flat submit schema keeps both compatibility payloads optional, then runtime semantic validation requires a non-empty structured change: either `finalFinding` or `revisedAnchor`. `finalFinding` must preserve the candidate id (contract); a `revisedAnchor` is accepted only if it validates against a changed diff line, otherwise the original validated anchor is preserved; a real-but-unanchorable issue may revise through a complete `finalFinding` and proceed as summary-only. Severity/confidence/wording/fix/test narrowing is accepted as submitted; lineage is preserved. An empty revise is recorded as incomplete with `verification_semantic_invalid`, never as a verifier rejection. - Verdicts referencing unknown candidate ids are discarded with telemetry. -Failure rules: authentication or provider-wide failures fail the run or mark the review incomplete (fatal per the global policy). Individual schema/parse failures get the one repair attempt; candidates still unverified are marked `verificationIncomplete: true`, suppressed from publication by default, and counted into `RunCoverageStatus.verificationIncompleteCount`. When `review.verify === false` (explicit configuration only), gates 1-6 still run, the LLM verifier is skipped, gate-surviving candidates pass through as the verified set, and the coverage summary discloses that verification was skipped. +Failure rules: authentication or provider-wide failures fail the run or mark the review incomplete (fatal per the global policy). Individual schema/parse failures get the one repair attempt. The stateless repair prompt carries a bounded, untrusted-data-fenced projection of candidate semantics and evidence; repository-tool results from the discarded response are explicitly not represented. An empty primary `{}` is classified as `empty_submit_object`; even a schema-valid repair is discarded and persisted incomplete because it cannot preserve the primary adjudication state. Empty revise remains a runtime semantic failure (`revise_without_revision_payload`). `verification_primary_submit_accepted` is emitted only for a non-empty, semantically complete, unrepaired primary verdict and carries submit schema version 3, so provider smoke acceptance cannot be inferred from HTTP acceptance or repair success alone. Candidates still unverified are marked `verificationIncomplete: true`, suppressed from publication by default, and counted into `RunCoverageStatus.verificationIncompleteCount`. When `review.verify === false` (explicit configuration only), gates 1-6 still run, the LLM verifier is skipped, gate-surviving candidates pass through as the verified set, and the coverage summary discloses that verification was skipped. + +The Stage-9 `p9.8` prompt makes confidence follow decisive verified evidence rather than inherited generation confidence or pressure from a secondary lookup. A confirmed low-confidence promoted predicate returns a complete calibrated `finalFinding`; unresolved secondary checks do not by themselves hold confidence low, while speculative reachability, ambiguous intent, or weak path matching still do. Its severity rubric is impact-based: low is bounded or localized, medium is material but limited, high is broad or serious user/system impact, and critical is catastrophic impact or a security-boundary compromise. Severity measures magnitude and reach rather than the mere existence of a technical invariant violation; a verifier changing severity by more than one level must quantify the concrete impact bound in the revised verification text. + +Every complete verifier `finalFinding` revision passively records `severityRevision: { original, submitted, applied, deltaLevels }` on the persisted verdict. `deltaLevels` is signed from the input candidate to the submitted severity, while `applied` reflects the existing behavior-change policy; the audit record never changes severity. The bounded `verification_severity_revision` event carries the same fields plus candidate id, category, and behavior change, at info level for increases of two or more levels and debug otherwise. Future deterministic calibration policy requires a fresh plan based on at least 20 runs spanning the `49f4645b`, `0c4d5213`, and `relay-wc` eval cases, and only after either three unquantified multi-level increases or one operator-confirmed severe user-facing calibration failure. BehaviorChange severity contract (plan 82): findings and verdicts may carry `behaviorChange: "accidental_regression" | "intentional_needs_confirmation" | "specified_change" | "unknown"`. Only `intentional_needs_confirmation` caps severity (critical/high → medium) — a deliberate-change callout reads differently from a regression. `"unknown"` and an omitted field are equivalent and never demote (punishing honest uncertainty taught models to omit the field). When the cap fires, the pre-cap severity is preserved as `severityBeforeCap`, and every never-hide-critical/high guarantee (composer pre-trim, soft comment cap, report cap) consults `max(severity, severityBeforeCap)` so a capped critical cannot be silently suppressed at composition. -Telemetry per candidate: pre-gate decision, verifier prompt size, tool calls, token usage, runtime, verdict, revision details, rejection reason, incomplete reason, plus `verifier_skill_provenance` with requested/resolved/dropped/unknown/language-incompatible ids and no repository content. `verification.json` persists one record per candidate: gate-rejected/suppressed candidates record `{ candidateId, gate: "suppressed", gateReason }`, and verified candidates record `{ candidateId, gate: "passed", verdict: VerificationVerdict }` (revised findings carry `verdict.finalFinding`). Pre-clustered duplicate members carry no record of their own — readers resolve them through `duplicateOf` to the representative's verdict. This is the reader contract consumed by `components/evals.md`. Stage 9 does not decide the final review shape. +Telemetry per candidate: pre-gate decision, verifier prompt size, tool calls, token usage, runtime, verdict, revision details (including `verification_severity_revision`), rejection reason, incomplete reason, plus `verifier_skill_provenance` with requested/resolved/dropped/unknown/language-incompatible ids and no repository content. `verification.json` persists one record per candidate: gate-rejected/suppressed candidates record `{ candidateId, gate: "suppressed", gateReason }`, and verified candidates record `{ candidateId, gate: "passed", verdict: VerificationVerdict }` (revised findings carry `verdict.finalFinding` and the optional severity-revision audit record). Pre-clustered duplicate members carry no record of their own — readers resolve them through `duplicateOf` to the representative's verdict. This is the reader contract consumed by `components/evals.md`. Stage 9 does not decide the final review shape. ### Run Coverage Aggregation @@ -659,10 +665,10 @@ Output validation and deterministic post-processing: 2. Re-insert any verified finding the composer omitted, as its own group with template wording (`composer_omitted_finding`); omission is not a suppression decision the model may make. 3. Merge `mergedCandidateIds` with Stage 9 pre-cluster members (`duplicateOf` lineage) so `FinalFinding.mergedCandidateIds` is complete; preserve lineage to packets, lenses, and evidence via `producedBy`. 4. Re-validate anchors. Inline publication requires a valid changed-line anchor; otherwise the finding is `summary-only`. When merged findings offer multiple valid anchors, prefer the representative's (clearest changed-line anchor by the pre-grouping representative rule). -5. Apply thresholds: severity below `review.minSeverity` (when set) → `suppressed`; confidence below `review.minConfidence` → `suppressed`; confidence below `review.minInlineConfidence` → `summary-only`. +5. Apply thresholds: severity below `review.minSeverity` (when set) → `suppressed`; confidence below `review.minConfidence` → `suppressed`; confidence below `review.minInlineConfidence` → `summary-only`. The low-confidence behavior-delta hatch is representative-local: the final finding's own id must have a complete keep/revise verdict with required evidence and non-high residual risk, plus concrete changed code, related code, failure mode, impact, and a confirmation path. A qualifying trusted anchored finding remains publishable at its pre-cap mode; if the gate-only representative anchor was withheld and no trusted merged anchor survives, it publishes `summary-only` with downgrade reason `low-confidence-anchorless`. A merged sibling can neither lend nor revoke this eligibility. The hatch does not bypass pre-trim or the report cap and never publishes a representative anchor inline. 6. Rank: a deterministic total order over severity, confidence, evidence strength, and actionability — the exact measures and tiebreakers are an implementation detail within that contract. The composer's ordering is advisory input; this deterministic rank is final. 7. Enforce caps: at most `review.softCommentCap` inline findings — beyond the cap, medium/low-severity findings move to `summary-only`; verified critical and high findings are never displaced or hidden by the cap. At most `review.maxFindings` total reported findings — beyond it, lowest-ranked non-critical/high findings become `suppressed` with disclosure. Neither cap ever suppresses verified critical/high findings. -8. Needs-human-attention notes: deterministically assemble every medium/high-confidence follow-up hint into `ReviewResult.needsHumanAttention` (`{ question, files, symbols, reason, confidence }` records), deduplicated by trimmed question. Renderers consume the field for the report's "needs human attention" section, and in posting mode `postingPlan.reviewBody` embeds the notes as well. The composer's summary wording may reference them, but the notes are code-assembled — they never become findings and are never silently dropped from the report. (Existing-PR-thread overlap recording is deferred to Future Considerations — see architecture.md.) +8. Needs-human-attention notes: deterministically assemble every medium/high-confidence follow-up hint into `ReviewResult.needsHumanAttention` (`{ question, files, symbols, reason, confidence }` records), deduplicated by trimmed question. Verification suppression is publication-aware: every evidence-backed reject remains authoritative, while keep/revise suppresses a matching group only when its candidate id is the published final representative or one of that final's merged candidate ids. Unpublished keep/revise matches become publication fallbacks and rank ahead of ordinary notes inside the unchanged five-note cap. When fallback groups overflow, the existing rank chooses the top five and the artifact/`human_attention_publication_fallback` telemetry record bounded group/candidate ids, total fallback count, and `omittedFallbackCount`. Renderers consume the selected notes for the report's "needs human attention" section, and in posting mode `postingPlan.reviewBody` embeds them as well. The composer's summary wording may reference them, but the notes are code-assembled — they never become findings and are never silently dropped from the report. (Existing-PR-thread overlap recording is deferred to Future Considerations — see architecture.md.) 9. Compute `fingerprint` on each `FinalFinding` (the Stage 11 duplicate-avoidance identity) and assemble `ReviewResult`: `summary` (composer wording, or template on fallback), `coverage` (the aggregated `RunCoverageStatus`, including partial disclosure), `findings` (publication `inline` only — suppressed findings never appear in `ReviewResult`; they are recorded solely in `final-findings.json` and `final-selection.json`), `summaryOnlyFindings`, `needsHumanAttention` (the step 8 notes), `noFindings` when nothing is publishable, and `postingPlan` only when `--post-github-comments` was passed (`inline` entries for `publication: "inline"` findings with validated anchors; `reviewBody` containing the summary, counts, summary-only findings, needs-human-attention notes, and partial-coverage disclosure). Posting itself is Stage 11 (`components/repository_and_github.md`). Composer terminal failure (after one repair retry, non-auth): deterministic fallback composition — fingerprint-level grouping only (steps 1-2 without semantic merging), template wording per finding (title, failure mode, evidence, why it matters, suggested fix/test), ranking, caps, and needs-human-attention notes per steps 4-8, and a coverage `reasons` disclosure note that semantic composition was skipped. The fallback never loses verified findings. @@ -772,6 +778,9 @@ Stage 9: - `verify_gates_order_and_reasons`: candidates missing evidence or failure mode are gate-rejected without verifier calls; a low-confidence medium-severity candidate is suppressed while a low-confidence critical-severity candidate proceeds to verification; an invalid anchor is stripped to summary-only rather than rejected. - `verify_precluster_representative_only`: three near-identical candidates produce one verifier call; members carry `clusterId`/`duplicateOf`; the verdict applies to the cluster and lineage survives to Stage 10. - `verify_verdict_handling`: keep/reject/revise are applied; a revised finding preserves the candidate id; a `revisedAnchor` on an unchanged line is discarded in favor of the original anchor. +- `verify_revision_payload_contract`: the provider-facing verdict schema has one object root; runtime semantics persist empty revise as incomplete, legacy keep-with-payload canonicalizes to revise, and a bare keep remains unchanged. +- `verify_empty_primary_submit_fails_closed`: a run-60-shaped evidence-backed candidate whose primary verdict arguments are `{}` gets a bounded evidence-bearing stateless repair prompt, distinct empty-submit telemetry, and an incomplete outcome even when repair returns a schema-valid reject. +- `promotion_related_signal_provenance`: run-57-shaped exact-input/output selection retains byte-identical selected ids/order, while related unselected output framings attach only to the output candidate, cap at eight, leave evidence/confidence unchanged, and unmatched or overflow signals remain lane-limited. - `verify_repair_then_incomplete`: a schema-invalid verdict gets one repair; persistent failure marks `verificationIncomplete`, suppresses the candidate, and increments the coverage count. - `verify_disabled_by_config`: with `review.verify = false`, gates still run, no verifier calls occur, gate-survivors pass through, and the coverage summary discloses skipped verification. @@ -781,6 +790,7 @@ Stage 10: - `compose_pretrim_over_40`: 45 verified findings trim to 40 by rank; critical/high are never trimmed; trimmed findings appear as suppressed finals with disclosure. - `compose_invented_and_omitted_findings`: a composed finding referencing unknown ids is dropped; a verified finding the composer omitted is re-inserted with template wording. - `compose_caps_protect_critical_high`: 10 inline-eligible findings with `softCommentCap = 7` move the lowest-ranked medium findings to summary-only while all critical/high stay inline; `maxFindings` suppresses only non-critical/high overflow. +- `compose_verified_low_confidence_delta_hatch`: evidence-backed complete keep/revise verdicts may publish concrete low-confidence behavior deltas; trusted anchors retain their mode, withheld representative anchors force summary-only, verdict-quality failures suppress, representative-local merged-member qualification cannot leak, and the ordinary report cap still applies. - `compose_terminal_failure_fallback`: composer fails after repair; fallback emits template-worded, fingerprint-grouped, severity-ranked findings plus the needs-human-attention notes, with the semantic-composition-skipped disclosure, and loses nothing. - `compose_posting_plan_pr_mode`: `--pr --post-github-comments` mode yields `postingPlan` with inline anchors only for `publication: "inline"` findings and a review body containing counts and partial disclosure. diff --git a/src/evals/eval-artifacts.ts b/src/evals/eval-artifacts.ts index 791e772..98dca10 100644 --- a/src/evals/eval-artifacts.ts +++ b/src/evals/eval-artifacts.ts @@ -48,6 +48,7 @@ export async function loadEvalArtifacts(telemetryDir: string): Promise(dir, "coverage.json"); const reviewPlan = await readOptionalArtifact(dir, "review-plan.json"); const attention = await readOptionalArtifact(dir, "attention.json"); + const humanAttention = await readOptionalArtifact(dir, "human-attention-notes.json"); const coverage = normalizeCoverage(coverageRaw); const metricsSources: EvalArtifacts["metricsSources"] = {}; const costProfile = await readOptionalArtifact(dir, "cost-profile.json"); @@ -88,11 +89,15 @@ export async function loadEvalArtifacts(telemetryDir: string): Promise(dir, "human-attention-notes.json")), + humanAttentionNotes: normalizeHumanAttentionNotes(humanAttention), packets: await loadPackets(path.join(dir, "stages", "06-packets", "packets")), hintEvents: await loadHintEvents(path.join(dir, "events.jsonl")), metricsSources }; + const humanAttentionOutputNotes = normalizeHumanAttentionOutputNotes(humanAttention); + if (humanAttentionOutputNotes !== undefined) { + artifacts.humanAttentionOutputNotes = humanAttentionOutputNotes; + } if (attention !== undefined && Array.isArray(attention)) { artifacts.attention = attention; } @@ -207,6 +212,13 @@ function normalizeHumanAttentionNotes(raw: unknown): EvalHumanAttentionNote[] { }); } +function normalizeHumanAttentionOutputNotes(raw: unknown): EvalHumanAttentionNote[] | undefined { + if (!isRecord(raw) || !("outputNotes" in raw)) { + return undefined; + } + return normalizeHumanAttentionNotes(raw.outputNotes); +} + // Optional artifact read with the same pre-layout-v2 root fallback as // readScoredJson (plan 83): old runs stay loadable for replay and compare. async function readOptionalArtifact(dir: string, logicalName: string): Promise { diff --git a/src/evals/eval-scoring.ts b/src/evals/eval-scoring.ts index 48eaa75..f05691f 100644 --- a/src/evals/eval-scoring.ts +++ b/src/evals/eval-scoring.ts @@ -9,6 +9,7 @@ import type { EvalExpectationResult, EvalFindingExpectation, EvalHintEvent, + EvalHumanAttentionNote, EvalLossDetail, EvalLossLabel, EvalMatchOutcome, @@ -337,8 +338,13 @@ function scorePositiveList( const loss = list === "should_find" ? attributeLoss(expectation, artifacts) : attributeCandidateLoss(expectation, artifacts); - if (list === "should_find" && expectationMatchesNote(expectation, artifacts)) { - loss.surfacedAsNote = true; + if (list === "should_find") { + if (expectationMatchesNoteCollection(expectation, artifacts.humanAttentionNotes ?? [])) { + loss.noteGroupExisted = true; + } + if (expectationMatchesNote(expectation, artifacts)) { + loss.surfacedAsNote = true; + } } return { expectationId: expectation.id, @@ -547,7 +553,11 @@ function sumStageLossCounts(counts: Array | undefi // the expectation's path glob against note files and its regex patterns // against the note's question and reasons. export function expectationMatchesNote(expectation: EvalFindingExpectation, artifacts: EvalArtifacts): boolean { - const notes = artifacts.humanAttentionNotes ?? []; + const notes = artifacts.humanAttentionOutputNotes ?? artifacts.humanAttentionNotes ?? []; + return expectationMatchesNoteCollection(expectation, notes); +} + +function expectationMatchesNoteCollection(expectation: EvalFindingExpectation, notes: EvalHumanAttentionNote[]): boolean { if (notes.length === 0) { return false; } @@ -1093,7 +1103,10 @@ function verificationOutcome( return { subReason: normalizeGateReason(record.gateReason), outcome: `pre-gate=${record.gateReason}${duplicateSuffix}` }; } if (record.verdict.verificationIncomplete === true) { - return { subReason: "verification-incomplete", outcome: `outcome=incomplete${duplicateSuffix} reason=${record.verdict.reason}` }; + const subReason = record.verdict.reason.includes("revise_without_revision_payload") + ? "empty-revision" + : "verification-incomplete"; + return { subReason, outcome: `outcome=incomplete${duplicateSuffix} reason=${record.verdict.reason}` }; } if (record.verdict.verdict === "reject") { return { subReason: "verifier-rejected", outcome: `verdict=reject${duplicateSuffix} reason=${record.verdict.reason}` }; diff --git a/src/llm/schemas.ts b/src/llm/schemas.ts index 4601fd3..a979b4b 100644 --- a/src/llm/schemas.ts +++ b/src/llm/schemas.ts @@ -247,18 +247,25 @@ export const SubmitSystemReviewSchema = Type.Object( { additionalProperties: false } ); +const VerificationVerdictSharedProperties = { + reason: Type.String({ minLength: 1, maxLength: 2000 }), + requiredEvidencePresent: Type.Boolean(), + falsePositiveRisk: Type.Union([Type.Literal("low"), Type.Literal("medium"), Type.Literal("high")]), + behaviorChange: Type.Optional(BehaviorChangeAssessmentSchema), + intentEvidence: Type.Optional(Type.Array(Type.String({ minLength: 1, maxLength: 500 }), { maxItems: 8 })) +}; + export const SubmitVerificationVerdictSchema = Type.Object( { verdict: Type.Union([Type.Literal("keep"), Type.Literal("reject"), Type.Literal("revise")]), - reason: Type.String({ minLength: 1, maxLength: 2000 }), - requiredEvidencePresent: Type.Boolean(), - falsePositiveRisk: Type.Union([Type.Literal("low"), Type.Literal("medium"), Type.Literal("high")]), + ...VerificationVerdictSharedProperties, finalFinding: Type.Optional(SubmittedFindingSchema), - revisedAnchor: Type.Optional(DiffAnchorSchema), - behaviorChange: Type.Optional(BehaviorChangeAssessmentSchema), - intentEvidence: Type.Optional(Type.Array(Type.String({ minLength: 1, maxLength: 500 }), { maxItems: 8 })) + revisedAnchor: Type.Optional(DiffAnchorSchema) }, - { additionalProperties: false } + { + additionalProperties: false, + description: "Submit one verifier verdict. A revise verdict must include finalFinding or revisedAnchor; this semantic requirement is enforced after provider-safe schema validation." + } ); export const SubmitCompositionSchema = Type.Object( @@ -289,7 +296,7 @@ export const SCHEMA_VERSIONS = { submit_plan: 5, submit_review: 4, submit_system_review: 1, - submit_verdict: 1, + submit_verdict: 3, submit_composition: 1 } as const; diff --git a/src/pipeline/composer.ts b/src/pipeline/composer.ts index fcc74f7..6b3b52d 100644 --- a/src/pipeline/composer.ts +++ b/src/pipeline/composer.ts @@ -1397,6 +1397,8 @@ type ApplyCapsOptions = { telemetry: TelemetryRecorder; }; +type LowConfidenceBehaviorDeltaOutcome = "publish" | "publish_summary_only" | "suppress"; + function applyCaps( findings: FinalFinding[], config: CodegenieConfig, @@ -1411,7 +1413,14 @@ function applyCaps( return { ...finding, publication: "suppressed" as const }; } if (belowConfidence(finding.confidence, config.review.minConfidence)) { - if (isPublishableLowConfidenceBehaviorDelta(finding, opts.lowConfidencePublishableIds)) { + const outcome = lowConfidenceBehaviorDeltaOutcome(finding, opts.lowConfidencePublishableIds); + if (outcome !== "suppress") { + const published = outcome === "publish_summary_only" + ? { ...finding, publication: "summary-only" as const } + : finding; + if (outcome === "publish_summary_only") { + downgradeReasons.set(finding.id, "low-confidence-anchorless"); + } opts.telemetry.event({ stage: 10, level: "info", @@ -1422,10 +1431,12 @@ function applyCaps( mergedCandidateIds: finding.mergedCandidateIds, category: finding.category, confidence: finding.confidence, - severity: finding.severity + severity: finding.severity, + anchorless: outcome === "publish_summary_only", + publication: published.publication } }); - return finding; + return published; } suppressedReasons.set(finding.id, "confidence-threshold"); return { ...finding, publication: "suppressed" as const }; @@ -1467,33 +1478,47 @@ function applyCaps( function lowConfidencePublishableCandidateIds(verdicts: VerificationVerdict[]): Set { return new Set(verdicts - .filter((verdict) => verdict.verdict === "keep" || verdict.verdict === "revise") + .filter((verdict) => + (verdict.verdict === "keep" || verdict.verdict === "revise") && + verdict.requiredEvidencePresent === true && + verdict.falsePositiveRisk !== "high" && + verdict.verificationIncomplete !== true + ) .map((verdict) => verdict.candidateId)); } -function isPublishableLowConfidenceBehaviorDelta( +function lowConfidenceBehaviorDeltaOutcome( finding: FinalFinding, verifiedPublishableIds: Set -): boolean { +): LowConfidenceBehaviorDeltaOutcome { if (finding.confidence !== "low") { - return false; + return "suppress"; } - if (!finding.mergedCandidateIds.some((id) => verifiedPublishableIds.has(id))) { - return false; + if (!verifiedPublishableIds.has(finding.id)) { + return "suppress"; } - if (finding.publication === "suppressed" || finding.anchor === undefined || finding.changedLine !== true) { - return false; + if (finding.publication === "suppressed") { + return "suppress"; } if (!isBehaviorDeltaCategory(finding.category)) { - return false; + return "suppress"; } if (!hasConcreteText(finding.evidence.changedCode, 12) || (finding.evidence.relatedCode ?? []).length === 0) { - return false; + return "suppress"; } if (!hasConcreteText(finding.failureMode, 36) || !hasConcreteText(finding.whyThisMatters, 24)) { - return false; + return "suppress"; + } + if (!hasConfirmationPath(finding)) { + return "suppress"; + } + if (finding.anchor !== undefined && finding.changedLine === true) { + return "publish"; + } + if (finding.anchor === undefined && finding.changedLine === false) { + return "publish_summary_only"; } - return hasConfirmationPath(finding); + return "suppress"; } function isBehaviorDeltaCategory(category: CandidateFinding["category"]): boolean { diff --git a/src/pipeline/human-attention.ts b/src/pipeline/human-attention.ts index 0193050..1a64ccf 100644 --- a/src/pipeline/human-attention.ts +++ b/src/pipeline/human-attention.ts @@ -102,6 +102,12 @@ type VerificationSuppressionRecord = { }; }; +type PublicationFallbackRecord = { + groupKey: string; + candidateId: string; + verdict: VerificationResolution["verdict"]; +}; + export type HumanAttentionOutput = { notes: NeedsHumanAttentionNote[]; omittedCount: number; @@ -110,6 +116,9 @@ export type HumanAttentionOutput = { suppressedByVerification: VerificationSuppressionRecord[]; keptGroups: AttentionHintGroup[]; selectedGroups: AttentionHintGroup[]; + publicationFallbacks: PublicationFallbackRecord[]; + fallbackGroupCount: number; + omittedFallbackCount: number; }; export function buildHumanAttentionNotes( @@ -239,8 +248,29 @@ export function selectHumanAttentionForOutput( const availableAfterFindings = groups.filter((group) => !findings.some((finding) => attentionGroupCoveredByFinding(group, finding, packetsById))); const suppressedByFindingGroups = groups.filter((group) => !availableAfterFindings.includes(group)); const suppressedByFindings = suppressedByFindingGroups.map(toAttentionNote); - const verificationSuppression = suppressAttentionGroupsResolvedByVerification(availableAfterFindings, verificationResolutions); - const selected = selectHumanAttentionGroups(verificationSuppression.available); + const publishedCandidateIds = new Set(findings.flatMap((finding) => [finding.id, ...finding.mergedCandidateIds])); + const activeResolutions = verificationResolutions.filter((resolution) => + resolution.verdict === "reject" || publishedCandidateIds.has(resolution.candidateId) + ); + const fallbackResolutions = verificationResolutions.filter((resolution) => + (resolution.verdict === "keep" || resolution.verdict === "revise") && + !publishedCandidateIds.has(resolution.candidateId) + ); + const verificationSuppression = suppressAttentionGroupsResolvedByVerification(availableAfterFindings, activeResolutions); + const publicationFallbacks = verificationSuppression.available.flatMap((group): PublicationFallbackRecord[] => { + const match = firstVerificationResolutionMatch(group, fallbackResolutions); + return match === undefined ? [] : [{ + groupKey: group.key, + candidateId: match.resolution.candidateId, + verdict: match.resolution.verdict + }]; + }); + const fallbackGroupKeys = new Set(publicationFallbacks.map((fallback) => fallback.groupKey)); + const fallbackGroups = verificationSuppression.available.filter((group) => fallbackGroupKeys.has(group.key)); + const ordinaryGroups = verificationSuppression.available.filter((group) => !fallbackGroupKeys.has(group.key)); + const selected = selectHumanAttentionGroups([...fallbackGroups, ...ordinaryGroups]); + const selectedGroupKeys = new Set(selected.groups.map((group) => group.key)); + const omittedFallbackCount = fallbackGroups.filter((group) => !selectedGroupKeys.has(group.key)).length; if (suppressedByFindings.length > 0) { telemetry?.event({ @@ -278,6 +308,22 @@ export function selectHumanAttentionForOutput( } }); } + if (fallbackResolutions.length > 0) { + telemetry?.event({ + stage: 10, + level: "info", + message: "human_attention_publication_fallback", + data: { + fallbackResolutionCount: fallbackResolutions.length, + fallbackCandidateIds: capStrings(fallbackResolutions.map((resolution) => resolution.candidateId)), + fallbackGroupCount: fallbackGroups.length, + fallbackGroupIds: capStrings(fallbackGroups.map((group) => group.key)), + selectedFallbackGroupIds: capStrings(fallbackGroups.filter((group) => selectedGroupKeys.has(group.key)).map((group) => group.key)), + omittedFallbackCount, + maxHumanAttentionNotes: MAX_HUMAN_ATTENTION_NOTES + } + }); + } return { notes: selected.notes, @@ -286,7 +332,10 @@ export function selectHumanAttentionForOutput( suppressedByFindingGroups, suppressedByVerification: verificationSuppression.suppressed, keptGroups: verificationSuppression.available, - selectedGroups: selected.groups + selectedGroups: selected.groups, + publicationFallbacks: publicationFallbacks.slice(0, HUMAN_ATTENTION_LOCATION_CAP), + fallbackGroupCount: fallbackGroups.length, + omittedFallbackCount }; } @@ -395,6 +444,10 @@ export function humanAttentionArtifact( outputGroupIds: output.selectedGroups.map((group) => group.key), outputNotes: output.notes, omittedCount: output.omittedCount, + publicationFallbacks: output.publicationFallbacks, + fallbackGroupIds: output.publicationFallbacks.map((fallback) => fallback.groupKey), + fallbackGroupCount: output.fallbackGroupCount, + omittedFallbackCount: output.omittedFallbackCount, suppressedByFindings: output.suppressedByFindingGroups.map((group) => ({ groupKey: group.key, noteIds: [...group.rawNoteIds].sort() diff --git a/src/pipeline/uncertainty-promotion.ts b/src/pipeline/uncertainty-promotion.ts index d7a9740..dcbbf60 100644 --- a/src/pipeline/uncertainty-promotion.ts +++ b/src/pipeline/uncertainty-promotion.ts @@ -11,12 +11,14 @@ import { sha256Hex } from "../util/hashing.js"; import { scaleBudgetValue } from "../util/budget.js"; import { isPromotionTestPath } from "../util/path-roles.js"; import { escapeRegExp } from "../util/regex.js"; +import { normalizeFollowUpQuestion, normalizedAttentionTerms, tokenJaccard } from "../util/text-similarity.js"; const MAX_PROMOTIONS = 4; const MIN_PROMOTIONS_WHEN_AVAILABLE = 2; const MAX_EVIDENCE_CHARS = 2400; const MAX_RELATED_CONTEXT_EVIDENCE = 3; const MAX_RELATED_CONTEXT_EVIDENCE_CHARS = 1200; +const MAX_RELATED_PROMOTION_SIGNALS = 8; type PromotionInput = { packetResults: PacketReviewResult[]; @@ -28,6 +30,8 @@ export type UncertaintyPromotionSummary = { considered: number; promoted: number; laneLimited: number; + representedRelatedSignals: number; + unrepresentedLaneLimited: number; notPromoted: Record; promotedCandidateIds: string[]; decisions: PromotionDecision[]; @@ -71,6 +75,15 @@ type RankedPromotionSource = { localityScore: number; }; type SelectedPromotionSource = RankedPromotionSource & { selectedBy: PromotionSelectionReason }; +type RelatedPromotionSignal = NonNullable["relatedSignals"]>[number]; +type RelatedPromotionAssociation = { + source: RankedPromotionSource; + selected: SelectedPromotionSource; + selectedIndex: number; + exactQuestion: boolean; + sharedTerms: number; + similarity: number; +}; export async function promoteUncertaintiesForVerification( input: PromotionInput, @@ -95,14 +108,23 @@ export async function promoteUncertaintiesForVerification( const selected = selectPromotionSources(eligible, maxPromotions); const selectedSet = new Set(selected.map((item) => item.source)); - const laneLimited = eligible.filter((item) => !selectedSet.has(item.source)); - for (const limited of laneLimited) { - decisions.push(baseDecision(limited.source, false, "promotion_lane_limited", promotionDecisionMetadata(limited))); + const unselected = eligible.filter((item) => !selectedSet.has(item.source)); + const relatedAssociations = associateRelatedPromotionSignals(unselected, selected); + const laneLimited = unselected.filter((item) => !relatedAssociations.has(item.source)); + for (const unselectedItem of unselected) { + const association = relatedAssociations.get(unselectedItem.source); + decisions.push(association === undefined + ? baseDecision(unselectedItem.source, false, "promotion_lane_limited", promotionDecisionMetadata(unselectedItem)) + : { + ...baseDecision(unselectedItem.source, false, "represented_as_related_signal", promotionDecisionMetadata(unselectedItem)), + candidateId: promotedCandidateId(association.selected.source, association.selectedIndex) + }); } selected.forEach((selectedItem, index) => { const { source } = selectedItem; - const candidate = promotedCandidate(source, index); + const relatedSignals = relatedSignalsForSelected(index, relatedAssociations); + const candidate = promotedCandidate(source, index, relatedSignals); const existing = promotedByPacket.get(source.packet.id) ?? []; existing.push(candidate); promotedByPacket.set(source.packet.id, existing); @@ -124,6 +146,8 @@ export async function promoteUncertaintiesForVerification( considered: sources.length, promoted: selected.length, laneLimited: laneLimited.length, + representedRelatedSignals: relatedAssociations.size, + unrepresentedLaneLimited: laneLimited.length, notPromoted, promotedCandidateIds: selected.map(({ source }, index) => promotedCandidateId(source, index)), decisions @@ -138,6 +162,8 @@ export async function promoteUncertaintiesForVerification( considered: summary.considered, promoted: summary.promoted, laneLimited: summary.laneLimited, + representedRelatedSignals: summary.representedRelatedSignals, + unrepresentedLaneLimited: summary.unrepresentedLaneLimited, maxPromotions, notPromoted: summary.notPromoted, promotedCandidateIds: summary.promotedCandidateIds @@ -259,7 +285,11 @@ function pointsAtDistinctScope(source: PromotionSource): boolean { )); } -function promotedCandidate(source: PromotionSource, index: number): CandidateFinding { +function promotedCandidate( + source: PromotionSource, + index: number, + relatedSignals: RelatedPromotionSignal[] +): CandidateFinding { const risk = riskProfile(source); const confidence = promotedConfidence(source, risk.category); const relatedCode = relatedEvidence(source); @@ -300,11 +330,117 @@ function promotedCandidate(source: PromotionSource, index: number): CandidateFin question: source.question.trim(), files: source.files, symbols: source.symbols, - reason: source.reason.trim() || "promoted unresolved predicate for verification" + reason: source.reason.trim() || "promoted unresolved predicate for verification", + ...(relatedSignals.length > 0 + ? { + relatedSignals, + crossPacketRelatedCount: new Set(relatedSignals + .filter((signal) => signal.packetId !== source.packet.id) + .map((signal) => signal.packetId)).size + } + : {}) } }; } +function associateRelatedPromotionSignals( + unselected: RankedPromotionSource[], + selected: SelectedPromotionSource[] +): Map { + const proposed = unselected.flatMap((source): RelatedPromotionAssociation[] => { + const matches = selected.flatMap((selectedItem, selectedIndex): RelatedPromotionAssociation[] => { + const match = relatedPromotionAssociation(source, selectedItem, selectedIndex); + return match === undefined ? [] : [match]; + }).sort(compareRelatedPromotionAssociations); + return matches[0] === undefined ? [] : [matches[0]]; + }); + const accepted = new Map(); + for (let selectedIndex = 0; selectedIndex < selected.length; selectedIndex += 1) { + const forSelected = proposed + .filter((association) => association.selectedIndex === selectedIndex) + .sort(compareRelatedPromotionSignals) + .slice(0, MAX_RELATED_PROMOTION_SIGNALS); + for (const association of forSelected) { + accepted.set(association.source.source, association); + } + } + return accepted; +} + +function relatedPromotionAssociation( + source: RankedPromotionSource, + selected: SelectedPromotionSource, + selectedIndex: number +): RelatedPromotionAssociation | undefined { + if (riskProfile(source.source).category !== riskProfile(selected.source).category || + source.promotionClass !== selected.promotionClass || + !normalizedValuesOverlap(source.source.files, selected.source.files) || + !normalizedValuesOverlap(source.source.symbols, selected.source.symbols)) { + return undefined; + } + const sourceQuestion = normalizeFollowUpQuestion(source.source.question); + const selectedQuestion = normalizeFollowUpQuestion(selected.source.question); + const exactQuestion = sourceQuestion === selectedQuestion; + const sourceTerms = normalizedAttentionTerms(sourceQuestion); + const selectedTerms = normalizedAttentionTerms(selectedQuestion); + const sharedTerms = setIntersectionCount(sourceTerms, selectedTerms); + const similarity = tokenJaccard(sourceTerms, selectedTerms); + if (!exactQuestion && sharedTerms < 3 && similarity < 0.24) { + return undefined; + } + return { source, selected, selectedIndex, exactQuestion, sharedTerms, similarity }; +} + +function compareRelatedPromotionAssociations(a: RelatedPromotionAssociation, b: RelatedPromotionAssociation): number { + return Number(b.exactQuestion) - Number(a.exactQuestion) || + b.sharedTerms - a.sharedTerms || + b.similarity - a.similarity || + b.selected.rank - a.selected.rank || + a.selected.source.question.localeCompare(b.selected.source.question) || + a.selected.source.packet.id.localeCompare(b.selected.source.packet.id) || + a.selectedIndex - b.selectedIndex; +} + +function compareRelatedPromotionSignals(a: RelatedPromotionAssociation, b: RelatedPromotionAssociation): number { + return Number(b.exactQuestion) - Number(a.exactQuestion) || + b.sharedTerms - a.sharedTerms || + b.similarity - a.similarity || + a.source.source.question.localeCompare(b.source.source.question) || + a.source.source.packet.id.localeCompare(b.source.source.packet.id) || + a.source.source.sourceKind.localeCompare(b.source.source.sourceKind); +} + +function relatedSignalsForSelected( + selectedIndex: number, + associations: Map +): RelatedPromotionSignal[] { + return [...associations.values()] + .filter((association) => association.selectedIndex === selectedIndex) + .sort(compareRelatedPromotionSignals) + .map(({ source: { source } }) => ({ + packetId: source.packet.id, + sourceKind: source.sourceKind, + question: source.question.trim(), + files: source.files, + symbols: source.symbols + })); +} + +function normalizedValuesOverlap(left: string[], right: string[]): boolean { + const leftValues = new Set(left.map(normalize).filter(Boolean)); + return right.some((value) => leftValues.has(normalize(value))); +} + +function setIntersectionCount(left: Set, right: Set): number { + let count = 0; + for (const value of left) { + if (right.has(value)) { + count += 1; + } + } + return count; +} + function promotedCandidateId(source: PromotionSource, index: number): string { return `${source.packet.id.slice(0, 8)}-u${index + 1}-${sha256Hex([ source.sourceKind, diff --git a/src/pipeline/verifier.ts b/src/pipeline/verifier.ts index c1e9937..a0ffa2f 100644 --- a/src/pipeline/verifier.ts +++ b/src/pipeline/verifier.ts @@ -1,6 +1,6 @@ import { buildRepositoryToolDefinitions } from "../llm/tool-definitions.js"; import type { LlmRunner, LlmSchemaRepairInput } from "../llm/llm-runner.js"; -import { SubmitVerificationVerdictSchema, type SubmitVerificationVerdict } from "../llm/schemas.js"; +import { SCHEMA_VERSIONS, SubmitVerificationVerdictSchema, type SubmitVerificationVerdict } from "../llm/schemas.js"; import { skillsCompatibleWithLanguage, type LensRegistry } from "../skills/lens-registry.js"; import { fenceUntrusted, stableJson, type PromptBuilder } from "../skills/prompt-builder.js"; import type { TelemetryRecorder } from "../telemetry/telemetry-recorder.js"; @@ -11,6 +11,7 @@ import type { RepositoryTools, ReviewPacket, ReviewStage, + Severity, UnifiedDiff, VerificationVerdict } from "../types.js"; @@ -118,6 +119,8 @@ type VerificationRuntimeStats = { type VerifierSchemaInvalidKind = | "xml_parameter_bleed" + | "empty_submit_object" + | "revise_without_revision_payload" | "missing_submit_tool" | "invalid_tool_arguments" | "extra_tool_calls" @@ -601,12 +604,13 @@ async function verifyCandidate( }); const submitted = await runVerifierStructured(candidate, prompt, tools, config, opts, workerId, telemetry, runtimeStats); const normalized = normalizeSubmittedVerdict(candidate, submitted, telemetry); - const revised = normalized.finalFinding !== undefined - ? revisedFinding(candidate, normalized.finalFinding, packet, opts.diff) + const submittedFinalFinding = normalized.finalFinding; + const revised = submittedFinalFinding !== undefined + ? revisedFinding(candidate, submittedFinalFinding, packet, opts.diff) : undefined; const revisedAnchor = normalizeAnchor(normalized.revisedAnchor, packet, opts.diff); const verificationIncomplete = normalized.reason.startsWith("verification incomplete:"); - return { + const verdict: VerificationVerdict = { candidateId: candidate.id, verdict: verificationIncomplete ? "incomplete" : normalized.verdict, reason: normalized.reason, @@ -618,6 +622,45 @@ async function verifyCandidate( ...(normalized.behaviorChange !== undefined ? { behaviorChange: normalized.behaviorChange } : {}), ...(normalized.intentEvidence !== undefined ? { intentEvidence: normalized.intentEvidence } : {}) }; + if (verdict.verdict === "revise" && revised !== undefined && submittedFinalFinding !== undefined) { + // Derive the audit from the same fully policy-applied candidate that enters + // the verified set, including any verdict-level behavior assessment. + const policyAppliedRevision = applyVerificationVerdict(candidate, verdict); + const severityRevision = buildSeverityRevision( + candidate.severity, + submittedFinalFinding.severity, + policyAppliedRevision.severity + ); + telemetry.event({ + stage: 9, + level: severityRevision.deltaLevels >= 2 ? "info" : "debug", + message: "verification_severity_revision", + file: candidate.path, + data: { + candidateId: candidate.id, + category: policyAppliedRevision.category, + behaviorChange: policyAppliedRevision.behaviorChange, + ...severityRevision + } + }); + return { ...verdict, severityRevision }; + } + return verdict; +} + +const SEVERITY_RANK: Record = { low: 0, medium: 1, high: 2, critical: 3 }; + +function buildSeverityRevision( + original: Severity, + submitted: Severity, + applied: Severity +): NonNullable { + return { + original, + submitted, + applied, + deltaLevels: SEVERITY_RANK[submitted] - SEVERITY_RANK[original] + }; } function candidateLanguageFromDiff(candidate: CandidateFinding, diff: UnifiedDiff | undefined): string | undefined { @@ -643,8 +686,48 @@ function normalizeSubmittedVerdict( submitted: SubmitVerificationVerdict, telemetry: TelemetryRecorder ): SubmitVerificationVerdict { - if (submitted.verdict === "reject" || submitted.requiredEvidencePresent === true) { - return submitted; + // Schema-valid adapters never return null payloads, but normalize them away + // defensively before applying the same semantic checks to every verdict. + const { + finalFinding: rawFinalFinding, + revisedAnchor: rawRevisedAnchor, + ...submittedWithoutPayloads + } = submitted; + const finalFinding = rawFinalFinding ?? undefined; + const revisedAnchor = rawRevisedAnchor ?? undefined; + let normalized = { + ...submittedWithoutPayloads, + ...(finalFinding !== undefined ? { finalFinding } : {}), + ...(revisedAnchor !== undefined ? { revisedAnchor } : {}) + } as SubmitVerificationVerdict; + + if (normalized.verdict === "keep" && (finalFinding !== undefined || revisedAnchor !== undefined)) { + const payloadKinds = [ + ...(finalFinding !== undefined ? ["finalFinding"] : []), + ...(revisedAnchor !== undefined ? ["revisedAnchor"] : []) + ]; + telemetry.event({ + stage: 9, + level: "warn", + message: "verification_keep_payload_canonicalized", + file: candidate.path, + data: { candidateId: candidate.id, payloadKinds } + }); + normalized = { ...normalized, verdict: "revise" } as SubmitVerificationVerdict; + } + if (normalized.verdict === "revise" && finalFinding === undefined && revisedAnchor === undefined) { + const reason = "revise_without_revision_payload"; + telemetry.event({ + stage: 9, + level: "warn", + message: "verification_semantic_invalid", + file: candidate.path, + data: { candidateId: candidate.id, reason } + }); + return incompleteSubmittedVerdict(reason); + } + if (normalized.verdict === "reject" || normalized.requiredEvidencePresent === true) { + return normalized; } telemetry.event({ stage: 9, @@ -654,12 +737,12 @@ function normalizeSubmittedVerdict( data: { candidateId: candidate.id, originalVerdict: submitted.verdict, - falsePositiveRisk: submitted.falsePositiveRisk + falsePositiveRisk: normalized.falsePositiveRisk } }); return { verdict: "reject", - reason: `required evidence missing; original ${submitted.verdict} verdict rejected: ${submitted.reason}`, + reason: `required evidence missing; original ${submitted.verdict} verdict rejected: ${normalized.reason}`, requiredEvidencePresent: false, falsePositiveRisk: "high" }; @@ -697,6 +780,43 @@ async function runVerifierStructured( }); if (repairAttempt !== undefined) { runtimeStats.repairSucceeded += 1; + if (repairAttempt.classification === "empty_submit_object") { + telemetry.event({ + stage: 9, + level: "warn", + message: "verification_empty_submit_repair_discarded", + file: candidate.path, + data: { + candidateId: candidate.id, + classification: repairAttempt.classification, + repairedVerdict: result.verdict + } + }); + return incompleteSubmittedVerdict("schema_invalid_after_repair: empty_submit_object"); + } + } else if (isPrimaryVerifierSubmitAccepted(result)) { + telemetry.event({ + stage: 9, + level: "info", + message: "verification_primary_submit_accepted", + file: candidate.path, + data: { + candidateId: candidate.id, + submitTool: "submit_verdict", + schemaVersion: SCHEMA_VERSIONS.submit_verdict, + argumentsNonEmpty: true, + schemaRepairUsed: false + } + }); + } else if (isEmptySubmitObject(result)) { + recordVerifierSchemaInvalid( + candidate, + "submit_verdict returned an empty object", + "empty_submit_object", + telemetry, + runtimeStats + ); + return incompleteSubmittedVerdict("schema_invalid: empty_submit_object"); } return result; } catch (error) { @@ -793,15 +913,7 @@ function buildVerifierSchemaRepairPrompt( input: LlmSchemaRepairInput, attempt: VerifierRepairAttempt ): string { - const anchor = candidate.anchor - ? `${candidate.anchor.path}:${String(candidate.anchor.line)} ${candidate.anchor.side}` - : `${candidate.path}:unanchored`; - const candidateSummary = fenceUntrusted(stableJson({ - id: candidate.id, - title: candidate.title, - path: candidate.path, - anchor - }), "verifier-repair-candidate-summary"); + const candidateSummary = fenceUntrusted(stableJson(verifierRepairCandidateProjection(candidate)), "verifier-repair-candidate-summary"); return [ "Repair the Stage 9 verifier response for codegenie.", "", @@ -821,14 +933,18 @@ function buildVerifierSchemaRepairPrompt( "- Do not call repository tools or ask for more context.", "", "Verdict reminder:", + "- Judge only the bounded candidate evidence above. It preserves the candidate claim, not repository-tool results from the discarded response.", "- keep only if the candidate is proven by concrete evidence.", - "- revise only when the same issue is real but the evidence, wording, or anchor needs correction.", + "- revise only when the same issue is real but the evidence, wording, or anchor needs correction; include finalFinding or revisedAnchor.", "- reject when required evidence is missing, the claim is speculative, or false-positive risk is high.", "- If rejecting because verification cannot be completed, set requiredEvidencePresent=false and falsePositiveRisk=high." ].join("\n"); } function classifyVerifierSchemaInvalid(input: LlmSchemaRepairInput | string): VerifierSchemaInvalidKind { + if (typeof input !== "string" && isEmptySubmitObject(input.submitCalls[0]?.arguments)) { + return "empty_submit_object"; + } const errorText = typeof input === "string" ? input : input.error; const serializedSubmitArgs = typeof input === "string" ? "" @@ -837,6 +953,15 @@ function classifyVerifierSchemaInvalid(input: LlmSchemaRepairInput | string): Ve if (/<\/?\s*parameter\b/u.test(text) || /<\/?\s*parameter\b/u.test(text)) { return "xml_parameter_bleed"; } + if (typeof input !== "string" && input.submitCalls.some((call) => { + const argumentsValue = call.arguments; + return typeof argumentsValue === "object" && argumentsValue !== null && + "verdict" in argumentsValue && argumentsValue.verdict === "revise" && + (!("finalFinding" in argumentsValue) || argumentsValue.finalFinding == null) && + (!("revisedAnchor" in argumentsValue) || argumentsValue.revisedAnchor == null); + })) { + return "revise_without_revision_payload"; + } if (typeof input !== "string" && input.extraToolNames.length > 0) { return "extra_tool_calls"; } @@ -855,6 +980,90 @@ function classifyVerifierSchemaInvalid(input: LlmSchemaRepairInput | string): Ve return "unknown"; } +function verifierRepairCandidateProjection(candidate: CandidateFinding): Record { + const relatedCode = (candidate.evidence.relatedCode ?? []).slice(0, 3).map((entry) => ({ + path: boundedVerifierRepairText(entry.path, 500), + lines: boundedVerifierRepairText(entry.lines, 1200), + whyRelevant: boundedVerifierRepairText(entry.whyRelevant, 500) + })); + return { + id: boundedVerifierRepairText(candidate.id, 200), + title: boundedVerifierRepairText(candidate.title, 240), + severity: candidate.severity, + confidence: candidate.confidence, + category: candidate.category, + path: boundedVerifierRepairText(candidate.path, 500), + changedLine: candidate.changedLine, + ...(candidate.anchor !== undefined + ? { + anchor: { + path: boundedVerifierRepairText(candidate.anchor.path, 500), + line: candidate.anchor.line, + side: candidate.anchor.side, + hunkId: boundedVerifierRepairText(candidate.anchor.hunkId, 200), + ...(candidate.anchor.startLine !== undefined ? { startLine: candidate.anchor.startLine } : {}), + ...(candidate.anchor.startSide !== undefined ? { startSide: candidate.anchor.startSide } : {}), + ...(candidate.anchor.commitSha !== undefined + ? { commitSha: boundedVerifierRepairText(candidate.anchor.commitSha, 80) } + : {}) + } + } + : {}), + evidence: { + changedCode: boundedVerifierRepairText(candidate.evidence.changedCode, 2400), + relatedCode, + relatedCodeOmitted: Math.max(0, (candidate.evidence.relatedCode?.length ?? 0) - relatedCode.length) + }, + failureMode: boundedVerifierRepairText(candidate.failureMode, 1400), + whyThisMatters: boundedVerifierRepairText(candidate.whyThisMatters, 1000), + verification: boundedVerifierRepairText(candidate.verification, 1200), + ...(candidate.suggestedFix !== undefined ? { suggestedFix: boundedVerifierRepairText(candidate.suggestedFix, 1200) } : {}), + ...(candidate.suggestedTest !== undefined ? { suggestedTest: boundedVerifierRepairText(candidate.suggestedTest, 800) } : {}), + ...(candidate.behaviorChange !== undefined ? { behaviorChange: candidate.behaviorChange } : {}), + ...(candidate.intentEvidence !== undefined + ? { intentEvidence: candidate.intentEvidence.slice(0, 8).map((entry) => boundedVerifierRepairText(entry, 500)) } + : {}), + ...(candidate.provenance !== undefined + ? { + provenance: { + source: candidate.provenance.source, + sourceKind: candidate.provenance.sourceKind, + question: boundedVerifierRepairText(candidate.provenance.question, 700), + reason: boundedVerifierRepairText(candidate.provenance.reason, 700), + files: candidate.provenance.files.slice(0, 8).map((entry) => boundedVerifierRepairText(entry, 500)), + symbols: candidate.provenance.symbols.slice(0, 8).map((entry) => boundedVerifierRepairText(entry, 200)) + } + } + : {}) + }; +} + +function boundedVerifierRepairText(input: string, maxChars: number): string { + const escaped = input.trim().replaceAll("<", "\\u003c").replaceAll(">", "\\u003e"); + return escaped.length <= maxChars ? escaped : `${escaped.slice(0, maxChars - 3).trimEnd()}...`; +} + +function isEmptySubmitObject(input: unknown): boolean { + return typeof input === "object" && input !== null && !Array.isArray(input) && Object.keys(input).length === 0; +} + +function isPrimaryVerifierSubmitAccepted(input: unknown): input is SubmitVerificationVerdict { + if (typeof input !== "object" || input === null || Array.isArray(input)) { + return false; + } + const verdict = input as Partial; + const sharedFieldsValid = typeof verdict.reason === "string" && verdict.reason.length > 0 && + typeof verdict.requiredEvidencePresent === "boolean" && + (verdict.falsePositiveRisk === "low" || verdict.falsePositiveRisk === "medium" || verdict.falsePositiveRisk === "high"); + if (!sharedFieldsValid) { + return false; + } + if (verdict.verdict === "keep" || verdict.verdict === "reject") { + return true; + } + return verdict.verdict === "revise" && (verdict.finalFinding != null || verdict.revisedAnchor != null); +} + function sanitizeVerifierSchemaError(error: string): string { return clampVerifierDiagnostic( error diff --git a/src/skills/prompt-builder.ts b/src/skills/prompt-builder.ts index 5080f79..5d68afc 100644 --- a/src/skills/prompt-builder.ts +++ b/src/skills/prompt-builder.ts @@ -73,7 +73,7 @@ export const PROMPT_TEMPLATE_VERSIONS: Record<5 | 7 | 8 | 9 | 10, string> = { 5: "p5.6", 7: "p7.10", 8: "p8.2", - 9: "p9.6", + 9: "p9.8", 10: "p10.2" }; @@ -114,10 +114,14 @@ export const PROMPT_TEMPLATE_WHY_LEDGER: Record<5 | 7 | 8 | 9 | 10, PromptLedger 9: [ { surface: "verdict/requiredEvidencePresent/falsePositiveRisk", reason: "Separates truth decision, evidence sufficiency, and residual risk for final selection.", evidence: "Plan 74 merged-confidence calibration and Plan 87 exact duplicate policy" }, { surface: "finalFinding/revisedAnchor", reason: "Allows revise-with-evidence without letting gate-only or stale anchors create identity.", evidence: "Plan 76 anchor rescue and Plan 87 identity hardening" }, + { surface: "non-empty revision payload", reason: "Keeps structured revisions from completing without an actual finding or anchor change.", evidence: "Plan 106 / eval 49f4645b run 57 empty revise" }, + { surface: "decisive-evidence confidence calibration", reason: "Confidence follows the proven failure predicate rather than pressure from an unresolved secondary lookup.", evidence: "Plan 106 / eval 49f4645b run 55 secondary-budget cap" }, + { surface: "magnitude/reach severity rubric", reason: "Keeps severity proportional to concrete impact rather than the mere presence of a correctness invariant violation.", evidence: "Plan 108 / eval 49f4645b run 56 low-to-high inconsistency" }, { surface: "promoted predicate guidance", reason: "Promotion/adaptive candidates must be judged on the preserved predicate, not rejected for their original hint wording.", evidence: "Plan 81 and Plan 92 adaptive-vs-promotion measurement" }, { surface: "helper/callee complete-branch guidance", reason: "Prevents keeping helper-dependent claims from truncated or partial source reads.", evidence: "Fable review verifier false-positive class" }, { surface: "testing candidate guidance", reason: "Keeps real test-boundary regressions while rejecting generic add-more-tests comments.", evidence: "Plan 92 E1 escalator and Plan 75 suppression" }, { surface: "conditional skill guidance block", reason: "An empty authoritative provenance list must not leave a provider-facing label that implies verifier guidance was supplied.", evidence: "Plan 101 exact skill provenance" }, + { surface: "bounded verifier repair candidate evidence", reason: "Stateless replacement repair needs the candidate's claim and evidence without replaying contaminated output or discarded repository-tool state.", evidence: "Private eval 49f4645b runs 58-60; run 60 evidence-starved empty-submit repairs" }, { surface: "strict submit_verdict closeout", reason: "Verifier model repair is still live and successful, so the structured closeout remains load-bearing.", evidence: "Plan 95 census: 3 Stage-9 schema repairs, all recovered" } ], 10: [ @@ -302,7 +306,9 @@ export function createPromptBuilder(_registry: LensRegistry, options: ProjectSki return buildPrompt(9, [ reviewerFrame("verification"), injectionInstruction(), - "Verify whether the candidate is a real, actionable finding. Reject false positives. Revise only when the same issue is real but the evidence or anchor needs correction.", + "Verify whether the candidate is a real, actionable finding. Reject false positives. A bare keep means the candidate's confidence, severity, evidence, wording, and placement are publishable unchanged. Use revise for every structured change; a revision must include finalFinding or revisedAnchor because prose in reason does not change the candidate.", + "When a low-confidence promoted predicate is confirmed, revise with a complete finalFinding whose confidence and evidence reflect the decisive changed-code proof. Add revisedAnchor only when exact changed-line placement is proven. Medium confidence is appropriate when decisive changed-code evidence and the failure mode are confirmed even if one narrow secondary check remains unresolved. Tool refusal, truncation, or budget pressure on a secondary check must not keep confidence low. If the decisive predicate is unconfirmed, reject or set requiredEvidencePresent=false. Reserve low confidence for speculative reachability, ambiguous intent, or weak path matching.", + "Severity calibration: low means bounded or localized impact; medium means material but limited impact; high means broad or serious user/system impact; critical means catastrophic impact or compromise of a security boundary. Measure magnitude and reach, not merely whether a correctness invariant is technically violated. When changing severity by more than one level from the input candidate, quantify the concrete impact bound in finalFinding.verification.", "For candidates promoted from a follow-up hint or uncertainty, verify the concrete predicate preserved in provenance, failureMode, and verification text. Do not reject a runtime/design/correctness predicate solely because the original question also mentioned tests or coverage.", "For promoted lossy-transform predicates, verify that caller-visible outputs or bounds remain deliverable/satisfiable; before rejecting as immaterial precision loss, trace whether the visible output is derived from the transformed value or from the original source value. Documented or deliberate transformation intent can explain why the conversion exists, but it is not evidence that an overstated caller-visible guarantee is safe.", "Commit titles, PR text, and intent signals are context, not proof. Refactor-like or behavior-preserving intent can guide framing, but it is not evidence against a behavior-bearing correctness, security, design, or testing candidate. Source behavior and changed diff evidence control the verdict.", diff --git a/src/types.ts b/src/types.ts index ec9e134..7b75496 100644 --- a/src/types.ts +++ b/src/types.ts @@ -769,6 +769,14 @@ export type CandidateFindingProvenance = { files: string[]; symbols: string[]; reason: string; + relatedSignals?: Array<{ + packetId: string; + sourceKind: "uncertainty" | "follow_up_hint"; + question: string; + files: string[]; + symbols: string[]; + }>; + crossPacketRelatedCount?: number; }; // Anchor provenance (plan 76). Publication trusts only "model", @@ -888,6 +896,12 @@ export type VerificationVerdict = { verificationIncomplete?: boolean; behaviorChange?: BehaviorChangeAssessment; intentEvidence?: string[]; + severityRevision?: { + original: Severity; + submitted: Severity; + applied: Severity; + deltaLevels: number; + }; }; export type FinalFinding = CandidateFinding & { @@ -1109,6 +1123,9 @@ export type EvalLossDetail = { // The lost expectation resurfaced as a published Needs Human Attention note // (plan 79) — the NOTE outcome, a less-bad loss than a silent miss. surfacedAsNote?: boolean; + // The predicate existed in an internal attention group even if output + // selection did not render it (plan 110 diagnostic only). + noteGroupExisted?: boolean; }; export type EvalExpectationResult = { @@ -1349,8 +1366,10 @@ export type EvalArtifacts = { // crashing and destroying the run's data point (plan 89 A1). missingArtifacts?: string[]; // Published Needs Human Attention notes (plan 79): lets scoring distinguish - // a finding that resurfaced as a note (NOTE) from a silent miss (MISS). + // historical internal groups from a silent miss. New artifacts additionally + // carry the actually rendered output notes below (plan 110). humanAttentionNotes?: EvalHumanAttentionNote[]; + humanAttentionOutputNotes?: EvalHumanAttentionNote[]; reviewPlan?: ReviewPlan; // Plan 92 Layer 1 attention records (attention.json); absent on runs // predating the instrument. diff --git a/tests/evals.test.ts b/tests/evals.test.ts index 64cdd97..dce68f7 100644 --- a/tests/evals.test.ts +++ b/tests/evals.test.ts @@ -404,6 +404,61 @@ describe("eval scoring", () => { expect(score.budgetResults.every((result) => result.status === "pass")).toBe(true); }); + it("distinguishes empty verifier revisions from other incomplete verification losses", () => { + const emptyRevision = candidate("cand-empty-revision", "src/empty.ts", 4); + const timedOut = candidate("cand-timeout", "src/timeout.ts", 8); + const score = scoreEvalRun({ + name: "incomplete-verification-reasons", + artifacts: { path: "unused" }, + should_find: [ + { id: "empty-revision", path: "src/empty.ts" }, + { id: "other-incomplete", path: "src/timeout.ts" } + ] + }, { + candidates: [emptyRevision, timedOut], + verification: [ + { + candidateId: emptyRevision.id, + gate: "passed", + verdict: { + candidateId: emptyRevision.id, + verdict: "incomplete", + reason: "verification incomplete: schema_invalid_after_repair: revise_without_revision_payload", + requiredEvidencePresent: false, + falsePositiveRisk: "high", + verificationIncomplete: true + } + }, + { + candidateId: timedOut.id, + gate: "passed", + verdict: { + candidateId: timedOut.id, + verdict: "incomplete", + reason: "verification incomplete: worker_timed_out", + requiredEvidencePresent: false, + falsePositiveRisk: "high", + verificationIncomplete: true + } + } + ], + finalSelection: [], + finalFindings: [], + packets: [], + hintEvents: [], + metricsSources: {} + }, "live"); + + expect(score.expectationResults.find((result) => result.expectationId === "empty-revision")?.loss).toMatchObject({ + label: "lost-at-verification", + subReason: "empty-revision" + }); + expect(score.expectationResults.find((result) => result.expectationId === "other-incomplete")?.loss).toMatchObject({ + label: "lost-at-verification", + subReason: "verification-incomplete" + }); + }); + it("renders minimum and maximum budget failures with the correct comparison direction", () => { const score = scoreEvalRun({ name: "budget-direction", @@ -1143,6 +1198,25 @@ describe("eval compare", () => { }); describe("eval artifacts", () => { + it("loads internal human-attention groups and rendered output notes separately", async () => { + const telemetry = mkdtempSync(path.join(tmpdir(), "codegenie-rendered-notes-")); + writeArtifactSet(telemetry, [], []); + writeTelemetryArtifact(telemetry, "human-attention-notes.json", { + schemaVersion: 2, + groups: [{ question: "Internal predicate", files: ["src/internal.ts"], reasons: ["internal only"] }], + outputNotes: [{ question: "Rendered predicate", files: ["src/rendered.ts"], reason: "visible to the user" }] + }); + + const artifacts = await loadEvalArtifacts(telemetry); + + expect(artifacts.humanAttentionNotes).toEqual([ + { question: "Internal predicate", files: ["src/internal.ts"], reasons: ["internal only"] } + ]); + expect(artifacts.humanAttentionOutputNotes).toEqual([ + { question: "Rendered predicate", files: ["src/rendered.ts"], reasons: ["visible to the user"] } + ]); + }); + it("loads packet ids from top-level hint telemetry events", async () => { const telemetry = mkdtempSync(path.join(tmpdir(), "codegenie-hints-")); writeArtifactSet(telemetry, [], []); @@ -1204,6 +1278,7 @@ describe("eval artifacts", () => { expect(artifacts.candidates).toHaveLength(1); expect(artifacts.finalFindings).toHaveLength(1); expect(artifacts.missingArtifacts).toEqual([]); + expect(artifacts.humanAttentionOutputNotes).toBeUndefined(); }); it("discloses unreadable previous findings in compare reports", () => { @@ -2135,6 +2210,40 @@ describe("eval repeats (plan 79)", () => { const result = score.expectationResults.find((entry) => entry.expectationId === "wc"); expect(result?.status).toBe("fail"); expect(result?.loss?.surfacedAsNote).toBe(true); + expect(result?.loss?.noteGroupExisted).toBe(true); + }); + + it("scores only rendered notes while retaining internal-group and legacy diagnostics", () => { + const evalCase: EvalCase = { + name: "truthful-note-case", + repo: { external: "/tmp/unused" }, + should_find: [{ id: "wc", path: "src/app.ts", titlePattern: "stale value" }] + }; + const note = { question: "Is a stale value served here?", files: ["src/app.ts"], reasons: ["stale value risk"] }; + + const internalOnly = scoreEvalRun(evalCase, emptyArtifacts({ + humanAttentionNotes: [note], + humanAttentionOutputNotes: [] + }), "live").expectationResults[0]?.loss; + expect(internalOnly).toMatchObject({ noteGroupExisted: true }); + expect(internalOnly?.surfacedAsNote).toBeUndefined(); + + const rendered = scoreEvalRun(evalCase, emptyArtifacts({ + humanAttentionNotes: [note], + humanAttentionOutputNotes: [note] + }), "live").expectationResults[0]?.loss; + expect(rendered).toMatchObject({ noteGroupExisted: true, surfacedAsNote: true }); + + const explicitEmptyWithoutInternalMatch = scoreEvalRun(evalCase, emptyArtifacts({ + humanAttentionNotes: [{ question: "Different predicate", files: ["src/other.ts"], reasons: [] }], + humanAttentionOutputNotes: [] + }), "live").expectationResults[0]?.loss; + expect(explicitEmptyWithoutInternalMatch?.noteGroupExisted).toBeUndefined(); + expect(explicitEmptyWithoutInternalMatch?.surfacedAsNote).toBeUndefined(); + + const legacy = scoreEvalRun(evalCase, emptyArtifacts({ humanAttentionNotes: [note] }), "replay") + .expectationResults[0]?.loss; + expect(legacy).toMatchObject({ noteGroupExisted: true, surfacedAsNote: true }); }); it("isolates the relay wrong-chain bug from the zero-guard and duration look-alikes", () => { diff --git a/tests/human-attention-adjudication.test.ts b/tests/human-attention-adjudication.test.ts index cabb4b3..283372e 100644 --- a/tests/human-attention-adjudication.test.ts +++ b/tests/human-attention-adjudication.test.ts @@ -1,11 +1,15 @@ import { describe, expect, it } from "vitest"; import { + type AttentionHintGroup, buildHumanAttentionNotes, buildVerificationResolutionIndex, + selectHumanAttentionForOutput, + type VerificationResolution, suppressAttentionGroupsResolvedByVerification } from "../src/pipeline/human-attention.js"; import type { CandidateFinding, + FinalFinding, PacketReviewResult, ReviewPacket, RunCoverageStatus, @@ -165,3 +169,164 @@ describe("plan 75 step 1: adjudicated-reject note suppression", () => { expect(suppressed).toHaveLength(0); }); }); + +describe("plan 110 publication-aware note fallback", () => { + it("keeps unpublished keep/revise resolutions as fallbacks while published resolutions and rejects stay active", () => { + const group = attentionGroup(1); + const keep = fallbackResolution(group, "candidate-keep", "keep"); + + const unpublished = selectHumanAttentionForOutput([group], [], new Map(), [keep]); + expect(unpublished.notes).toHaveLength(1); + expect(unpublished.suppressedByVerification).toEqual([]); + expect(unpublished.publicationFallbacks).toEqual([ + { groupKey: group.key, candidateId: "candidate-keep", verdict: "keep" } + ]); + + const published = selectHumanAttentionForOutput( + [group], + [publishedFinding("candidate-keep")], + new Map(), + [keep] + ); + expect(published.notes).toEqual([]); + expect(published.suppressedByVerification).toEqual([ + expect.objectContaining({ candidateId: "candidate-keep", verdict: "keep" }) + ]); + + const reject = selectHumanAttentionForOutput( + [group], + [], + new Map(), + [fallbackResolution(group, "candidate-reject", "reject")] + ); + expect(reject.notes).toEqual([]); + expect(reject.suppressedByVerification).toEqual([ + expect.objectContaining({ candidateId: "candidate-reject", verdict: "reject" }) + ]); + }); + + it("prioritizes a sixth-ranked publication fallback inside the unchanged five-note cap", () => { + const groups = Array.from({ length: 6 }, (_, index) => attentionGroup(index + 1)); + const fallback = fallbackResolution(groups[5]!, "candidate-sixth", "keep"); + const events: Array<{ message: string; data?: Record }> = []; + + const output = selectHumanAttentionForOutput( + groups, + [], + new Map(), + [fallback], + { ...nullTelemetry(), event: (event) => events.push(event as never) } + ); + + expect(output.selectedGroups.map((group) => group.key)).toEqual([ + groups[5]!.key, + groups[0]!.key, + groups[1]!.key, + groups[2]!.key, + groups[3]!.key + ]); + expect(output.notes).toHaveLength(5); + expect(output.omittedCount).toBe(1); + expect(output.fallbackGroupCount).toBe(1); + expect(output.omittedFallbackCount).toBe(0); + expect(events).toContainEqual(expect.objectContaining({ + message: "human_attention_publication_fallback", + data: expect.objectContaining({ + fallbackGroupCount: 1, + fallbackGroupIds: [groups[5]!.key], + omittedFallbackCount: 0, + maxHumanAttentionNotes: 5 + }) + })); + }); + + it("renders the highest-ranked five when publication fallbacks overflow the cap", () => { + const groups = Array.from({ length: 6 }, (_, index) => attentionGroup(index + 1)); + const output = selectHumanAttentionForOutput( + groups, + [], + new Map(), + groups.map((group, index) => fallbackResolution(group, `candidate-${String(index + 1)}`, "revise")) + ); + + expect(output.selectedGroups.map((group) => group.key)).toEqual(groups.slice(0, 5).map((group) => group.key)); + expect(output.fallbackGroupCount).toBe(6); + expect(output.omittedFallbackCount).toBe(1); + expect(output.publicationFallbacks).toHaveLength(6); + }); +}); + +function attentionGroup(index: number): AttentionHintGroup { + const packetId = `packet-${String(index)}`; + const question = `Does fallback predicate ${String(index)} remain valid?`; + const file = `src/file-${String(index)}.ts`; + const symbol = `predicate${String(index)}`; + return { + key: `group-${String(index)}`, + representative: { + id: `note-${String(index)}`, + source: "follow_up_hint", + question, + files: [file], + originalFiles: [file], + droppedPaths: [], + symbols: [symbol], + suggestedLenses: [], + reason: `Predicate ${String(index)} needs verification.`, + confidence: "medium", + packetId + }, + files: [file], + symbols: [symbol], + reasons: [`Predicate ${String(index)} needs verification.`], + rawNoteIds: new Set([`note-${String(index)}`]), + droppedPaths: [], + invalidPathCount: 0, + packetIds: new Set([packetId]), + sources: new Set(["follow_up_hint"]), + count: 1 + }; +} + +function fallbackResolution( + group: AttentionHintGroup, + candidateId: string, + verdict: VerificationResolution["verdict"] +): VerificationResolution { + return { + source: "stage9_verified_predicate", + candidateId, + verdict, + reason: "The predicate was adjudicated.", + files: group.files, + symbols: group.symbols, + terms: new Set(), + questionKeys: new Set(), + provenance: { + source: "uncertainty_promotion", + sourceKind: "follow_up_hint", + sourcePacketId: group.representative.packetId, + question: group.representative.question, + files: group.files, + symbols: group.symbols, + reason: "promoted fallback predicate" + } + }; +} + +function publishedFinding(candidateId: string): FinalFinding { + return { + ...promotedCandidate(), + id: "published-final", + path: "src/unrelated.ts", + producedBy: { kind: "packet", stage: 7, packetId: "unrelated", lensId: "core/code-review", skillIds: [] }, + fingerprint: "published-fingerprint", + finalBody: "Published body.", + publication: "summary-only", + mergedCandidateIds: [candidateId], + mergedCategories: ["correctness"], + mergedSeverities: ["medium"], + mergedPaths: ["src/unrelated.ts"], + mergedTitles: ["Published final"] + }; +} diff --git a/tests/phase4-llm.test.ts b/tests/phase4-llm.test.ts index def71a2..c40ca87 100644 --- a/tests/phase4-llm.test.ts +++ b/tests/phase4-llm.test.ts @@ -66,6 +66,7 @@ describe("Phase 4 schemas and repository tool definitions", () => { expect(submitToolNameForStage(9)).toBe("submit_verdict"); expect(submitToolNameForStage(10)).toBe("submit_composition"); expect(SCHEMA_VERSIONS.submit_plan).toBe(5); + expect(SCHEMA_VERSIONS.submit_verdict).toBe(3); const valid = { diffUnderstanding: { declaredIntent: "Small change", inferredBehavior: "The diff makes a small change." }, @@ -171,6 +172,55 @@ describe("Phase 4 schemas and repository tool definitions", () => { ).toThrow(); }); + it("exposes a provider-safe flat verdict schema while leaving revise payload semantics to runtime", () => { + const tool = { name: "submit_verdict", description: "submit", parameters: SubmitVerificationVerdictSchema }; + const common = { + reason: "decisive evidence supports this verdict", + requiredEvidencePresent: true, + falsePositiveRisk: "low" + }; + const finalFinding = validCandidateReviewFinding(); + const revisedAnchor = { path: "src/a.ts", line: 4, side: "RIGHT", hunkId: "hunk-1" }; + const validate = (argumentsValue: Record) => validateToolCall([tool], { + type: "toolCall", + id: "submit-verdict-contract", + name: "submit_verdict", + arguments: argumentsValue + }); + const rootSchema = SubmitVerificationVerdictSchema as { + type?: string; + anyOf?: unknown; + required?: string[]; + properties?: Record; + }; + + expect(rootSchema.type).toBe("object"); + expect(rootSchema.anyOf).toBeUndefined(); + expect(rootSchema.required).toEqual(expect.arrayContaining([ + "verdict", + "reason", + "requiredEvidencePresent", + "falsePositiveRisk" + ])); + expect(rootSchema.required).not.toContain("finalFinding"); + expect(rootSchema.required).not.toContain("revisedAnchor"); + expect(rootSchema.properties).toHaveProperty("verdict"); + + expect(validate({ verdict: "keep", ...common })).toEqual({ verdict: "keep", ...common }); + expect(validate({ verdict: "reject", ...common })).toEqual({ verdict: "reject", ...common }); + expect(validate({ verdict: "keep", ...common, finalFinding })).toEqual({ verdict: "keep", ...common, finalFinding }); + expect(validate({ verdict: "revise", ...common, finalFinding })).toEqual({ verdict: "revise", ...common, finalFinding }); + expect(validate({ verdict: "revise", ...common, revisedAnchor })).toEqual({ verdict: "revise", ...common, revisedAnchor }); + expect(validate({ verdict: "revise", ...common, finalFinding, revisedAnchor })).toEqual({ + verdict: "revise", + ...common, + finalFinding, + revisedAnchor + }); + expect(validate({ verdict: "revise", ...common })).toEqual({ verdict: "revise", ...common }); + expect(() => validate({ verdict: "revise", ...common, finalFinding, extra: true })).toThrow(); + }); + it("defines all nine repository tools and renders tool failures as model-visible errors", async () => { const defs = buildRepositoryToolDefinitions(fakeRepositoryTools()); expect(defs.map((tool) => tool.name)).toEqual([ diff --git a/tests/pipeline-phase5.test.ts b/tests/pipeline-phase5.test.ts index d20432a..453a13a 100644 --- a/tests/pipeline-phase5.test.ts +++ b/tests/pipeline-phase5.test.ts @@ -45,7 +45,8 @@ import type { StaticSignal, SymbolMentionOptions, TelemetryEvent, - UnifiedDiff + UnifiedDiff, + VerificationVerdict } from "../src/types.js"; import { CodegenieError } from "../src/util/errors.js"; import { sha256Hex } from "../src/util/hashing.js"; @@ -8338,6 +8339,10 @@ describe("phase 5 pipeline regressions", () => { expect(calls).toBe(1); expect(repairPrompt).toContain("untrusted-data label=verifier-repair-candidate-summary"); expect(repairPrompt).toContain("\"id\": \"finding-1\""); + expect(repairPrompt).toContain("\"changedCode\": \"bad\""); + expect(repairPrompt).toContain("\"failureMode\": \"bad\""); + expect(repairPrompt).toContain("\"whyThisMatters\": \"matters\""); + expect(repairPrompt).toContain("\"verification\": \"verified\""); expect(repairPrompt).toContain("- class: xml_parameter_bleed"); expect(repairPrompt).toContain("Do not output XML."); expect(repairPrompt).toContain("Do not write `` tags."); @@ -8375,9 +8380,185 @@ describe("phase 5 pipeline regressions", () => { }) }) })); + expect(events).not.toEqual(expect.arrayContaining([ + expect.objectContaining({ message: "verification_primary_submit_accepted" }) + ])); + }); + + it("fails closed when an empty authoritative Stage 9 submit precedes later XML", async () => { + let repairPrompt = ""; + const events: Array> = []; + const artifacts = new Map(); + const finding: CandidateFinding = { + ...fakeFinding(), + title: "EXACT_OUTPUT cross-decimal quote overstates the deliverable amount", + evidence: { + changedCode: "+ transferAmount = scaleAmount(amountBig, destinationDecimals, originDecimals)", + relatedCode: [{ + path: "process_quote.go", + lines: "ToAmountMin: destinationAmount", + whyRelevant: "The published minimum uses the unrounded destination amount." + }] + }, + failureMode: "Integer division truncates the packed transfer amount while ToAmountMin retains the larger requested destination amount.", + whyThisMatters: "The quote can promise a minimum output that the packed transfer cannot deliver.", + verification: `The changed scaling branch and the ToAmountMin assignment establish the concrete mismatch. CANDIDATE_XML ${"bounded ".repeat(250)}OMITTED_TAIL` + }; + const runner: LlmRunner = { + runStructured: async (request: LlmStructuredRequest) => { + repairPrompt = request.schemaRepair?.buildPrompt?.({ + stage: 9, + submitTool: "submit_verdict", + error: "submit_verdict arguments were schema-invalid: missing required fields; later call contained BAD_LATER_XML", + submitCalls: [ + { id: "submit-verdict-empty", arguments: {} }, + { id: "submit-verdict-later-xml", arguments: { parameter: "BAD_LATER_XML" } } + ], + extraToolNames: [] + }) ?? ""; + return { + verdict: "keep", + reason: "The bounded candidate evidence supports the finding.", + requiredEvidencePresent: true, + falsePositiveRisk: "low" + } as T; + } + }; + + const verified = await verifyFindings( + { + packetResults: [{ packetId: "packet-1", lenses: ["core/code-review"], findings: [finding], followUpHints: [], uncertainties: [], status: "completed" }], + packets: [fakePacket()] + }, + fakeTools(), + config(), + { + ...nullTelemetry(), + event: (event: Omit) => { + events.push(event); + }, + writeArtifact: async (name: string, data: unknown) => { + artifacts.set(name, data); + } + }, + { + runner, + promptBuilder: fakePromptBuilder(), + lensRegistry: fakeLensRegistry(), + diff: fakeDiff(), + checkpoint: () => "ok" + } + ); + + expect(repairPrompt.length).toBeLessThan(15_000); + expect(repairPrompt).toContain("- class: empty_submit_object"); + expect(repairPrompt).not.toContain("BAD_LATER_XML"); + expect(repairPrompt).toContain("\"title\": \"EXACT_OUTPUT cross-decimal quote overstates the deliverable amount\""); + expect(repairPrompt).toContain("\"changedCode\": \"+ transferAmount = scaleAmount"); + expect(repairPrompt).toContain("\"relatedCode\": ["); + expect(repairPrompt).toContain("\"failureMode\": \"Integer division truncates"); + expect(repairPrompt).toContain("\"whyThisMatters\": \"The quote can promise"); + expect(repairPrompt).toContain("\"verification\": \"The changed scaling branch"); + expect(repairPrompt).toContain("\\\\u003cparameter\\\\u003eCANDIDATE_XML"); + expect(repairPrompt).not.toContain("CANDIDATE_XML"); + expect(repairPrompt).not.toContain("OMITTED_TAIL"); + expect(repairPrompt).toContain("Judge only the bounded candidate evidence above"); + expect(verified.verified).toEqual([]); + expect(verified.incompleteCount).toBe(1); + expect(verified.verdicts[0]).toMatchObject({ + verdict: "incomplete", + verificationIncomplete: true, + reason: "verification incomplete: schema_invalid_after_repair: empty_submit_object" + }); + expect(artifacts.get("verification.json")).toEqual([ + expect.objectContaining({ + candidateId: finding.id, + verificationStatus: "incomplete", + incompleteReason: "schema_invalid" + }) + ]); + expect(events).toEqual(expect.arrayContaining([ + expect.objectContaining({ + message: "verification_schema_invalid", + data: expect.objectContaining({ candidateId: finding.id, classification: "empty_submit_object" }) + }), + expect.objectContaining({ + message: "verification_schema_repair_attempted", + data: expect.objectContaining({ candidateId: finding.id, classification: "empty_submit_object" }) + }), + expect.objectContaining({ + message: "verification_empty_submit_repair_discarded", + data: expect.objectContaining({ candidateId: finding.id, repairedVerdict: "keep" }) + }) + ])); + expect(events).not.toEqual(expect.arrayContaining([ + expect.objectContaining({ message: "verification_primary_submit_accepted" }) + ])); + }); + + it("repairs an empty revise with a required structured payload", async () => { + let repairPrompt = ""; + const events: Array> = []; + const runner: LlmRunner = { + runStructured: async (request: LlmStructuredRequest) => { + repairPrompt = request.schemaRepair?.buildPrompt?.({ + stage: 9, + submitTool: "submit_verdict", + error: "schema-invalid arguments: revise requires a revision payload", + submitCalls: [{ + id: "submit-verdict-empty-revise", + arguments: { + verdict: "revise", + reason: "The issue is real but needs changes.", + requiredEvidencePresent: true, + falsePositiveRisk: "low" + } + }], + extraToolNames: [] + }) ?? ""; + return { + verdict: "revise", + reason: "The issue is real and the exact changed line is proven.", + requiredEvidencePresent: true, + falsePositiveRisk: "low", + revisedAnchor: { path: "app.ts", line: 1, side: "RIGHT", hunkId: "h1" } + } as T; + } + }; + + const verified = await verifyFindings( + { + packetResults: [{ packetId: "packet-1", lenses: ["core/code-review"], findings: [fakeFinding()], followUpHints: [], uncertainties: [], status: "completed" }], + packets: [fakePacket()] + }, + fakeTools(), + config(), + { + ...nullTelemetry(), + event: (event: Omit) => { + events.push(event); + } + }, + { + runner, + promptBuilder: fakePromptBuilder(), + lensRegistry: fakeLensRegistry(), + diff: fakeDiff(), + checkpoint: () => "ok" + } + ); + + expect(repairPrompt).toContain("- class: revise_without_revision_payload"); + expect(repairPrompt).toContain("include finalFinding or revisedAnchor"); + expect(verified.verdicts[0]).toMatchObject({ verdict: "revise", revisedAnchor: { path: "app.ts", line: 1 } }); + expect(verified.verified).toHaveLength(1); + expect(events).toContainEqual(expect.objectContaining({ + message: "verification_schema_repair_attempted", + data: expect.objectContaining({ classification: "revise_without_revision_payload" }) + })); }); - it("marks verifier schema-invalid after compact repair incomplete with classification", async () => { + it("marks an empty revise incomplete when its compact repair also fails", async () => { const events: Array> = []; const artifacts = new Map(); const runner: LlmRunner = { @@ -8385,8 +8566,11 @@ describe("phase 5 pipeline regressions", () => { request.schemaRepair?.buildPrompt?.({ stage: 9, submitTool: "submit_verdict", - error: "schema-invalid arguments: BAD_PRIOR_XML_BODY", - submitCalls: [{ id: "submit-verdict-bad", arguments: { parameter: "BAD_PRIOR_XML_BODY" } }], + error: "schema-invalid arguments: revise requires a revision payload", + submitCalls: [{ + id: "submit-verdict-bad", + arguments: { verdict: "revise", reason: "prose-only revision", requiredEvidencePresent: true, falsePositiveRisk: "low" } + }], extraToolNames: [] }); throw new CodegenieError("llm_schema_invalid", "bad verifier schema after repair", { @@ -8430,7 +8614,7 @@ describe("phase 5 pipeline regressions", () => { incompleteReason: "schema_invalid", verdict: expect.objectContaining({ verificationIncomplete: true, - reason: expect.stringContaining("schema_invalid_after_repair: xml_parameter_bleed") + reason: expect.stringContaining("schema_invalid_after_repair: revise_without_revision_payload") }) }) ]); @@ -8440,7 +8624,7 @@ describe("phase 5 pipeline regressions", () => { message: "verification_schema_repair_failed", data: expect.objectContaining({ candidateId: "finding-1", - classification: "xml_parameter_bleed" + classification: "revise_without_revision_payload" }) })); expect(events).toContainEqual(expect.objectContaining({ @@ -8456,7 +8640,7 @@ describe("phase 5 pipeline regressions", () => { })); }); - it("marks schema-invalid verifier output incomplete when repair cannot be dispatched", async () => { + it("marks an empty revise incomplete when budget prevents repair dispatch", async () => { let calls = 0; const artifacts = new Map(); const runner: LlmRunner = { @@ -8465,8 +8649,11 @@ describe("phase 5 pipeline regressions", () => { request.schemaRepair?.buildPrompt?.({ stage: 9, submitTool: "submit_verdict", - error: "missing required property verdict", - submitCalls: [{ id: "submit-verdict-bad", arguments: { reason: "missing verdict" } }], + error: "schema-invalid arguments: revise requires a revision payload", + submitCalls: [{ + id: "submit-verdict-bad", + arguments: { verdict: "revise", reason: "prose-only revision", requiredEvidencePresent: true, falsePositiveRisk: "low" } + }], extraToolNames: [] }); throw new CodegenieError("budget_exhausted", "budget exhausted before repair dispatch", { @@ -8508,7 +8695,7 @@ describe("phase 5 pipeline regressions", () => { incompleteReason: "schema_invalid", verdict: expect.objectContaining({ verificationIncomplete: true, - reason: expect.stringContaining("repair not dispatched") + reason: expect.stringContaining("repair not dispatched because budget was exhausted: revise_without_revision_payload") }) }) ]); @@ -11584,10 +11771,188 @@ describe("phase 5 pipeline regressions", () => { stage: 10, message: "low_confidence_verified_delta_published", file: finding.path, - data: expect.objectContaining({ findingId: finding.id }) + data: expect.objectContaining({ findingId: finding.id, anchorless: false, publication: "inline" }) })); }); + it("publishes a qualified promotion after withholding its gate-only anchor as summary-only", async () => { + const finding = lowConfidenceDeltaFinding("promotion-anchorless", { + anchorSource: "backfill_packet_representative", + provenance: { + source: "uncertainty_promotion", + sourceKind: "follow_up_hint", + sourcePacketId: "packet-1", + question: "Does the scaled amount remain deliverable?", + files: ["app.ts"], + symbols: ["calculateAmountFromUSD"], + reason: "The changed conversion may overstate a caller-visible bound." + } + }); + const events: Array> = []; + const artifacts = new Map(); + + const result = await dedupeRankAndComposeReview( + { + verified: [finding], + verdicts: [qualifiedLowConfidenceVerdict(finding.id)] + }, + fakePlan(), + { mode: "branch", repoRoot: "/tmp/repo", commits: [], rawDiff: "" }, + fakeCoverage(), + config(), + { + ...nullTelemetry(), + event: (event) => { + events.push(event); + }, + writeArtifact: async (name, data) => { + artifacts.set(name, data); + } + }, + { + runner: { + runStructured: async () => ({ + summary: "Found 1 verified issue.", + composedFindings: [{ findingIds: [finding.id], finalBody: "The changed conversion can overstate the deliverable amount.", publication: "inline" }] + }) as T + }, + promptBuilder: fakePromptBuilder(), + diff: fakeDiff() + } + ); + + expect(result.noFindings).toBe(false); + expect(result.findings).toEqual([]); + expect(result.summaryOnlyFindings).toEqual([ + expect.objectContaining({ id: finding.id, publication: "summary-only", changedLine: false }) + ]); + expect(result.summaryOnlyFindings[0]?.anchor).toBeUndefined(); + expect(events).toEqual(expect.arrayContaining([ + expect.objectContaining({ message: "representative_anchor_withheld", data: expect.objectContaining({ candidateId: finding.id }) }), + expect.objectContaining({ + message: "low_confidence_verified_delta_published", + data: expect.objectContaining({ findingId: finding.id, anchorless: true, publication: "summary-only" }) + }) + ])); + expect(artifacts.get("final-selection.json")).toMatchObject({ + records: [expect.objectContaining({ findingId: finding.id, decision: "published", reason: "low-confidence-anchorless" })] + }); + }); + + it("requires complete evidence-backed non-high-risk keep or revise verdicts for the low-confidence hatch", async () => { + const finding = lowConfidenceDeltaFinding(); + const cases: Array<{ name: string; verdict: VerificationVerdict; published: boolean }> = [ + { name: "keep", verdict: qualifiedLowConfidenceVerdict(finding.id), published: true }, + { name: "revise", verdict: { ...qualifiedLowConfidenceVerdict(finding.id), verdict: "revise" }, published: true }, + { name: "missing evidence", verdict: { ...qualifiedLowConfidenceVerdict(finding.id), requiredEvidencePresent: false }, published: false }, + { name: "high risk", verdict: { ...qualifiedLowConfidenceVerdict(finding.id), falsePositiveRisk: "high" }, published: false }, + { name: "incomplete", verdict: { ...qualifiedLowConfidenceVerdict(finding.id), verificationIncomplete: true }, published: false }, + { name: "reject", verdict: { ...qualifiedLowConfidenceVerdict(finding.id), verdict: "reject" }, published: false } + ]; + + for (const testCase of cases) { + const result = await dedupeRankAndComposeReview( + { verified: [finding], verdicts: [testCase.verdict] }, + fakePlan(), + { mode: "branch", repoRoot: "/tmp/repo", commits: [], rawDiff: "" }, + fakeCoverage(), + config(), + nullTelemetry(), + { + runner: { runStructured: async () => { throw composerTransientError(); } }, + promptBuilder: fakePromptBuilder(), + diff: fakeDiff() + } + ); + + expect(result.noFindings, testCase.name).toBe(!testCase.published); + expect(result.findings, testCase.name).toHaveLength(testCase.published ? 1 : 0); + } + }); + + it("uses only the final representative verdict when merged members disagree on hatch qualification", async () => { + const representative = lowConfidenceDeltaFinding("a-representative"); + const sibling = lowConfidenceDeltaFinding("z-sibling"); + for (const { name, representativeVerdict, siblingVerdict, published } of [ + { + name: "qualifying sibling cannot lend eligibility", + representativeVerdict: { ...qualifiedLowConfidenceVerdict(representative.id), requiredEvidencePresent: false }, + siblingVerdict: qualifiedLowConfidenceVerdict(sibling.id), + published: false + }, + { + name: "non-qualifying sibling cannot revoke eligibility", + representativeVerdict: qualifiedLowConfidenceVerdict(representative.id), + siblingVerdict: { ...qualifiedLowConfidenceVerdict(sibling.id), falsePositiveRisk: "high" as const }, + published: true + } + ]) { + const result = await dedupeRankAndComposeReview( + { verified: [representative, sibling], verdicts: [representativeVerdict, siblingVerdict] }, + fakePlan(), + { mode: "branch", repoRoot: "/tmp/repo", commits: [], rawDiff: "" }, + fakeCoverage(), + config(), + nullTelemetry(), + { + runner: { runStructured: async () => { throw composerTransientError(); } }, + promptBuilder: fakePromptBuilder(), + diff: fakeDiff() + } + ); + + expect(result.noFindings, name).toBe(!published); + expect(result.findings, name).toEqual(published + ? [expect.objectContaining({ id: representative.id, mergedCandidateIds: expect.arrayContaining([representative.id, sibling.id]) })] + : []); + } + }); + + it("still applies the ordinary report cap after the low-confidence hatch", async () => { + const first = lowConfidenceDeltaFinding("a-first"); + const second = lowConfidenceDeltaFinding("z-second", { + path: "second.ts", + anchor: { path: "second.ts", line: 1, side: "RIGHT", hunkId: "h2" }, + category: "security", + evidence: { + changedCode: "+ return calculateSecondAmount(price, decimals)", + relatedCode: [{ path: "src/second-caller.ts", lines: "17: calculateSecondAmount(price, decimals)", whyRelevant: "A second caller reaches this changed conversion." }] + } + }); + const artifacts = new Map(); + const result = await dedupeRankAndComposeReview( + { + verified: [first, second], + verdicts: [qualifiedLowConfidenceVerdict(first.id), qualifiedLowConfidenceVerdict(second.id)] + }, + fakePlan(), + { mode: "branch", repoRoot: "/tmp/repo", commits: [], rawDiff: "" }, + fakeCoverage(), + { ...config(), review: { ...config().review, maxFindings: 1, softCommentCap: 100 } }, + { + ...nullTelemetry(), + writeArtifact: async (name, data) => { + artifacts.set(name, data); + } + }, + { + runner: { runStructured: async () => { throw composerTransientError(); } }, + promptBuilder: fakePromptBuilder(), + diff: fakeChangedLineDiff([ + { path: "app.ts", hunkId: "h1", line: 1, content: "return calculateAmountFromUSD(price, decimals)" }, + { path: "second.ts", hunkId: "h2", line: 1, content: "return calculateSecondAmount(price, decimals)" } + ]) + } + ); + + expect(result.findings).toEqual([expect.objectContaining({ id: first.id, publication: "inline" })]); + expect(artifacts.get("final-selection.json")).toMatchObject({ + records: expect.arrayContaining([ + expect.objectContaining({ findingId: second.id, decision: "suppressed", reason: "report-cap" }) + ]) + }); + }); + it("continues suppressing broad low-confidence findings even after verification", async () => { const artifacts = new Map(); const finding: CandidateFinding = { @@ -12650,6 +13015,138 @@ describe("phase 5 pipeline regressions", () => { expect(result.needsHumanAttention).toEqual([]); }); + it("renders the matching fallback note when a completed keep is suppressed by publication quality", async () => { + const base = verifierResolutionCandidate(); + const candidate: CandidateFinding = { + ...base, + confidence: "low", + anchorSource: "backfill_packet_representative", + failureMode: "Maybe this path changes.", + whyThisMatters: "A caller-visible fee may be wrong.", + suggestedTest: "Confirm the zero-price fee case." + }; + const artifacts = new Map(); + const events: Array> = []; + const result = await dedupeRankAndComposeReview( + { + verified: [candidate], + verdicts: [{ + candidateId: candidate.id, + verdict: "keep", + reason: "The predicate is confirmed, but the retained wording is still broad.", + requiredEvidencePresent: true, + falsePositiveRisk: "medium" + }] + }, + fakePlan("billing/fee.ts"), + { mode: "branch", repoRoot: "/tmp/repo", commits: [], rawDiff: "" }, + fakeCoverage(), + config(), + { + ...nullTelemetry(), + event: (event) => events.push(event), + writeArtifact: async (name, data) => { + artifacts.set(name, data); + } + }, + { + runner: { + runStructured: async () => ({ + summary: "Found one issue.", + composedFindings: [{ findingIds: [candidate.id], finalBody: "The fee predicate remains broad.", publication: "inline" }] + }) as T + }, + promptBuilder: fakePromptBuilder(), + packets: [verifierResolutionPacket()], + packetResults: [packetResultWithFindingAndHint(candidate, "billing/fee.ts")], + diff: fakeChangedLineDiff([{ path: "billing/fee.ts", hunkId: "h1", line: 12, content: "return calculateFee(input)" }]) + } + ); + + expect(result.findings).toEqual([]); + expect(result.summaryOnlyFindings).toEqual([]); + expect(result.needsHumanAttention).toEqual([ + expect.objectContaining({ + question: "Check whether normalizeAmount rejects zero prices before fee calculation.", + files: ["billing/fee.ts"] + }) + ]); + expect(artifacts.get("final-selection.json")).toMatchObject({ + records: [expect.objectContaining({ findingId: candidate.id, decision: "suppressed", reason: "confidence-threshold" })] + }); + expect(artifacts.get("human-attention-notes.json")).toMatchObject({ + outputNotes: [expect.objectContaining({ question: "Check whether normalizeAmount rejects zero prices before fee calculation." })], + fallbackGroupCount: 1, + omittedFallbackCount: 0, + publicationFallbacks: [expect.objectContaining({ candidateId: candidate.id, verdict: "keep" })] + }); + expect(events).toContainEqual(expect.objectContaining({ + message: "human_attention_publication_fallback", + data: expect.objectContaining({ fallbackCandidateIds: [candidate.id], fallbackGroupCount: 1, omittedFallbackCount: 0 }) + })); + }); + + it("publishes a fully concrete completed keep summary-only and suppresses its redundant note", async () => { + const base = verifierResolutionCandidate(); + const candidate: CandidateFinding = { + ...base, + confidence: "low", + anchorSource: "backfill_packet_representative", + failureMode: "The changed fee path accepts a zero price and produces an incorrect caller-visible fee instead of rejecting input.", + whyThisMatters: "A reachable billing caller can receive an invalid fee for a zero-price quote.", + suggestedTest: "Add a regression test that confirms zero-price input is rejected before fee calculation.", + verification: "Verifier confirmed the changed calculateFee path and the related normalizeAmount behavior." + }; + const artifacts = new Map(); + const result = await dedupeRankAndComposeReview( + { + verified: [candidate], + verdicts: [{ + candidateId: candidate.id, + verdict: "keep", + reason: "The concrete behavior delta and caller path are confirmed.", + requiredEvidencePresent: true, + falsePositiveRisk: "medium" + }] + }, + fakePlan("billing/fee.ts"), + { mode: "branch", repoRoot: "/tmp/repo", commits: [], rawDiff: "" }, + fakeCoverage(), + config(), + { + ...nullTelemetry(), + writeArtifact: async (name, data) => { + artifacts.set(name, data); + } + }, + { + runner: { + runStructured: async () => ({ + summary: "Found one issue.", + composedFindings: [{ findingIds: [candidate.id], finalBody: "The zero-price fee path remains reachable.", publication: "inline" }] + }) as T + }, + promptBuilder: fakePromptBuilder(), + packets: [verifierResolutionPacket()], + packetResults: [packetResultWithFindingAndHint(candidate, "billing/fee.ts")], + diff: fakeChangedLineDiff([{ path: "billing/fee.ts", hunkId: "h1", line: 12, content: "return calculateFee(input)" }]) + } + ); + + expect(result.summaryOnlyFindings).toEqual([ + expect.objectContaining({ id: candidate.id, publication: "summary-only", changedLine: false }) + ]); + expect(result.needsHumanAttention).toEqual([]); + expect(artifacts.get("final-selection.json")).toMatchObject({ + records: [expect.objectContaining({ findingId: candidate.id, decision: "published", reason: "low-confidence-anchorless" })] + }); + expect(artifacts.get("human-attention-notes.json")).toMatchObject({ + outputNotes: [], + fallbackGroupCount: 0, + omittedFallbackCount: 0 + }); + }); + it("suppresses human-attention notes resolved by verifier rejection with evidence", async () => { const artifacts = new Map(); const events: Array> = []; @@ -13527,6 +14024,39 @@ function fakeFinding(): CandidateFinding { }; } +function lowConfidenceDeltaFinding(id = "finding-1", overrides: Partial = {}): CandidateFinding { + return { + ...fakeFinding(), + id, + confidence: "low", + anchorSource: "model", + category: "correctness", + evidence: { + changedCode: "+ return calculateAmountFromUSD(price, decimals)", + relatedCode: [{ + path: "src/caller.ts", + lines: "42: calculateAmountFromUSD(price, decimals)", + whyRelevant: "The caller still reaches the changed conversion path." + }] + }, + failureMode: "The changed conversion path rejects a concrete token-decimal case that the previous implementation accepted.", + whyThisMatters: "A reachable caller can now fail a request that previously succeeded.", + suggestedTest: "Add a regression test for the changed conversion path with the affected decimal case.", + verification: "Verifier confirmed the changed line and related caller path; reproduce the affected case.", + ...overrides + }; +} + +function qualifiedLowConfidenceVerdict(candidateId: string): VerificationVerdict { + return { + candidateId, + verdict: "keep", + reason: "The decisive behavior delta is verified.", + requiredEvidencePresent: true, + falsePositiveRisk: "medium" + }; +} + function manyFindings(count: number): CandidateFinding[] { return Array.from({ length: count }, (_, index) => { const id = `finding-${String(index + 1)}`; diff --git a/tests/shared-utils.test.ts b/tests/shared-utils.test.ts index 87ee04a..968f7b5 100644 --- a/tests/shared-utils.test.ts +++ b/tests/shared-utils.test.ts @@ -100,6 +100,15 @@ describe("shared utility helpers", () => { expect(entry.evidence.trim()).not.toBe(""); } } + expect(PROMPT_TEMPLATE_WHY_LEDGER[9]).toEqual(expect.arrayContaining([ + expect.objectContaining({ evidence: expect.stringContaining("run 57 empty revise") }), + expect.objectContaining({ evidence: expect.stringContaining("run 55 secondary-budget cap") }), + expect.objectContaining({ evidence: expect.stringContaining("run 56 low-to-high inconsistency") }), + expect.objectContaining({ + surface: "bounded verifier repair candidate evidence", + evidence: expect.stringContaining("runs 58-60") + }) + ])); }); }); diff --git a/tests/uncertainty-promotion.test.ts b/tests/uncertainty-promotion.test.ts index 32f1b58..427160e 100644 --- a/tests/uncertainty-promotion.test.ts +++ b/tests/uncertainty-promotion.test.ts @@ -757,8 +757,158 @@ describe("uncertainty promotion", () => { expect(carried.length).toBeLessThanOrEqual(3); expect(new Set(carried.map((entry) => entry.path)).size).toBe(carried.length); }); + + it("retains run-57-shaped related output signals without changing selected candidates", async () => { + const fixture = relatedPromotionFixture(3, true); + const baseline = await promoteUncertaintiesForVerification({ + packets: fixture.packets.slice(0, 2), + packetResults: fixture.packetResults.slice(0, 2) + }, captureTelemetry().recorder); + const telemetry = captureTelemetry(); + const result = await promoteUncertaintiesForVerification(fixture, telemetry.recorder); + + const baselineCandidates = baseline.packetResults.flatMap((packetResult) => packetResult.findings); + const candidates = result.packetResults.flatMap((packetResult) => packetResult.findings); + expect(result.summary.promotedCandidateIds).toEqual(baseline.summary.promotedCandidateIds); + expect(candidates.map((candidate) => candidate.id)).toEqual(baselineCandidates.map((candidate) => candidate.id)); + expect(result.summary).toMatchObject({ + promoted: 2, + representedRelatedSignals: 3, + laneLimited: 1, + unrepresentedLaneLimited: 1 + }); + + const inputCandidate = candidates.find((candidate) => candidate.provenance?.question.includes("unauthorized token input")); + const outputCandidate = candidates.find((candidate) => candidate.provenance?.question.includes("exact output amount")); + const baselineOutputCandidate = baselineCandidates.find((candidate) => candidate.provenance?.question.includes("exact output amount")); + expect(inputCandidate?.provenance?.relatedSignals).toBeUndefined(); + expect(outputCandidate).toBeDefined(); + expect(outputCandidate?.confidence).toBe(baselineOutputCandidate?.confidence); + expect(outputCandidate?.evidence).toEqual(baselineOutputCandidate?.evidence); + expect(outputCandidate?.provenance).toMatchObject({ + question: "Verify whether scaleExactOutput now truncates the exact output amount and under-delivers the caller contract.", + crossPacketRelatedCount: 3, + relatedSignals: fixture.relatedQuestions.map((question, index) => expect.objectContaining({ + packetId: `packet-output-related-${String(index + 1)}`, + sourceKind: "follow_up_hint", + question, + files: ["src/amount.ts"], + symbols: ["scaleExactOutput"] + })) + }); + expect(outputCandidate?.provenance?.relatedSignals?.some((signal) => signal.question.includes("unauthorized token input"))).toBe(false); + expect(result.summary.decisions.filter((decision) => decision.reason === "represented_as_related_signal")).toHaveLength(3); + expect(result.summary.decisions.filter((decision) => decision.reason === "promotion_lane_limited")).toEqual([ + expect.objectContaining({ packetId: "packet-unmatched" }) + ]); + expect(telemetry.events).toContainEqual(expect.objectContaining({ + message: "uncertainty_promotion", + data: expect.objectContaining({ + promoted: 2, + representedRelatedSignals: 3, + unrepresentedLaneLimited: 1 + }) + })); + }); + + it("bounds related promotion provenance at eight and leaves overflow lane-limited", async () => { + const fixture = relatedPromotionFixture(10, false); + const result = await promoteUncertaintiesForVerification(fixture, captureTelemetry().recorder); + const outputCandidate = result.packetResults + .flatMap((packetResult) => packetResult.findings) + .find((candidate) => candidate.provenance?.question.includes("exact output amount")); + + expect(outputCandidate?.provenance?.relatedSignals).toHaveLength(8); + expect(outputCandidate?.provenance?.crossPacketRelatedCount).toBe(8); + expect(result.summary).toMatchObject({ + promoted: 2, + representedRelatedSignals: 8, + laneLimited: 2, + unrepresentedLaneLimited: 2 + }); + }); }); +function relatedPromotionFixture( + relatedCount: number, + includeUnmatched: boolean +): { packets: ReviewPacket[]; packetResults: PacketReviewResult[]; relatedQuestions: string[] } { + const exactInput = fakePacket("packet-exact-input", "src/input.ts", { + symbol: "decodeExactInput", + line: "+ return decodeExactInputStrict(value)" + }); + const exactOutput = fakePacket("packet-exact-output", "src/amount.ts", { + symbol: "scaleExactOutput", + line: "+ return Math.floor(scaleExactOutput(value))" + }); + const relatedPackets = Array.from({ length: relatedCount }, (_, index) => fakePacket( + `packet-output-related-${String(index + 1)}`, + "src/amount.ts", + { symbol: "scaleExactOutput", line: "+ return Math.floor(scaleExactOutput(value))" } + )); + const unmatched = fakePacket("packet-unmatched", "src/retry.ts", { + symbol: "retryRequest", + line: "+ return retryRequest(value)" + }); + const relatedQuestions = relatedPackets.map((_packet, index) => + `Confirm whether scaleExactOutput now truncates exact output amount framing ${String(index + 1)} and under-delivers the caller contract.` + ); + const packets = [exactInput, exactOutput, ...relatedPackets, ...(includeUnmatched ? [unmatched] : [])]; + const packetResults: PacketReviewResult[] = [ + promotionPacketResult( + exactInput, + "Verify whether decodeExactInput now allows unauthorized token input when the signature is missing.", + "The changed authorization validation contract can expose production callers.", + "high" + ), + promotionPacketResult( + exactOutput, + "Verify whether scaleExactOutput now truncates the exact output amount and under-delivers the caller contract.", + "The changed conversion precision can under-deliver the exact output promised to callers.", + "high" + ), + ...relatedPackets.map((packet, index) => promotionPacketResult( + packet, + relatedQuestions[index]!, + "The changed conversion truncation can under-deliver the exact output promised to callers.", + "medium" + )), + ...(includeUnmatched + ? [promotionPacketResult( + unmatched, + "Verify whether retryRequest now loses the timeout fallback and breaks the caller contract.", + "The changed retry fallback can fail production callers.", + "medium" + )] + : []) + ]; + return { packets, packetResults, relatedQuestions }; +} + +function promotionPacketResult( + packet: ReviewPacket, + question: string, + reason: string, + confidence: "medium" | "high" +): PacketReviewResult { + return { + packetId: packet.id, + lenses: ["core/code-review"], + findings: [], + followUpHints: [{ + question, + files: [packet.path], + symbols: [packet.symbolFacts[0]!.enclosingSymbol!], + suggestedLenses: ["core/code-review"], + reason, + confidence, + projectedSkillIds: ["core/code-review"] + }], + uncertainties: [], + status: "completed" + }; +} + function fakePacket( id: string, filePath: string, diff --git a/tests/verifier.test.ts b/tests/verifier.test.ts index b2d555f..7df3630 100644 --- a/tests/verifier.test.ts +++ b/tests/verifier.test.ts @@ -725,6 +725,13 @@ describe("stage 9 eval diagnostics and prompts", () => { expect(verifierPrompt).toContain("caller-visible"); expect(verifierPrompt).toContain("transformed value"); expect(verifierPrompt).toContain("original source value"); + expect(verifierPrompt).toContain("A bare keep means"); + expect(verifierPrompt).toContain("a revision must include finalFinding or revisedAnchor"); + expect(verifierPrompt).toContain("Tool refusal, truncation, or budget pressure on a secondary check must not keep confidence low"); + expect(verifierPrompt).toContain("Reserve low confidence for speculative reachability, ambiguous intent, or weak path matching"); + expect(verifierPrompt).toContain("low means bounded or localized impact"); + expect(verifierPrompt).toContain("Measure magnitude and reach"); + expect(verifierPrompt).toContain("changing severity by more than one level"); }); }); @@ -938,6 +945,481 @@ function captureTelemetry(): { }; } +describe("plan 106 verifier revision semantics", () => { + it("canonicalizes legacy keep payloads to revise while preserving both payload kinds", async () => { + const fixture = reviewFixture(["src/app.ts"]); + const packet = fixture.packets[0]!; + const finding = candidate("legacy-keep-payload", packet); + const telemetry = captureTelemetry(); + + const result = await verifyFindings( + { packetResults: [packetResult(packet.id, [finding])], packets: fixture.packets }, + fakeTools(), + config(), + telemetry.recorder, + { + runner: verifierRunner(() => ({ + verdict: "keep", + reason: "Legacy provider changed the finding while saying keep.", + requiredEvidencePresent: true, + falsePositiveRisk: "low", + finalFinding: { + title: "Calibrated legacy finding", + severity: "medium", + confidence: "medium", + path: packet.path, + category: "correctness", + evidence: { changedCode: "+ return route(provider);" }, + failureMode: finding.failureMode, + whyThisMatters: finding.whyThisMatters, + verification: "The decisive changed branch confirms the failure mode." + }, + revisedAnchor: { path: packet.path, line: 2, side: "RIGHT", hunkId: packet.hunks[0]!.hunkId } + })), + promptBuilder: createPromptBuilder(fakeLensRegistry()), + lensRegistry: fakeLensRegistry(), + diff: fixture.diff + } + ); + + expect(result.verdicts[0]).toMatchObject({ verdict: "revise", finalFinding: { title: "Calibrated legacy finding" } }); + expect(result.verified[0]).toMatchObject({ title: "Calibrated legacy finding", confidence: "medium", anchorSource: "verifier_revised" }); + expect(telemetry.events).toContainEqual(expect.objectContaining({ + message: "verification_keep_payload_canonicalized", + data: { candidateId: finding.id, payloadKinds: ["finalFinding", "revisedAnchor"] } + })); + }); + + it("rejects a canonicalized keep payload when required evidence is missing", async () => { + const fixture = reviewFixture(["src/app.ts"]); + const packet = fixture.packets[0]!; + const finding = candidate("legacy-keep-missing-evidence", packet); + const telemetry = captureTelemetry(); + + const result = await verifyFindings( + { packetResults: [packetResult(packet.id, [finding])], packets: fixture.packets }, + fakeTools(), + config(), + telemetry.recorder, + { + runner: verifierRunner(() => ({ + verdict: "keep", + reason: "Legacy payload is not backed by required evidence.", + requiredEvidencePresent: false, + falsePositiveRisk: "high", + finalFinding: { + title: "Unsupported legacy revision", + severity: "medium", + confidence: "medium", + path: packet.path, + category: "correctness", + evidence: { changedCode: "+ return route(provider);" }, + failureMode: finding.failureMode, + whyThisMatters: finding.whyThisMatters, + verification: "The decisive predicate was not confirmed." + } + })), + promptBuilder: createPromptBuilder(fakeLensRegistry()), + lensRegistry: fakeLensRegistry(), + diff: fixture.diff + } + ); + + expect(result.verified).toEqual([]); + expect(result.verdicts[0]).toMatchObject({ + verdict: "reject", + requiredEvidencePresent: false, + falsePositiveRisk: "high", + reason: expect.stringContaining("required evidence missing; original keep verdict rejected") + }); + expect(telemetry.events).toEqual(expect.arrayContaining([ + expect.objectContaining({ + message: "verification_keep_payload_canonicalized", + data: { candidateId: finding.id, payloadKinds: ["finalFinding"] } + }), + expect.objectContaining({ + message: "verification_missing_evidence_normalized_to_reject", + data: expect.objectContaining({ candidateId: finding.id, originalVerdict: "keep" }) + }) + ])); + expect(telemetry.events).not.toEqual(expect.arrayContaining([ + expect.objectContaining({ message: "verification_severity_revision" }) + ])); + }); + + it("leaves a bare keep unchanged", async () => { + const fixture = reviewFixture(["src/app.ts"]); + const packet = fixture.packets[0]!; + const finding = candidate("bare-keep", packet); + const telemetry = captureTelemetry(); + + const result = await verifyFindings( + { packetResults: [packetResult(packet.id, [finding])], packets: fixture.packets }, + fakeTools(), + config(), + telemetry.recorder, + { + runner: verifierRunner(() => ({ + verdict: "keep", + reason: "The candidate is publishable unchanged.", + requiredEvidencePresent: true, + falsePositiveRisk: "low" + })), + promptBuilder: createPromptBuilder(fakeLensRegistry()), + lensRegistry: fakeLensRegistry(), + diff: fixture.diff + } + ); + + expect(result.verdicts[0]?.verdict).toBe("keep"); + expect(result.verdicts[0]?.severityRevision).toBeUndefined(); + expect(result.verified[0]?.title).toBe(finding.title); + expect(telemetry.events).not.toEqual(expect.arrayContaining([ + expect.objectContaining({ message: "verification_keep_payload_canonicalized" }), + expect.objectContaining({ message: "verification_severity_revision" }) + ])); + expect(telemetry.events).toContainEqual(expect.objectContaining({ + message: "verification_primary_submit_accepted", + data: { + candidateId: finding.id, + submitTool: "submit_verdict", + schemaVersion: 3, + argumentsNonEmpty: true, + schemaRepairUsed: false + } + })); + }); + + it("treats null keep payloads from a non-validating adapter as absent", async () => { + const fixture = reviewFixture(["src/app.ts"]); + const packet = fixture.packets[0]!; + const finding = candidate("null-keep-payloads", packet); + const telemetry = captureTelemetry(); + + const result = await verifyFindings( + { packetResults: [packetResult(packet.id, [finding])], packets: fixture.packets }, + fakeTools(), + config(), + telemetry.recorder, + { + runner: verifierRunner(() => ({ + verdict: "keep", + reason: "The unchanged candidate is supported.", + requiredEvidencePresent: true, + falsePositiveRisk: "low", + finalFinding: null, + revisedAnchor: null + })), + promptBuilder: createPromptBuilder(fakeLensRegistry()), + lensRegistry: fakeLensRegistry(), + diff: fixture.diff + } + ); + + expect(result.verdicts[0]).toMatchObject({ verdict: "keep" }); + expect(result.verified).toEqual([finding]); + expect(telemetry.events).not.toEqual(expect.arrayContaining([ + expect.objectContaining({ message: "verification_keep_payload_canonicalized" }), + expect.objectContaining({ message: "verification_semantic_invalid" }) + ])); + }); + + it("persists an empty revise from a non-validating adapter as incomplete", async () => { + const fixture = reviewFixture(["src/app.ts"]); + const packet = fixture.packets[0]!; + const finding = candidate("empty-revise-defense", packet); + const telemetry = captureTelemetry(); + + const result = await verifyFindings( + { packetResults: [packetResult(packet.id, [finding])], packets: fixture.packets }, + fakeTools(), + config(), + telemetry.recorder, + { + runner: verifierRunner(() => ({ + verdict: "revise", + reason: "Changed only in prose.", + requiredEvidencePresent: true, + falsePositiveRisk: "low" + })), + promptBuilder: createPromptBuilder(fakeLensRegistry()), + lensRegistry: fakeLensRegistry(), + diff: fixture.diff + } + ); + + expect(result.verified).toEqual([]); + expect(result.incompleteCount).toBe(1); + expect(result.verdicts[0]).toMatchObject({ + verdict: "incomplete", + verificationIncomplete: true, + reason: "verification incomplete: revise_without_revision_payload" + }); + expect(telemetry.events).toContainEqual(expect.objectContaining({ + message: "verification_semantic_invalid", + data: { candidateId: finding.id, reason: "revise_without_revision_payload" } + })); + expect(telemetry.events).not.toEqual(expect.arrayContaining([ + expect.objectContaining({ message: "verification_primary_submit_accepted" }) + ])); + }); + + it("treats null revise payloads from a non-validating adapter as empty", async () => { + const fixture = reviewFixture(["src/app.ts"]); + const packet = fixture.packets[0]!; + const finding = candidate("null-revise-payloads", packet); + const telemetry = captureTelemetry(); + + const result = await verifyFindings( + { packetResults: [packetResult(packet.id, [finding])], packets: fixture.packets }, + fakeTools(), + config(), + telemetry.recorder, + { + runner: verifierRunner(() => ({ + verdict: "revise", + reason: "No structured revision was actually supplied.", + requiredEvidencePresent: true, + falsePositiveRisk: "low", + finalFinding: null, + revisedAnchor: null + })), + promptBuilder: createPromptBuilder(fakeLensRegistry()), + lensRegistry: fakeLensRegistry(), + diff: fixture.diff + } + ); + + expect(result.verified).toEqual([]); + expect(result.verdicts[0]).toMatchObject({ + verdict: "incomplete", + verificationIncomplete: true, + reason: "verification incomplete: revise_without_revision_payload" + }); + expect(telemetry.events).toContainEqual(expect.objectContaining({ + message: "verification_semantic_invalid", + data: { candidateId: finding.id, reason: "revise_without_revision_payload" } + })); + }); +}); + +describe("plan 108 verifier severity revision observability", () => { + const cases = [ + { name: "decrease", original: "high", submitted: "medium", applied: "medium", deltaLevels: -1, level: "debug" }, + { name: "unchanged", original: "medium", submitted: "medium", applied: "medium", deltaLevels: 0, level: "debug" }, + { name: "one-level increase", original: "low", submitted: "medium", applied: "medium", deltaLevels: 1, level: "debug" }, + { name: "two-level increase", original: "low", submitted: "high", applied: "high", deltaLevels: 2, level: "info" }, + { + name: "behavior-change cap", + original: "medium", + submitted: "high", + applied: "medium", + deltaLevels: 1, + level: "debug", + behaviorChange: "intentional_needs_confirmation" + } + ] as const; + + for (const testCase of cases) { + it(`persists and emits a ${testCase.name}`, async () => { + const fixture = reviewFixture(["src/severity.ts"]); + const packet = fixture.packets[0]!; + const finding = candidate(`severity-${testCase.name.replaceAll(" ", "-")}`, packet, { + severity: testCase.original + }); + const telemetry = captureTelemetry(); + + const result = await verifyFindings( + { packetResults: [packetResult(packet.id, [finding])], packets: fixture.packets }, + fakeTools(), + config(), + telemetry.recorder, + { + runner: verifierRunner(() => ({ + verdict: "revise", + reason: "The issue is real with calibrated impact.", + requiredEvidencePresent: true, + falsePositiveRisk: "low", + ...("behaviorChange" in testCase ? { behaviorChange: testCase.behaviorChange } : {}), + finalFinding: { + title: "Calibrated severity finding", + severity: testCase.submitted, + confidence: "high", + path: packet.path, + category: "correctness", + evidence: { changedCode: "+ return route(provider);" }, + failureMode: finding.failureMode, + whyThisMatters: finding.whyThisMatters, + verification: "The concrete impact is bounded to callers of this changed route." + } + })), + promptBuilder: createPromptBuilder(fakeLensRegistry()), + lensRegistry: fakeLensRegistry(), + diff: fixture.diff + } + ); + + const expectedRevision = { + original: testCase.original, + submitted: testCase.submitted, + applied: testCase.applied, + deltaLevels: testCase.deltaLevels + }; + expect(result.verdicts[0]?.severityRevision).toEqual(expectedRevision); + expect(result.verified[0]?.severity).toBe(testCase.applied); + expect(telemetry.events).toContainEqual(expect.objectContaining({ + stage: 9, + level: testCase.level, + message: "verification_severity_revision", + data: expect.objectContaining({ + candidateId: finding.id, + category: "correctness", + ...expectedRevision, + ...("behaviorChange" in testCase ? { behaviorChange: testCase.behaviorChange } : {}) + }) + })); + + const records = telemetry.artifacts.get("verification.json") as EvalVerificationRecord[]; + expect(records[0]).toMatchObject({ + candidateId: finding.id, + verdict: { severityRevision: expectedRevision } + }); + if ("behaviorChange" in testCase) { + expect(result.verified[0]).toMatchObject({ + behaviorChange: "intentional_needs_confirmation", + severity: "medium", + severityBeforeCap: "high" + }); + } else { + expect(result.verified[0]?.severityBeforeCap).toBeUndefined(); + } + }); + } + + it("does not record severity revision telemetry for reject compatibility payloads", async () => { + const fixture = reviewFixture(["src/severity.ts"]); + const packet = fixture.packets[0]!; + const finding = candidate("reject-severity-payload", packet, { severity: "low" }); + const telemetry = captureTelemetry(); + + const result = await verifyFindings( + { packetResults: [packetResult(packet.id, [finding])], packets: fixture.packets }, + fakeTools(), + config(), + telemetry.recorder, + { + runner: verifierRunner(() => ({ + verdict: "reject", + reason: "The candidate is unsupported despite the compatibility payload.", + requiredEvidencePresent: false, + falsePositiveRisk: "high", + finalFinding: { + title: "Rejected compatibility payload", + severity: "high", + confidence: "medium", + path: packet.path, + category: "correctness", + evidence: { changedCode: "+ return route(provider);" }, + failureMode: finding.failureMode, + whyThisMatters: finding.whyThisMatters, + verification: "The decisive predicate was not confirmed." + } + })), + promptBuilder: createPromptBuilder(fakeLensRegistry()), + lensRegistry: fakeLensRegistry(), + diff: fixture.diff + } + ); + + expect(result.verified).toEqual([]); + expect(result.verdicts[0]).toMatchObject({ verdict: "reject" }); + expect(result.verdicts[0]?.severityRevision).toBeUndefined(); + expect(telemetry.events).not.toEqual(expect.arrayContaining([ + expect.objectContaining({ message: "verification_severity_revision" }) + ])); + const records = telemetry.artifacts.get("verification.json") as EvalVerificationRecord[]; + expect(records[0]).toMatchObject({ candidateId: finding.id, verdict: { verdict: "reject" } }); + expect((records[0] as Extract).verdict).not.toHaveProperty("severityRevision"); + }); +}); + +describe("plan 107 related promotion signal handoff", () => { + it("keeps related signals inside candidate provenance without changing verifier resources or primary provenance", async () => { + const fixture = reviewFixture(["src/amount.ts"]); + const packet = fixture.packets[0]!; + const primaryQuestion = "Verify whether scaleExactOutput now truncates the exact output amount."; + const relatedQuestion = "Confirm whether the exact output amount can under-deliver the caller contract."; + const finding = candidate("related-signal-handoff", packet, { + producedBy: { + kind: "packet", + stage: 9, + packetId: packet.id, + lensId: "shared/review", + skillIds: ["projection/neutral"] + }, + provenance: { + source: "uncertainty_promotion", + sourceKind: "follow_up_hint", + sourcePacketId: packet.id, + question: primaryQuestion, + files: [packet.path], + symbols: ["scaleExactOutput"], + reason: "Primary promoted predicate.", + relatedSignals: [{ + packetId: "packet-related", + sourceKind: "follow_up_hint", + question: relatedQuestion, + files: [packet.path], + symbols: ["scaleExactOutput"] + }], + crossPacketRelatedCount: 1 + } + }); + const registry = registryWithSkills([projectionSkill("neutral", [], "RELATED_SIGNAL_SKILL_MARKER")]); + let request: LlmStructuredRequest | undefined; + + await verifyFindings( + { packetResults: [packetResult(packet.id, [finding])], packets: fixture.packets }, + fakeTools(), + config(), + captureTelemetry().recorder, + { + runner: { + runStructured: async (input: LlmStructuredRequest) => { + request = input; + return { + verdict: "keep", + reason: "The primary predicate is confirmed unchanged.", + requiredEvidencePresent: true, + falsePositiveRisk: "low" + } as T; + } + }, + promptBuilder: createPromptBuilder(registry), + lensRegistry: registry, + diff: fixture.diff + } + ); + + expect(request?.toolBudget).toEqual({ + maxToolCalls: 8, + maxInvestigationRounds: 3, + maxResultChars: 16_000, + maxSingleToolResultChars: 6_000, + reservedSourceResultChars: 4_000, + sourceExtension: { maxToolCalls: 2, maxResultChars: 8_000 } + }); + expect(request?.prompt).toContain("RELATED_SIGNAL_SKILL_MARKER"); + expect(request?.prompt).toContain(`\"question\": \"${primaryQuestion}\"`); + expect(request?.prompt).toContain("\"relatedSignals\""); + expect(request?.prompt).toContain("\"crossPacketRelatedCount\": 1"); + expect(request?.prompt?.split(relatedQuestion)).toHaveLength(2); + const candidateBlock = /untrusted-data label=candidate-finding\n(?[\s\S]*?)\n`{4,}/u.exec(request?.prompt ?? "")?.groups?.body; + expect(candidateBlock).toContain(relatedQuestion); + expect(candidateBlock).toContain(primaryQuestion); + }); +}); + describe("plan 76 anchor reconstruction", () => { it("tier 1: reconstructs a precise anchor from quoted changed code with whitespace variance", () => { const fixture = reviewFixture(["src/app.ts"]); @@ -1127,7 +1609,7 @@ describe("plan 76 anchor reconstruction", () => { captureTelemetry().recorder, { runner: verifierRunner(() => ({ - verdict: "keep", + verdict: "revise", reason: "Confirmed; anchor refined to the changed line.", requiredEvidencePresent: true, falsePositiveRisk: "low",