[asteier2026]

Related Documentation

More information about the problem, solution and testing that was done can be found here.

Summary

Sensitivity disposition previously asked one model call to classify labels, assess risk, and echo entity identities without enforcing complete inventory coverage. This change resolves default labels locally, classifies custom explicit labels separately, and requires exactly one decision for each supplied explicit or latent entity.

• Add an authoritative direct/quasi category table and an entity_classifier role. Skip the classifier call when no custom labels are present; omitted role selections reuse the effective disposition analyzer. Local categories cannot be overridden by the model.
• Define contextual protection rules: direct identifiers are high; low sensitivity requires leave_as_is and an explanation; protected latent entities use suppress_inference. Remove the separate combined-risk field.
• Assign stable request-local IDs per explicit (value, label) pair and latent entry. Request compact decisions containing only ID, sensitivity, method, and low-sensitivity reason; reconstruct identity and category from the supplied inventory. Reject incomplete, duplicate, or reordered IDs.
• The sensitivity prompt wording was significantly changed.
• The domain supplement was removed from the sensitivity disposition. That was a mistake to include it since the current supplements are primarily for Privacy QA.
• The data_summary was removed from the sensitivity disposition. This was in an effort to lighten the prompt. This param's originally purpose (to summarize the nature of the data) serves no purpose here. PrivacyGoal was left in. Sometimes, however, we've used data_summary to do the job of the PrivacyGoal and specify the nature of what needs protecting and what needs preserving. The issue is Replace mode doesn't have a PrivacyGoal. We should possibly pursue that issue independently of this PR.
• In order to handle docs with very large quantities of entities, the initial output of the sensitivity disposition prompt was streamlined and rebuilt after.
• Use the same DataDesigner constraint as main (>=0.9.4,<0.10).
• A new model for the entity classification needs to be assigned in the config. For now if it is not present, it falls back and uses the same model as the sensitivity disposition prompt
• Evaluation was done on which models would be best for these prompts and the decision is nemotron-3.5-super-vl for the sensitivity disposition and gemma-4-26b-a4b-it for the entity classification. These updates will be made when we make the large shift to the full suite of models Marjan found best.
• Note Codex does not think this PR conflicts with PR refactor(detection): establish deterministic entity candidate lifecycle #305. The observation IDs created in refactor(detection): establish deterministic entity candidate lifecycle #305 serve a different purpose and don’t replace the disposition-ID assignments in this PR.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Refactoring
• CI, release, or contributor workflow update

Contributor Checklist

• PR title follows Conventional Commits
• Maintainer-owned no-issue reason is documented above
• Plan document is linked above
• Public API impact checked; the new model role has a backward-compatible fallback and requires no bundled skill-template changes
• No real PII added to tests, docs, notebooks, fixtures, or artifacts
• No API keys, service tokens, private keys, credentials, or real endpoint secrets added

Validation

• Full unit suite on this branch: 1,477 passed (three dependency deprecation warnings).
• make check: passed (format, lint, type checking, lockfile, copyright).
• make docs-build: passed (strict mode).
• Replayed 608 decisions from eight exported agent traces through compact reconstruction: normalized outputs were identical; serialized raw payloads were 48% smaller.
• Manual ten-trace compact-output run returned ten records, compared with eight in the earlier run. Detection inventories differ, so this is not an isolated model-quality comparison. Placeholder-heavy repair output and a remaining medium privacy judgment warrant further evaluation.
• Tests cover strict/default response schemas, authoritative identity and category reconstruction, Unicode values, full coverage, invalid IDs, policy consistency, classifier fallback/skip/merge behavior, Parquet serialization, scheduler propagation, and both workflow paths.
• No fresh end-to-end provider calls were made for PR preparation. Validation used a separate environment with local-inference dependencies installed for this branch's type checks.

Documentation and Artifacts

• Model selection and rewrite documentation updated
• Strict docs build passes locally
• Tutorial sources unchanged; no notebook regeneration required.
• Evaluation CSVs, personal notebooks, notes, and local model configs are not included.

[greptile-apps]

Confidence Score: 5/5

[High impact] The PR appears safe to merge; the skill-test response mismatch is fixed.

Summary

The PR separates entity classification from sensitivity decisions and reconstructs entity identities from stable IDs.

• Rewrite mode classifies entity labels before making privacy decisions.
• Every explicit and latent entity gets a stable place in the disposition.
• Disposition decisions tie each sensitivity rating to a protection method.

_{Reviews (18) · Last reviewed commit: "fix(tests): return compact dispositions ..." · Reviewed by Greptile}

[asteier2026]

The DataDesigner constraint matches current main (>=0.9.4,<0.10). This PR does not intend to change the supported DataDesigner versions. It is mentioned above in the bot summary because I did actually accidentally change it at one point and then reversed the change.

[binaryaaron]

Some recorded-run evidence for verify_disposition_coverage (sensitivity_disposition.py:292-312 at c5d4438), offered as test material rather than a blocker. I am closing #353 in favour of this PR, so this is where the coverage question lives now.

Short count. On one input row (27 detected entities plus latent ones), the disposition model left out a detected occupation entity in 2 of 8 runs at max_tokens 16384; in one of those it returned 30 entries for 31 supplied. Your length check raises on that, as intended. At max_tokens 4096, 9 of 10 recorded dispositions were short, by 4 to 17 detected entities each, and in every case the uncovered values, first and last names among them, survived the rewrite with no review flag.

Two questions:

1. Drop or flag? The ValueError makes DataDesigner skip the record, so it surfaces as a failed record. At small max_tokens that could drop most of a batch, or trip early shutdown. Is dropping the intended behaviour, or would you rather emit the row with needs_human_review set?
2. Misalignment. With the compact output, an equal-count response is accepted by construction, since identity comes from the ID. In the other short run, the model returned 30 of 30 entries with one value duplicated and the occupation value missing. In the compact format that shape would surface as decisions attached to the wrong IDs, which nothing downstream can detect. That may be an acceptable trade for the smaller output; if so, a sentence in the plan's tradeoffs section would record it.

The compact format cuts about 55% of tokens per entry on the recorded dispositions (106 to 48 mean), which matches your figure. The recorded omissions, though, happened well below the output ceiling (30 entries at 16384, 5-18 at 4096), so the format alone may not remove them, as the plan already notes.

If useful, I can add a parametrized test to tests/engine/test_sensitivity_disposition.py: inventory as (value, label) pairs plus the model's compact entries, no document text; the short-count case expects ValueError, and the equal-count case expects whichever behaviour you choose.

[asteier2026]

Thanks—this is useful regression material. The intended behavior for incomplete coverage is currently to fail the record rather than continue with partial protection decisions. Returning a row with needs_human_review would require a defined fallback, which this change doesn’t add. I agree that compact output cannot detect a decision assigned to the wrong otherwise-valid ID; I’ll document that limitation. The recorded short-count fixture would be useful, and the equal-count fixture can document acceptance by ID without implying semantic correctness.

I added all the necessary tests.

[binaryaaron]

agent (review-pr): Review of head 041a260, checked on the head and on a local merge with current main (7605c13). The conflict with main is limited to the import block in tests/engine/test_rewrite_workflow.py: keep both sides (COL_QUALITY_QA_COMPARE, COL_QUALITY_QA_REANSWER, COL_RAW_SENSITIVITY_DISPOSITION). With that resolved, the head passes its suite, but the merge has five new test failures, from two semantic conflicts with main.

Breaks after merging with main

1. A rewrite run crashes when no entity in the batch is rated low. With fix(ndd): preserve nested JSON storage across workflow handoffs #375's nested JSON storage, _sensitivity_disposition comes back as an Arrow struct. When low_sensitivity_reason is null in every entry, df.to_dict("records") in _run_evaluate_repair_loop raises ArrowIndexError: index with value of 0 is out-of-bounds for array of length 0. The error is not caught, so the whole run fails, not one record. This causes the four failures in tests/skills/test_skill_template_runs.py (rewrite-plain, rewrite-evaluate, test_rewrite_evaluate_with_entities_in_every_row_round_trips_through_parquet, test_template_run_reaches_detector_validator_and_rewriter_through_custom_providers). Strict mode forbids low, so every strict-mode batch has an all-null column. This looks like the same nested-array read problem reported upstream for refactor(detection): establish deterministic entity candidate lifecycle #305 (Annotated Parquet reads can corrupt nested Arrow arrays with null children DataDesigner#1143), so an upstream fix may remove the crash; until then, registering COL_RAW_SENSITIVITY_DISPOSITION and COL_SENSITIVITY_DISPOSITION for JSON storage in engine/ndd/storage.py avoids it, as fix(ndd): preserve nested JSON storage across workflow handoffs #375 does for the other engine-owned nested columns. In a local strict-mode run of 5 rows on the merged tree, that change completed with no failed records. Merging or rebasing onto current main is needed first, and storage.py comes from fix(ndd): preserve nested JSON storage across workflow handoffs #375, so the fix below applies only after that. A small patch with tests that fail without it is available once the branch is current with main; it can be applied directly or sent as a PR into this branch.
2. combined_rewrite_workflow.py:341 adds COL_SENSITIVITY_DISPOSITION to the finalizer's required_columns, but nothing in the finalizer reads it. After the merge, main's test_combined_and_legacy_paths_return_equivalent_repaired_result fails with Column '_rewritten_text' requires '_sensitivity_disposition' ... which is not a known producer, because that test builds a graph with no disposition producer. The requirement is also why this PR adds COL_SENSITIVITY_DISPOSITION: [None] to two existing test fixtures. Dropping the requirement fixes both. The patch above includes it.

Design question

3. The raw decision validator rejects entries that the coverage step could have normalized (schemas/rewrite.py:182-186). It raises on medium or high with leave_as_is, so the record fails. That includes direct identifiers, which verify_disposition_coverage would otherwise force to high/replace. Main raised only on high with leave_as_is. Normalizing direct identifiers before this check would keep a model slip on a value the policy already decides from costing the record. If failing the record is the intent, a line in the plan's tradeoffs section next to incomplete coverage would be enough.

Behavior worth recording

4. No privacy question can carry low sensitivity any more. low now requires leave_as_is, and generate_privacy_qa_from_disposition creates questions only for protected_entities. Every leakage_mass term is therefore weighted 0.6 or 1.0, and the 0.3 weight in SENSITIVITY_WEIGHTS is never used; docs/concepts/rewrite.md still lists low=0.3 as a live weight. Default labels classified as direct identifiers are always high, so any confident leak of one sets any_high_leaked and adds 1.0 to the mass. The risk_tolerance thresholds were tuned against main's distribution, and fix(rewrite): repair confident medium leaks on the stricter risk_tolerance presets #379 is changing them now, so the two are worth settling together. One local run on the merged tree (25 rows of a public CSV, a local gpt-oss-120b judge, risk_tolerance="low") shows the size of the shift: protected entities were 142 high / 514 medium / 0 low, against 134 / 58 / 2 on main; privacy questions per record rose from 7.8 to 26.2; mean leakage mass rose from 0.58 to 4.67; all 25 records were repaired, and utility on repaired records fell from 0.884 to 0.618. Replaying that run's first-evaluation judge output with main's thresholds repairs 25, 25, 24 and 21 of 25 records on minimal, low, moderate and high, so the presets barely separate. The cause appears to be the prompt policy that rates most quasi-identifiers medium (one model, one temperature, three runs). Either the policy or the thresholds needs to change before risk_tolerance separates presets again.

Lower priority

5. model_loader.py:260-264: when a user passes any selected_models without naming entity_classifier, the code sets it to the disposition analyzer and ignores the bundled rewrite.yaml value; the bundled default applies only when no selections are passed at all. Falling back to the user's disposition_analyzer only when the user overrides disposition_analyzer would keep the two cases consistent.
6. Custom-label classification runs once per row, although the prompt calls it a fixed taxonomy decision. The same custom label can get different categories in different rows of one dataset. Classifying the dataset's custom labels once would make the categories consistent and remove the per-row calls.

Earlier threads

The Unicode line-split issue, the import suppressions (now covered by #357), the duplicate and missing custom-label classifications, and the rewrite-docs definition of sensitivity are all addressed on this head. The question about drop-or-flag on incomplete coverage is answered: the record is failed, the tradeoff is documented in the plan, and failed-record propagation is tested through the real adapter. The plan-retention question remains open.

[binaryaaron]

agent (review-pr): Re-review of head e6bac99, which is current with main. Both merge breaks from the previous review are fixed, and there are no new blocking findings. One new inconsistency in the retention normalization is below. The low-weight item from the previous review is now a note for the record rather than a merge condition: this PR lands first, and the preset thresholds will be re-derived on top of it in #379. The DCO check still fails.

Resolved: 1 (the disposition columns now use JSON storage), 2 (the unused finalizer requirement is removed), 3 (medium/high with leave_as_is is now normalized instead of failing the record).

New (124d3c8, e6bac99), non-blocking

1. The method a direct identifier ends up with depends on which level the model picked. _normalize_protection_policy (schemas/rewrite.py:169-179) runs first and turns medium + leave_as_is into generalize. verify_disposition_coverage (sensitivity_disposition.py:325-328) then raises a direct identifier to high, but it forces replace only when the method is still leave_as_is. So the same slip (leave_as_is on a name) has different outcomes:

   • rated low with a low_sensitivity_reason: ends high/replace
   • rated low without one: the record fails validation (schemas/rewrite.py:199-201)
   • rated high: ends high/replace
   • rated medium: ends high/generalize

   test_compact_retention_normalizes_independently_of_category asserts the generalize outcome. docs/concepts/rewrite.md lists replace as the method for names, IDs and contact details. A replace entity is substituted from the replacement map before the rewrite model sees the text (_prepare_rewrite_tagged_text, rewrite_generation.py:205-243), while a generalize entity is left to the rewrite model. The root cause is that the retention policy is now split across three places: the schema's before-validator (by level), its after-validator, and the coverage step (by category). The coverage step only sees the method after it has been normalized. To fix this, resolve leave_as_is in one place that knows the category. For example, the coverage step could decide it from the raw decision: replace for direct identifiers, and the level-based method otherwise. Then update the test.

2. The leave_as_is correction is logged inconsistently. The opposite correction (low with a protective method promoted to medium, schemas/rewrite.py:181-190) logs a warning. This one does not:

   • quasi-identifier, medium or high: nothing is logged.
   • direct identifier, high: nothing is logged. The before-validator has already produced high/replace, so the coverage step sees no change.
   • direct identifier, medium: the coverage warning reports medium/generalize → high/generalize, so the original leave_as_is never appears.

   A warning in _normalize_protection_policy would show how often this happens, which matters for the note below because each normalized entity adds a privacy question weighted 0.6 or 1.0.

Note for the record: fixed leakage-mass thresholds no longer separate the presets

low is still unreachable for protected entities, and docs/concepts/rewrite.md:26,99 still list low=0.3 as a live weight. The disposition prompt has not changed since the earlier measurement. In a comparison run on the 25-row public biographies CSV, this PR against main:

• Privacy questions answered as leaked: 31% against 10%. Most of the increase is in medium entities: generalize 123 of 389 against 10 of 51, and suppress_inference 77 of 116 against 3 of 7.
• The lowest first-evaluation leakage mass on this PR's tree was 1.2 in one run and 0.96 in a second. Both threshold proposals in fix(rewrite): repair confident medium leaks on the stricter risk_tolerance presets #379 put the minimal, low and moderate repair thresholds at 1.0 or lower.
• At low with 3 repair rounds, main repaired 1 of 25 records. With main's low repair threshold (1.0), this PR's tree would have repaired 24 of 25, judging from the recorded first-evaluation masses. (The measured run on this PR's tree also carried fix(rewrite): repair confident medium leaks on the stricter risk_tolerance presets #379's lower low threshold of 0.5 and repaired all 25, so its request count and utility are not comparable to main's run and are left out here.)

So with this PR on main, either proposal would make the minimal, low and moderate presets repair 24 or 25 of the 25 records each, leaving them nearly indistinguishable. The existing weighted_leakage_rate column still separated the presets on both trees in the same run, which may be a better basis than raw mass. The preset thresholds will be re-derived in a follow-up after this lands; #379 is where that goes. Not a merge condition here.

Unchanged from the previous review

3. Item 5: model_loader.py:260-264 still sets entity_classifier to disposition_analyzer whenever entity_classifier is not named.
4. Item 6: custom-label classification still runs once per row.
5. The plan-retention question is still open.

The DCO check fails because commit fd78a76 has no Signed-off-by line.

[binaryaaron]

agent (review-pr): Edited the review above (the note on fixed leakage-mass thresholds) to correct three figures. The conclusion is unchanged: with this PR on main, fixed mass thresholds no longer separate minimal, low and moderate.

• The lowest first-evaluation mass was 1.2 in one run on this PR's tree and 0.96 in a second, not 1.2 throughout.
• The run on this PR's tree also carried fix(rewrite): repair confident medium leaks on the stricter risk_tolerance presets #379's lower low repair threshold (0.5), while main's run used 1.0. With main's threshold, 24 of 25 records would have been repaired, not 25. The request-count ratio and the utility comparison came from runs at different thresholds and have been removed.
• Under either fix(rewrite): repair confident medium leaks on the stricter risk_tolerance presets #379 proposal, minimal, low and moderate repair 24 or 25 of the 25 records, not always 25.

[asteier2026]

Code changes made Nov 11:

Updated the branch with main and fixed compatibility issues.

• Registered the new structured disposition columns for JSON storage to avoid the nested Arrow/null-field crash.
• Removed the finalizer’s unused _sensitivity_disposition dependency.
• Reconciled tests and checks after the rebase.

Made inconsistent retention decisions recoverable. Instead of failing the record:

• high + leave_as_is becomes replace.
• medium + leave_as_is becomes generalize.
• These corrections depend on sensitivity, regardless of direct versus quasi-identifier classification.

Separated sensitivity from the requirement to protect an entity.

• Normal mode: low-sensitivity entities use leave_as_is.
• Strict mode: every entity receives protection, including low-sensitivity entities, while preserving its low rating.
• Removed the automatic promotion of protected low entities to medium.
• Updated the prompt builder so the model sees instructions for the active mode only.

Made low-sensitivity explanations optional for validation. The prompt still requests low_sensitivity_reason for every low rating, but a missing, null, or blank reason no longer fails the record.

Confirmed privacy questions follow the protection decision.

• leave_as_is entities receive no privacy question.
• Protected entities receive questions, including protected low entities in strict mode.
• Protected low entities retain the 0.3 leakage weight. Risk-tolerance thresholds were unchanged.

Updated documentation, plans, and regression tests to reflect those policies.

Broadened the sensitivity prompt’s opening to cover linkage to people, organizations, records, systems, and other protected assets, plus disclosure of credentials, access details, and proprietary information.

Fixed model-selection defaults: entity_classifier follows disposition_analyzer only when the user explicitly overrides the disposition analyzer. Unrelated model overrides preserve the bundled classifier default, and an explicit classifier selection takes priority.

Moved custom-label classification to once per dataset: both rewrite paths classify custom labels together and reuse their categories across records. Default labels remain classified locally, and sensitivity is still assessed per record.

Commit 37e743b makes retention corrections consistent and visible:

• Direct identifiers marked leave_as_is become high / replace, regardless of the model’s initial sensitivity.
• Other entities follow sensitivity: high → replace, medium → generalize; low stays unchanged in normal mode and becomes generalize in strict mode.
• Raw decisions remain intact until the authoritative category is known.
• Corrections log warnings with the entity ID and old/new policy, without exposing sensitive values.
• Tests, documentation, and the plan were updated.

[greptile-apps]

Rewrite skill tests fail

The new compact schema rejects extra fields, but tests/skills/stub_endpoint.py::_disposition still returns the old full response, including source, category, entity_label, entity_value, protection_reason, and combined_risk_level. The skill tests send rewrite requests through this endpoint, so every disposition is rejected and the tests cannot produce their expected rows.

Update the stub to read the numbered inventory and return only the four compact fields.

[asteier2026]

This is fixed.

[binaryaaron]

agent (review-pr): Re-check at head dde3645. The findings from the previous comment are fixed. Nothing here blocks merge.

Optional: log the low override in normal mode

_verify_disposition_coverage (sensitivity_disposition.py:416-419) sets every low decision to leave_as_is, including one where the model picked a protective method. That matches the documented normal-mode policy. Unlike the other corrections, though, it logs nothing. A warning here, plus a test for a low + protective decision without a reason, would make the case visible if it happens.

For the record, not merge conditions

• Thresholds. low=0.3 now applies only under strict_entity_protection. The sensitivity policy changed from "materially helps an adversary link the document to, or recognize, the subject" to "could identify or enable linkage to a person, organization, record, system, or other protected asset, or expose confidential information". That is a lower bar over a wider scope. It will probably raise ratings beyond the earlier comparison, which was measured on the older prompt. The threshold re-derivation in #379 should use this head's prompt.
• Classification failures. A classification response that is still invalid after its correction attempt now fails the whole run (classify_dataset raises AnonymizerWorkflowError), where before it failed only the affected records. The docs say so; please confirm it is intended.