Skip to content

feat(review): add advisory finding utility contract - #614

Draft
zzwong wants to merge 5 commits into
mainfrom
zzwong/finding-utility-contract
Draft

zzwong wants to merge 5 commits into
mainfrom
zzwong/finding-utility-contract

Conversation

@zzwong

@zzwong zzwong commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add the pure, advisory-only contract for evaluating code-review finding utility. This foundation defines typed state and question schemas, deterministic fail-safe policy behavior, canonical provenance, duplicate handling, and verifiable audit records without connecting the feature to the review pipeline.

Changes

  • define the six-object evaluator state, 13 binary judgments, utility and duplicate choices, and optional diagnostic score
  • add deterministic guards that keep protected, incomplete, uncalibrated, unaudited, or otherwise uncertain findings
  • preserve raw findings and enforce effective_decision: keep for every advisory record
  • add canonical digest, duplicate-finalization, artifact-integrity, and symlink-containment checks
  • add synthetic fixtures and focused regression tests for protected risk, missing context, strict thresholds, malformed responses, duplicate cycles, prompt injection, and label leakage

Testing

  • go test ./internal/findingutility -count=1
  • go test -race ./internal/findingutility -count=1
  • all JSON fixtures validated with jq
  • go build ./cmd/cr
  • git diff --check

The full repository test run reached the existing installed-Pi integration test and failed because the host selected Node 18, while the installed Pi bundle imports node:fs.globSync. The package under this PR and the remaining Go packages passed. Repository lint also could not type-check Go 1.27's standard-library math/rand/v2 with the installed linter.

Scope limits

This PR does not add a provider transport, network or credential access, calibrated thresholds, production suppression, pipeline integration, CLI flags, ledger changes, or a development binary. Exported runtime policy remains non-applying; only private test helpers exercise suppression signatures.

Fresh-head hosted verification (2026-10-08)

  • Refreshed the existing draft branch by a non-force forward merge of main 999e59ddfc74a67ffeef3c4b4a78a8528365afe6. New head: 078c8ec47cf5bfeaacf3c05025b4d5adce470331. All 20 finding-utility files retain their original bytes and modes; all 409 current-main leaves are preserved. Advisory-only scope is unchanged.
  • Normal PR CI run 37852059518 passed all 10 jobs: normal and opt-out tests, static smoke, lint, Linux/macOS/Windows builds, aggregate build, identity and PR title. The tested merge 7ca8cd4de44230626eec7d7a1f1ef721085ed8a9 has the exact proposed tree ca02512ecfaea0bffabfd6fd728815ee0aec8969.
  • The normal test log confirms TestPiRPCReviewerHelperStaysInParentProcessGroup passed in 0.35 seconds without skipping, and internal/findingutility passed in 0.224 seconds, using Go 1.26.8 on Linux amd64.
  • Preserve the original failed run 35964315353: its helper PID readiness check timed out within three seconds. Main already contains PR 643's startup-budget correction. No old workflow was rerun or manually dispatched. The earlier local Node and lint limitations remain recorded above.

This PR remains a draft. No PR merge, release or production activation was performed.

@zzwong zzwong changed the title Add advisory finding utility contract feat(review): add advisory finding utility contract Sep 18, 2026
Rename the Noul question type to Binary and the jev fixture profile to
evaluator-advisory-fixture-v1, so the package describes finding-utility
judgments without tying cr's core types to one evaluator vendor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant