[binaryaaron]

Rewrite's repair and human-review decisions are now named findings, and each risk-tolerance preset lists, as data, which findings send a record to repair and which flag it for review. On the way, a record whose evaluation metrics came back missing or NaN now gets flagged instead of passing review.

Why

The rules were inline arithmetic: determine_repair_needs had two hard-coded triggers and the needs_human_review formula was written twice (staged path, combined graph), each with its own coercions. Adding a condition, as #328 did with replacement_unavailable, meant a new clause in two places with nothing forcing a test or preset entry. The coercions also hid a bug: float(nan) > threshold is False, so a NaN leakage_mass or utility_score passed, and None raised.

What changes

• Finding is an enum in config/rewrite.py, next to the preset table: rewrite_missing, replacement_unavailable, high_leak, leakage_above_repair, leakage_above_flag, utility_below_flag, unavailable_evidence. The two leakage findings are named after the preset fields they read.
• engine/rewrite/findings.py (60 lines): present_findings(row, evaluation) -> frozenset[Finding] reads each metric once as float | None, so the type checker enforces the missing-value guard; needs_repair and needs_human_review are the overlap with the preset's repair_on and flag_on. The repair column and both review sites call these; the duplicated formula and its parameter models are deleted.
• Each _RiskToleranceBundle row spells out repair_on: tuple[RepairFinding, ...] and flag_on. RepairFinding is a Literal of the findings repair can act on (high_leak, leakage_above_repair); listing anything else for repair fails the type check. Thresholds, weights and today's behaviour are unchanged: minimal, low, moderate repair on high_leak or leakage_above_repair, high on leakage_above_repair only, every preset flags the same set. EvaluationCriteria gains read-only repair_on and flag_on; Rewrite, RiskTolerance and the public exports are unchanged; config/rewrite.py imports nothing from the engine.
• unavailable_evidence (any of leakage_mass, utility_score, any_high_leaked missing or NaN) is in every preset's flag set and never triggers repair; missing metrics no longer raise.
• docs/concepts/rewrite.md ("Working with flagged records") names the findings and the unavailable-evidence flag. The bundled skill is unaffected.

Not here: the confirmed-leak finding from #358 ("any privacy question answered yes"; a one-line computation plus a preset entry) lands with the preset decision on #358.

About +43 source lines net (+122/-79), including the new module.

Checks

• ruff format --check, ruff check and ty pass; make test: 1,607 passed.
• tests/engine/test_findings.py: a table-driven test per finding with threshold cases and NaN, None and missing keys; a test that every finding holds in some case and every preset names only known findings, with unavailable_evidence in each flag set and no repair set; an equivalence grid running the old formulas, copied verbatim from main, against the new predicates for every preset across text, replacement readiness, high-leak, utility and leakage values. Outcomes match exactly, exceptions included, except unavailable-evidence rows, now flagged and never repaired.
• Commit 3 against commit 2: 134,400 preset-by-row decisions, 0 differences, 0 exceptions.
• Staged path: RewriteWorkflow.run() with a NaN leakage_mass flags the record and leaves the rest of the frame identical. Combined path: the finalizer flags NaN and None leakage. Both fail without the fix commit.

For reviewers

• Targeted tests: tests/engine/test_findings.py, plus the unavailable cases in tests/engine/test_rewrite_workflow.py and tests/engine/test_combined_rewrite_workflow.py.
• One place the two old formulas disagreed: combined checked the rewritten text with is None, staged with isna(). The shared rule uses isna(); they differ only for a float-NaN rewrite, which the combined graph does not produce.
• Quirk kept because the grid pins it: a NaN any_high_leaked counts as high_leak (bool(nan) is True), None does not.
• To change a preset, edit repair_on or flag_on in _RISK_TOLERANCE_BUNDLES. To add a condition, add a Finding and one line in present_findings; the coverage test fails until both exist.
• fix(interface): flag rows whose coverage judge finds missed entities #356 flags missed_entities after evaluate() and could become a finding later; no dependency. feat(rewrite): classify entity labels and enforce sensitivity policy #333 touches the same two workflow files; a trial merge of its head onto this branch has no conflict markers.

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; the latest documentation and test agree with the existing rules.

Summary

The PR puts rewrite repair and review rules in shared named findings and flags missing evaluation metrics for human review.

• Risk presets now say which rewrite findings trigger repair and review.
• Records with missing evaluation evidence now get flagged for human review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Evaluated record] --> B[present_findings]
  B --> C[Preset repair_on]
  B --> D[Preset flag_on]
  C --> E[Repair decision]
  D --> F[Human review decision]

Loading

_{Reviews (4) · Last reviewed commit: "docs(rewrite): name the NaN high-leak ex..." · Reviewed by Greptile}

[andreatnvidia]

Looks good to me. Having the preset table drive both repair and review makes future policy changes easier to follow.

One small clarification: the docs say unavailable evidence doesn’t trigger repair, but any_high_leaked=NaN still produces high_leak through bool(nan) and triggers repair for minimal/low/moderate. Keeping that behavior for compatibility makes sense. Could we mention the exception beside that sentence and add an explicit NaN case to the finding-rule table? The equivalence grid covers it, but a named case would make the intent easier to preserve.

When this moves onto the SDK branch, let’s also carry the unavailable-utility check into its final review projection so it doesn’t overwrite the flag from the combined finalizer.

[binaryaaron]

agent (review-pr): Pushed 7c33e06, which names the NaN exception without changing behaviour. In docs/concepts/rewrite.md, the sentence "Unavailable evidence flags a record but does not trigger repair." is now followed by a note that a NaN any_high_leaked also counts as high_leak and so triggers repair under every preset except high. tests/engine/test_findings.py adds test_nan_any_high_leaked_is_high_leak_and_repairs_except_high, parametrized per preset: it asserts high_leak and unavailable_evidence are both present, repair for minimal/low/moderate, no repair for high, and review in all cases. Carrying the unavailable-utility check into the final review projection is left as a follow-up for the SDK line.