55: normalize reproducibility hash recipe across audit writers - #84
Conversation
`prune.audit._compute_config_hash` and `safety.policy._compute_policy_hash` were the last two outliers using `SHA-256[:16]` while every other reproducibility hash in the audit / sidecar corpus (`draft`, `grade`, `diff`, plus prune's `compiled_sql_hash`) already used `blake2b(digest_size=8)` over canonical JSON. Migrating both to the same recipe so a reviewer correlating `safety.jsonl` / `llm_responses.jsonl` / `prune.jsonl` / `grade.jsonl` / `diff.json` reads one family of digests. `PruneEvent.audit_schema_version` bumped 1 → 2; safety `AuditEvent` bumped 2 → 3. Fixtures + drift detectors + ops docs + rule files refreshed in lockstep. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (4)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughStandardizes reproducibility hash computation across prune and safety audit writers by migrating from SHA-256 truncation to unified ChangesAudit hash recipe normalization (issue
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
This PR standardizes reproducibility hashes across the audit/sidecar corpus by migrating remaining SHA-256[:16] usages to blake2b(digest_size=8) (16 hex chars), and bumps audit schema versions in prune and safety to reflect the hash-recipe change while keeping JSONL round-trippable across historical versions.
Changes:
- Migrate
policy_hash(safety) andconfig_hash(prune) hashing toblake2b(digest_size=8)and update schema-version constants/defaults accordingly. - Refresh drift detectors, unit tests, and committed JSONL fixtures for the new
audit_schema_versionvalues (safety: 3, prune: 2). - Update ops/docs and rule files to describe the new hash recipe and version gating guidance.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/safety/test_request.py | Updates safety request audit-schema-version expectation to 3. |
| tests/safety/test_models.py | Updates AuditEvent default version to 3 and adds legacy v2 parsing coverage. |
| tests/safety/test_drift_detector.py | Updates drift-detector fixture pinning docs for the v3 bump. |
| tests/safety/test_audit.py | Updates audit writer tests to assert audit_schema_version == 3. |
| tests/prune/test_engine.py | Updates pinned fixture assertions to expect prune audit_schema_version == 2. |
| tests/prune/test_drift_detector.py | Updates strict prune event schema literal to v2. |
| tests/prune/test_audit.py | Updates prune audit writer tests to assert audit_schema_version == 2. |
| tests/fixtures/safety/regenerate.sh | Updates fixture regeneration notes and bumps fixture records to v3. |
| tests/fixtures/safety/audit_events_sample.jsonl | Updates committed safety JSONL fixture lines to audit_schema_version: 3. |
| tests/fixtures/prune/prune_event_v1.jsonl | Updates committed prune JSONL fixture lines to audit_schema_version: 2. |
| tests/fixtures/e2e_helpers/happy/.signalforge/prune.jsonl | Updates e2e prune fixture JSONL to audit_schema_version: 2. |
| src/signalforge/safety/request.py | Bumps safety writer _AUDIT_SCHEMA_VERSION to 3 and documents the reason. |
| src/signalforge/safety/policy.py | Migrates _compute_policy_hash to blake2b(digest_size=8) and updates docstrings. |
| src/signalforge/safety/models.py | Bumps AuditEvent.audit_schema_version default to 3 and updates schema history docs. |
| src/signalforge/prune/engine.py | Updates prune pipeline documentation to the new hash recipe. |
| src/signalforge/prune/audit.py | Migrates _compute_config_hash to blake2b(digest_size=8) and bumps prune schema version to 2. |
| docs/safety-ops.md | Updates ops docs for safety policy_hash recipe and current schema version (3). |
| docs/prune-ops.md | Updates ops docs for prune config_hash recipe and current schema version (2). |
| CLAUDE.md | Updates public-surface notes to reflect the new safety/prune schema versions and hash recipe. |
| .claude/rules/safety-layer.md | Updates safety layer rules describing the reproducibility fields and versioning. |
| .claude/rules/prune-engine.md | Adds/updates guidance documenting the hash recipe normalization and schema bump rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/rules/safety-layer.md:
- Line 34: Update the audit_schema_version signature so it matches production:
change the declaration audit_schema_version: int = 1 to audit_schema_version:
int = 3 (or alternatively split into two explicit items such as
audit_schema_version (schema default) vs current_production_audit_schema_version
= 3) so the signature no longer contradicts the explanatory text; edit the
single symbol audit_schema_version in the doc and adjust the explanatory
parenthetical to remove confusion.
In `@src/signalforge/prune/audit.py`:
- Line 106: The field audit_schema_version is currently typed as Literal[2]
which prevents reading historical prune.jsonl entries with value 1; change the
annotation on audit_schema_version (in the model where it is declared) from
Literal[2] to int (keeping the default value 2) so older rows validate; keep the
existing runtime gate that checks audit_schema_version >= 2 where applicable and
move the strict assertion of "we emit 2" into writer tests (update the
writer/unit tests to assert the emitted JSONL contains audit_schema_version ==
2).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2c0ae7bf-f2a4-4102-9ba6-8cfec5edc659
📒 Files selected for processing (21)
.claude/rules/prune-engine.md.claude/rules/safety-layer.mdCLAUDE.mddocs/prune-ops.mddocs/safety-ops.mdsrc/signalforge/prune/audit.pysrc/signalforge/prune/engine.pysrc/signalforge/safety/models.pysrc/signalforge/safety/policy.pysrc/signalforge/safety/request.pytests/fixtures/e2e_helpers/happy/.signalforge/prune.jsonltests/fixtures/prune/prune_event_v1.jsonltests/fixtures/safety/audit_events_sample.jsonltests/fixtures/safety/regenerate.shtests/prune/test_audit.pytests/prune/test_drift_detector.pytests/prune/test_engine.pytests/safety/test_audit.pytests/safety/test_drift_detector.pytests/safety/test_models.pytests/safety/test_request.py
Three review fixes from PR #84: 1. `_compute_config_hash` now canonicalises internally (`json.dumps(..., sort_keys=True, separators=(",", ":"))`) so callers can't accidentally feed non-canonical input. Pydantic's `model_dump_json` does not contractually guarantee sorted keys across point releases; the call site in `prune.engine` was passing the bare dump. Mirrors `safety.policy._compute_policy_hash` verbatim. 2. `PruneEvent.audit_schema_version` typed back to `int` (default `_PRUNE_AUDIT_SCHEMA_VERSION`) rather than `Literal[2] = 2`. The `Literal` form rejected pre-#55 records with `audit_schema_version: 1` and broke the audit-replay round-trip contract that safety's `AuditEvent` already honours. Drift-detector strict mirror updated to `int` to match; added two new pin tests — current-version fixture pin plus legacy-v1 round-trip — so the contract is exercised both ways. 3. `safety-layer.md` AuditEvent bullet now reads `audit_schema_version: int` (was stale `int = 1`) with the round-trip rationale called out. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR Review SummaryFixed (3 items)
Added two new pin tests in
False Positives (1 item)
🤖 Generated with Claude Code |
Summary
prune.audit._compute_config_hashandsafety.policy._compute_policy_hashfromSHA-256[:16]toblake2b(digest_size=8)so every reproducibility hash in the audit / sidecar corpus reads one recipe.PruneEvent.audit_schema_version1 → 2 and safetyAuditEvent.audit_schema_version2 → 3 in lockstep; fields stayintso older JSONLs still round-trip.docs/safety-ops.md,docs/prune-ops.md), and rule files (.claude/rules/safety-layer.md,.claude/rules/prune-engine.md) +CLAUDE.mdpublic-surface notes in lockstep.Closes #55.
Why
Pre-#55 the audit corpus used two hash families:
blake2b-8(draft, grade, diff, prune'scompiled_sql_hash) andSHA-256[:16](pruneconfig_hash, safetypolicy_hash). A reviewer correlating acrosssafety.jsonl/llm_responses.jsonl/prune.jsonl/grade.jsonl/diff.jsonneeded to know which family applied where. One recipe is the explainability surface Architectural Commitment #5 asks for.Test plan
ruff check . && ruff format --check . && pyrightcleanpytest— 1805 pass, 6 pre-existing WSL2 symlink-loop failures (pass on GHA), 96% coverage maintainedgrep -rn "sha256\|SHA-256"sweep — remaining references are historical-context notes in docstrings/rules + orthogonal dbt manifest fixtures🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores