feat(identity): freeze finding-pattern/v1 with canonical vectors - #55
Conversation
Slice 0 of the identity/provenance work (Own.NET#266): make the pattern-level identity a contract with one implementation and executable vectors, WITHOUT changing a single emitted byte. The recipe — sha1(path \x1f rule \x1f message)[:16] — moves from viz/apply_verdicts into identity/pattern.py unchanged, and finding_id stays as the overlay-facing alias. Nothing about the output moves, which is the entire point: every stored fp-verdicts.json is keyed by those bytes, the freshness guard only proves the verdicts were judged against the same findings file, and an overlay that stops matching yields an EMPTY REPORT rather than an error. A silent failure mode earns a test, not a comment. contracts/finding-pattern-v1.json is the executable half. Its vectors pin what is deliberately NOT normalized — path separator, path case, digits and whitespace in the message — each as its own distinctness pair, so a well-meant cleanup fails a test instead of quietly re-keying every verdict in existence. Also pinned: the intentional collision (two physical findings sharing (path, rule, message) share an id, because one judged verdict covers the repeated pattern), UTF-8 handling for non-ASCII paths and messages, and that a MISSING field raises while an EMPTY one hashes. The expected ids were not generated by the implementation under test. They were computed from an independent transcription of the recipe and cross-checked against the shipped apply_verdicts.finding_id before being frozen; the test keeps that outside opinion as a live check, because vectors regenerated from their own implementation agree with each other and with nothing else. Recorded in the domain contract and AGENTS.md, because it has already misled once: SARIF's partialFingerprints["ownAudit/v1"] is a DIFFERENT value — it lower-cases the message, collapses digits and spaces, keeps the full digest and appends a /ordinal suffix for repeats. It is a legacy GitHub-correlation key. The ordinal alone disqualifies it as identity: presentation order must never change an id. New fingerprints will go alongside it, never in place of it, or the migration would reset every alert's history while claiming to improve correlation. Verified: new contract tests pass under python3 and -O; apply_verdicts selftest unchanged; the full Linux-safe sweep from AGENTS.md — all 15 suites — passes. Refs PhysShell/Own.NET#266. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change defines the ChangesFinding-pattern identity
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant FindingRecord
participant finding_id
participant pattern_id_of
participant fp_verdicts
FindingRecord->>finding_id: path, rule, message
finding_id->>pattern_id_of: compute canonical pattern ID
pattern_id_of-->>finding_id: 16-character fingerprint
finding_id->>fp_verdicts: match stored verdict overlay
fp_verdicts-->>finding_id: verdict result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b36496fa94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/ci.yml:
- Line 37: Apply the PYTHONUTF8=1 prefix consistently to the normal and
optimized identity test commands in .github/workflows/ci.yml at lines 37-37 and
57-57. Update the documented bare-script command in
identity/tests/test_pattern.py lines 1-14 and the self-test and CLI usage
examples in identity/pattern.py lines 107-108 and 157-158 to include
PYTHONUTF8=1.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9807e27a-3133-478a-979e-80510caaf48e
📒 Files selected for processing (7)
.github/workflows/ci.ymlAGENTS.mdcontracts/finding-pattern-v1.jsondocs/fp-judge/verdict-contract.mdidentity/pattern.pyidentity/tests/test_pattern.pyviz/apply_verdicts.py
…proof Two review findings, both reproduced before fixing. 1. AN INSTALLED `identity` PACKAGE SHADOWED OURS (Codex P2). Without `__init__.py` the directory is only a NAMESPACE portion: during the sys.path scan a namespace portion is remembered and the scan CONTINUES, so a regular package found later still wins -- inserting the repo root first does not help. `identity` is a real name on PyPI, so this is not hypothetical. Reproduced with a stand-in installed package: `ModuleNotFoundError: No module named 'identity.pattern'`, resolving to the other distribution. Adding the initializer fixes it; verified against the same stand-in. 2. THE TEST'S OWN OUTPUT COULD CRASH IT (CodeRabbit, with a correction). The claim was a missing PYTHONUTF8=1 prefix. Reading the vectors was never at risk -- every read passes `encoding="utf-8"` explicitly -- but the printed summary contained an em-dash, and under a legacy stdout encoding that raises UnicodeEncodeError. Reproduced with `PYTHONUTF8=0 LC_ALL=C`: the contract verified correctly and then the process died printing the result. A failing test that fails on its own output is worse than a failing test. Fixed at the source rather than documented around: the identity module and its test are now pure ASCII, including the docstring that the CLI branch prints. Both now pass under `PYTHONUTF8=0 LC_ALL=C`, so they no longer depend on the ambient encoding at all. The documented invocations do carry PYTHONUTF8=1, matching AGENTS.md. The CI lines deliberately do NOT: this workflow has never used the prefix on any of its sixteen suites, and adding it to only the two new lines would make them inconsistent with the rest while fixing nothing that (2) has not already fixed. If the runners' locale were ever non-UTF-8 that would be a repo-wide problem, not this slice's. Verified: both hostile scenarios now pass; the full Linux-safe sweep -- now 16 suites -- passes, plus -O. Refs PhysShell/Own.NET#266. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
|
Учтено в Чтение векторов под угрозой не было — все чтения идут с явным Починил в источнике, а не обошёл документацией: модуль identity и его тест теперь полностью ASCII, включая docstring, который печатает CLI-ветка. Оба проходят под Документированные команды запуска Generated by Claude Code |
2 participants
Что и зачем
Срез 0 работы над identity/provenance (Own.NET#266): сделать pattern-level идентичность контрактом с единственной реализацией и исполняемыми векторами, не изменив ни одного эмитируемого байта.
Рецепт
sha1(path \x1f rule \x1f message)[:16]переезжает изviz/apply_verdictsвidentity/pattern.pyбез изменений, аfinding_idостаётся алиасом для overlay. Ничего в выводе не двигается — и в этом весь смысл: каждый сохранённыйfp-verdicts.jsonключуется этими байтами, guard свежести доказывает лишь то, что вердикты выносились против того же файла находок, а overlay, переставший совпадать, даёт пустой отчёт, а не ошибку. Тихий отказ заслуживает теста, а не комментария.Векторы считались не той реализацией, которую проверяют
contracts/finding-pattern-v1.json— исполняемая половина контракта. Ожидаемые id получены независимой транскрипцией рецепта из текста доменного контракта и сверены со штатнойapply_verdicts.finding_idдо заморозки; совпали на всех девяти. Эта транскрипция осталась в тесте живой проверкой — векторы, сгенерированные собственной реализацией, согласны сами с собой и больше ни с чем.Отдельными парами различимости закреплено то, что намеренно не нормализуется:
windows-style-path≠posix-style-pathposix-style-path≠path-case-uppermessage-with-digits≠message-with-other-digitsmessage-with-digits≠whitespace-differsposix-style-path=repeated-identicalПлюс UTF-8 для не-ASCII пути и сообщения, и разница между отсутствующим полем (ошибка) и пустым (хешируется).
Разграничение с SARIF-ключом
Записано в доменном контракте таблицей, в
AGENTS.mdи в docstring модуля, потому что оно уже один раз ввело в заблуждение.partialFingerprints["ownAudit/v1"]— другое значение: нормализованное сообщение, полный digest, суффикс/ordinalдля повторов. Это legacy-ключ корреляции GitHub. Одного ординала достаточно, чтобы дисквалифицировать его как идентичность: порядок подачи результатов не имеет права менять id. Новые fingerprints пойдут рядом с ним, а не вместо — иначе миграция сбросит историю всех alert'ов, попутно объявив это улучшением корреляции.Non-goals
Тип изменения
Как проверено
PYTHONPATH=. python3 identity/pattern.py --selftest— 9 векторов, 1 пара коллизии, 4 пары различимостиPYTHONPATH=. python3 identity/tests/test_pattern.py— включая побайтовую сверку алиасаapply_verdicts.finding_idна каждом векторе-O(assert-stripping safety), как требует конвенция репозиторияviz/apply_verdicts.py --selftest— без измененийAGENTS.md— все 15 сьют проходят-O)Связанные issue
Refs PhysShell/Own.NET#266 — Slice 0: freeze
finding-pattern/v1. Issue не закрывается: впереди перенос нормализатора с parity, схема с occurrence/provenance и модель lineage.Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests