Skip to content

feat(diagnostics): port the SARIF 2.1.0 projection with canonical parity - #324

Merged
PhysShell merged 2 commits into
mainfrom
claude/own256-sarif-parity
Aug 7, 2026
Merged

feat(diagnostics): port the SARIF 2.1.0 projection with canonical parity#324
PhysShell merged 2 commits into
mainfrom
claude/own256-sarif-parity

Conversation

@PhysShell

@PhysShellPhysShell commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Что и зачем

P-022 шаг 5b. Порт ownlang/diag_sarif.py и двух builder'ов из ownlang/evidence.py в own_diagnostics::sarif, с replay из Python-авторитетных фикстур и нулём Python в steady state.

Типизированные структуры, а не свободный JSON: форма проверяется компилятором, и крейт не получает runtime-зависимости на JSON. serde_json остаётся dev-dependency — ровно как уже сделано для Diagnostic.

Тип изменения

  • feat — новая возможность
  • fix — исправление бага
  • docs — документация
  • refactor / chore / test / ci — без изменения поведения

Как проверено

  • python tests/run_tests.py
  • ruff check . и mypy
  • cargo fmt + cargo clippy --all-targets + cargo test --workspace --no-fail-fast

Python-выход не менялся: порт читает diag_sarif, не правит его.

Связанные issue

Refs #250, #256. Ничего не закрывает — см. «Что вычеркнуто».

Чеклист

  • изменение покрыто тестом/селфтестом (или объяснено, почему нет)
  • README/docs обновлены при необходимости
  • коммиты в conventional-commit стиле (feat:, fix:, docs: …)

Три семантики, на которых наивный порт ломается

Каждая закреплена своим case в tests/fixtures/sarif_parity.json.

ЛовушкаЭталонЧто сделал бы наивный порт
Evidence.filee.file or filenametruthiness: Some("") берёт fallback так же, как NoneOption-presence → пустой artifactLocation.uri, и весь лог непроцессируем для code scanning
resource_kindif d.resource_kind — тоже truthiness: пустая строка не даёт суффикс" [resource: ]" в пользовательском тексте сообщения
порядокresults в порядке входа, rules отсортированы — два разных порядка в одном логеотсортировать results под каталог и уничтожить tie-порядок

Канонное сравнение

#256 требует перечислить каждое удалённое поле и запрещает wildcard «ignore metadata». Перечень пуст, и это измерение, а не срезанный угол:

  • генератор делает census на timestamp / GUID / абсолютный путь — чисто;
  • и проверяет детерминизм: два прогона того же входа байт-в-байт равны.

Поэтому нормализуется ровно одна ось — порядок ключей объекта, единственное, где два корректных сериализатора имеют право разойтись. Массивы не трогаются: results, relatedLocations и threadFlows[].locations — упорядоченные контракты, и сортировка стёрла бы ровно то, ради чего шаг существует. Именно это делает сравнение строгим: порт с правильным множеством результатов в неправильном порядке здесь падает.

Отдельный тест мутационно защищает сам канонизатор: если бы он сортировал массивы, контракты порядка молча перестали бы проверяться.

Mutation-проверка

--no-fail-fast, чтобы увидеть все слои-ловцы, а не первый упавший таргет.

МутацияКто ловит
Evidence.file как Option-presenceтолько parity replay
resource_kind как Option-presenceтолько parity replay
results отсортированы по кодутолько parity replay
убрана нормализация обратных слэшейparity replay и uri-инвариант
region для file-level строкитолько parity replay
канонизатор сортирует массивысобственный guard канонизатора
rules усечены (висячий ruleId)parity replay и code-scanning инварианты

Первая строка стоит отдельного слова, потому что очевидная интуиция неверна: тест «нет пустых artifactLocation.uri» не ловит truthiness-баг. Под мутацией file резолвится в "", и шаг отбрасывается фильтром, а не эмитится с пустым uri — симптом в молча пропавшем evidence-шаге. Порт, который отгрузил бы uri-инвариант и посчитал ловушку покрытой, ошибался бы насчёт собственного покрытия.

Контроли из #256

Покрыты: пустой результат-сет, диагностика без Evidence, multi-file relatedLocations, multi-step codeFlows, Windows-путь и Unix-путь, не-ASCII путь, равные code+message в разных строках, стабильный порядок таблицы правил, warning/error mapping, Owen как tool identity.

Про «note»: у Severity в ядре есть только ERROR и WARNING — уровень note этим путём не производится. Это измерение, а не пропуск.

Что вычеркнуто, а не подделано

Три требования #256 оказались невыполнимыми как написано. Каждое проверено прогоном против дерева.

.ownreport.json. Issue описывает «schema/version, tool and run metadata, diagnostics and ordered Evidence». В дереве это {module, buffers[]} — отчёт о буферах. Диагностики дают четыре булевых checks на буфер; ни Evidence, ни tool-метаданных, ни schema version, ни одного пути. Плюс report.py импортирует ast_nodes и buffers.resolve — то есть честный порт требует AST и резолвер политик, чего guardrail самого же #256 крейту не разрешает.

И на эту форму уже стоит явный запрет: «НЕ перегружать .ownreport.json» в AGENTS.execution-surfaces.md, плюс acceptance-строка «build_report untouched» в docs/tasks/evidence-coverage.md. То есть половину про отчёт не просто не сделали — её просили в форме, от которой проект уже отказался.

Поэтому она вычеркнута из шага 5b, а не отложена: cutover не требует Rust-овского buffer-отчёта, а изобрести диагностико-несущий отчёт под текст issue значило бы протащить новое поведение под видом порта.

DI004/DI005. Эти коды даёт ownir.build_sarif над ownir.Finding — путь OwnIR, исключённый guardrail'ом «no OwnIR bridge» того же issue. Уезжает с мостом (#259).

Загрузка Rust-SARIF в Code Scanning. В Rust-воркспейсе нет ни одного binary target — писать лог физически нечем, а #256 запрещает CLI-миграцию. Добавить одноразовый бинарник ради галочки значило бы пересечь ту самую границу. Вместо этого проверяются структурные правила, которые реально энфорсит ingest: каждый ruleId разрешается в каталоге драйвера, level из SARIF-энума, startLine ≥ 1, uri непустой и без обратных слэшей — над Rust-произведённым логом. Вместе с байтовой идентичностью Python-логу, который CI уже успешно загружает, acceptance следует из идентичности, а не из непроверенного утверждения.

Всё это записано в P-022 с измерениями, и строка индекса плюс метаданные крейта (description, докстринг Evidence, обещавшие «SARIF — это позже») переставлены тем же изменением.


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added SARIF 2.1.0 output for flow diagnostics.
    • Preserved diagnostic ordering, severity, locations, related findings, and code-flow details in generated reports.
    • Improved handling of file paths, Unicode paths, missing evidence, file-level findings, and unknown diagnostic codes.
  • Bug Fixes

    • Improved consistency and compatibility between diagnostic SARIF output paths.
  • Documentation

    • Updated migration documentation to reflect completed SARIF support and clarify remaining reporting scope.

P-022 step 5b. Ports `ownlang/diag_sarif.py` and the two `ownlang/evidence.py`
builders it uses into `own_diagnostics::sarif`, replayed from Python-authored
fixtures with zero Python at steady state.
Modelled as typed structs rather than a JSON value: the shape is checked at
compile time and the crate gains no runtime JSON dependency, matching how
Diagnostic is already done. serde_json stays a dev-dependency.
Three semantics a naive port gets wrong, each with a fixture case:
* Evidence.file falls back on EMPTINESS, not absence. Python writes
`e.file or filename`, so Some("") takes the fallback exactly as None does.
* resource_kind appends on emptiness too: `if d.resource_kind` is truthiness,
so Some("") adds no suffix where Option-presence would emit " [resource: ]".
* results follow input order while rules are sorted — two orderings in one log.
Sorting results to match the catalogue destroys the tie order that equal-code,
equal-message diagnostics at different lines depend on.
Canonical comparison: parse both, remove NOTHING, normalize only object key
order, leave every array in place, compare. The volatile-field list is empty as
a measurement, not a shortcut — the generator censuses for timestamps, GUIDs and
absolute paths and asserts the builder is deterministic. Arrays being left alone
is what makes the comparison strict: a port with the right set of results in the
wrong order fails.
Six mutations, each caught, measured with --no-fail-fast. Worth recording that
the empty-uri invariant does NOT catch the Evidence.file truthiness bug: under
that mutation the resolved file is "", so the step is DROPPED rather than
emitted with an empty uri, and only the reference comparison sees it.
Three of #256's requirements were unbuildable as written and are struck rather
than faked, each verified against the tree:
* .ownreport.json is a buffer report ({module, buffers[]}) with no diagnostics,
Evidence or tool metadata, and porting it needs ast_nodes + buffers.resolve —
which #256's own guardrail forbids this crate from reaching. A standing house
rule already refuses the shape #256 described.
* the DI004/DI005 control comes from ownir.build_sarif, excluded by the same
issue's "no OwnIR bridge".
* "upload Rust-generated SARIF" needs a binary; the workspace has none, and
#256 forbids CLI migration. Replaced by asserting the structural rules an
ingest enforces over the Rust-produced log.
Recorded in P-022 with the measurements, and the index row and crate metadata
moved in the same change.
Refs #256, #250.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM
@coderabbitai

coderabbitaiBot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@PhysShell, you've reached your PR review limit, so we couldn't start this review.

Next review available in:50 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b5fa5f27-b09a-4808-9a54-ab58df88662a

📥 Commits

Reviewing files that changed from the base of the PR and between 984de7d and a10af9b.

📒 Files selected for processing (4)
  • rust/crates/own-diagnostics/src/diagnostic.rs
  • rust/crates/own-diagnostics/src/sarif.rs
  • tests/fixtures/sarif_parity.json
  • tests/test_sarif_parity_fixtures.py
📝 Walkthrough

Walkthrough

The PR adds typed Rust SARIF 2.1.0 generation to own-diagnostics, exports the SARIF model and builders, and adds parity fixtures plus replay tests for output structure, ordering, paths, evidence, severity, and metadata. P-022 documentation records SARIF completion and removes .ownreport.json from step 5b.

Changes

SARIF projection

Layer / File(s)Summary
SARIF contract and public API
rust/crates/own-diagnostics/..., docs/proposals/...
Adds serializable SARIF model types, crate-root exports, and documentation for the completed SARIF scope.
Diagnostic-to-SARIF builders
rust/crates/own-diagnostics/src/sarif.rs
Builds SARIF locations, related evidence, code flows, results, rules, tool metadata, and complete logs with path normalization and deterministic ordering.
Parity fixtures and validation
tests/fixtures/sarif_parity.json, tests/test_sarif_parity_fixtures.py, rust/crates/own-diagnostics/tests/sarif_replay.rs
Generates and verifies parity fixtures, then checks canonical output, URI validity, typed round trips, SARIF invariants, and tool metadata.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

Possibly related PRs

  • PhysShell/Own.NET#42 — Implements and validates the SARIF generation behavior that this PR ports to Rust.
  • PhysShell/Own.NET#118 — Adds the structured evidence-to-SARIF functionality consumed by this projection.
  • PhysShell/Own.NET#168 — Defines the P-022 SARIF responsibilities and the boundary with own-bridge.

Sequence Diagram(s)

sequenceDiagram
participant Diagnostics
participant build_sarif
participant SarifLog
participant ReplayTests
Diagnostics->>build_sarif: Diagnostic values, filename, severity
build_sarif->>SarifLog: Typed SARIF results and metadata
SarifLog-->>ReplayTests: Generated SARIF document
ReplayTests->>ReplayTests: Compare canonical output and validate invariants
Loading

Suggested reviewers:claude

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly and concisely describes the main change: porting the SARIF 2.1.0 projection with parity validation.
Description check✅ PassedThe description fills all required sections, explains the scope and exclusions, and lists relevant validation commands and tests.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/own256-sarif-parity

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@PhysShellClaude

Copy link
Copy Markdown
OwnerAuthor

@coderabbitai review

The automatic pass on 6cc5c52..984de7d was skipped by the quota window at 15:22Z, so this commit has not been reviewed by anyone yet — the commit status still reads "Review rate limited". No files have changed since; HEAD is unchanged at 984de7d.

Scope note, so the pass is spent on the port rather than on re-opening settled ground: this PR deliberately covers the .own flow-diagnostic SARIF path only. .ownreport.json, the DI004/DI005 control and the Code Scanning upload were struck with measurements against the tree, not skipped — the reasoning is in the PR body and now in #256 and #250 as well. Suggestions to port ownir.build_sarif or to build a diagnostics-bearing .ownreport.json are out of scope by design and will be declined.

Most useful places to look: the truthiness fallbacks in steps_for and result_for (Some("") must behave as None does), the two independent orderings in build_sarif (sorted rules, input-order results), and whether the canonicalizer in sarif_replay.rs really leaves arrays untouched.


Generated by Claude Code

@coderabbitai

coderabbitaiBot commented Aug 7, 2026

Copy link
Copy Markdown

@PhysShell I will review commit 984de7d.

I will keep the review within the stated .own flow-diagnostic SARIF scope. I will check the empty-string fallbacks, the independent rules and results ordering, and array preservation in sarif_replay.rs.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (4)
rust/crates/own-diagnostics/src/sarif.rs (1)

302-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse Diagnostic::kind_suffix instead of re-deriving the suffix.

render.rs already defines Diagnostic::kind_suffix, with the identical truthiness rule and the identical " [resource: {kind}]" format. Two copies of one user-visible text contract can drift independently. Call the existing method here.

♻️ Proposed refactor
- // Truthiness, not presence: an empty `resource_kind` adds no suffix.- let kind = match diagnostic.resource_kind.as_deref() {- Some(k) if !k.is_empty() => format!(" [resource: {k}]"),- _ => String::new(),- };+ // Truthiness, not presence: an empty `resource_kind` adds no suffix.+ // `kind_suffix` owns that rule for both the text and the SARIF renderings.+ let kind = diagnostic.kind_suffix();
🤖 Prompt for 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.
In `@rust/crates/own-diagnostics/src/sarif.rs` around lines 302 - 313, Update the
SARIF result construction near steps_for to call Diagnostic::kind_suffix for the
message suffix instead of matching diagnostic.resource_kind and formatting it
locally. Remove the duplicated kind logic while preserving the existing
empty-resource behavior and message format.
tests/test_sarif_parity_fixtures.py (2)

342-348: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Narrow the nt-abs-path exemption to the fixture path.

The (?!src) lookahead exempts every NT drive path whose first segment is src. The census is meant to catch a leaked absolute path from a developer machine. A leak such as D:\src\... or C:\srcgen\... passes the lookahead and stays invisible. Anchor the exemption to the two paths the fixture deliberately uses instead.

♻️ Proposed refactor
 patterns = {
"iso-timestamp": r"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}",
"guid": r"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-",
"posix-abs-path": r'"/(?:home|tmp|Users|var)/',
- "nt-abs-path": r'"[A-Za-z]:\\\\(?!src)',+ # The windows-drive-path case intentionally carries `C:\src\App\...`;+ # every other NT drive path is a leaked machine path.+ "nt-abs-path": r'"[A-Za-z]:\\\\(?!src\\\\App\\\\)',
}
🤖 Prompt for 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.
In `@tests/test_sarif_parity_fixtures.py` around lines 342 - 348, Update the
nt-abs-path pattern in the patterns mapping to exempt only the two deliberate
fixture paths, rather than any Windows path beginning with src. Preserve
detection for other absolute paths, including D:\src\... and C:\srcgen\...,
while keeping the existing path-matching behavior unchanged.

318-330: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the unused canonical helper.

canonical is not called from this module, and no Python code imports it. Delete the helper and keep the canonicalization rationale in the module docstring if needed.
[maintain ibility_and_code_quality]

🤖 Prompt for 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.
In `@tests/test_sarif_parity_fixtures.py` around lines 318 - 330, Remove the
unused canonical function, including its docstring, from the test module.
Preserve any necessary canonicalization rationale in the module-level docstring
without retaining the unreferenced helper.
rust/crates/own-diagnostics/tests/sarif_replay.rs (1)

344-352: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extend the startLine check to the evidence projections.

The loop checks only result.locations. relatedLocations and codeFlows[].threadFlows[].locations also carry regions, and an ingest validates those the same way. Today emittable drops any step with line < 1, so the extra check cannot fail — which is exactly the point: it pins that contract at the invariant layer instead of leaving it to the parity replay alone, as the module docs in rust/crates/own-diagnostics/src/sarif.rs line 41 record.

💚 Proposed test extension
+ let regions = result+ .locations+ .iter()+ .map(|l| &l.physical_location)+ .chain(result.related_locations.iter().map(|l| &l.physical_location))+ .chain(result.code_flows.iter().flat_map(|cf| {+ cf.thread_flows.iter().flat_map(|tf| {+ tf.locations+ .iter()+ .map(|tfl| &tfl.location.physical_location)+ })+ }));+ for physical in regions {+ if let Some(region) = &physical.region {
assert!(
region.start_line >= 1,
"case {name:?}: startLine must be 1-based, got {}",
region.start_line
);
}
}
🤖 Prompt for 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.
In `@rust/crates/own-diagnostics/tests/sarif_replay.rs` around lines 344 - 352,
Extend the SARIF replay invariant check beyond result.locations to also validate
regions in relatedLocations and codeFlows[].threadFlows[].locations. Reuse the
existing start_line >= 1 assertion and case context for every evidence
projection, preserving the same behavior and diagnostic message style.
🤖 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 `@rust/crates/own-diagnostics/src/diagnostic.rs`:
- Around line 34-36: Update the SARIF reference in the documentation comment
near Diagnostic::render_pretty to link to the public crate::build_sarif entry
point instead of the private crate::sarif module, preserving the surrounding
description.
In `@rust/crates/own-diagnostics/src/sarif.rs`:
- Around line 292-301: Correct the documentation comment above result_for to
describe the actual severity behavior: requesting "warning" can lower an
intrinsic error to warning, while requesting "error" does not raise an intrinsic
warning, which remains warning. Leave the implementation unchanged.
---
Nitpick comments:
In `@rust/crates/own-diagnostics/src/sarif.rs`:
- Around line 302-313: Update the SARIF result construction near steps_for to
call Diagnostic::kind_suffix for the message suffix instead of matching
diagnostic.resource_kind and formatting it locally. Remove the duplicated kind
logic while preserving the existing empty-resource behavior and message format.
In `@rust/crates/own-diagnostics/tests/sarif_replay.rs`:
- Around line 344-352: Extend the SARIF replay invariant check beyond
result.locations to also validate regions in relatedLocations and
codeFlows[].threadFlows[].locations. Reuse the existing start_line >= 1
assertion and case context for every evidence projection, preserving the same
behavior and diagnostic message style.
In `@tests/test_sarif_parity_fixtures.py`:
- Around line 342-348: Update the nt-abs-path pattern in the patterns mapping to
exempt only the two deliberate fixture paths, rather than any Windows path
beginning with src. Preserve detection for other absolute paths, including
D:\src\... and C:\srcgen\..., while keeping the existing path-matching behavior
unchanged.
- Around line 318-330: Remove the unused canonical function, including its
docstring, from the test module. Preserve any necessary canonicalization
rationale in the module-level docstring without retaining the unreferenced
helper.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9f56ca65-ca33-46f3-855e-204d139a53d0

📥 Commits

Reviewing files that changed from the base of the PR and between 6cc5c52 and 984de7d.

📒 Files selected for processing (9)
  • docs/proposals/P-022-rust-core-migration.md
  • docs/proposals/README.md
  • rust/crates/own-diagnostics/Cargo.toml
  • rust/crates/own-diagnostics/src/diagnostic.rs
  • rust/crates/own-diagnostics/src/lib.rs
  • rust/crates/own-diagnostics/src/sarif.rs
  • rust/crates/own-diagnostics/tests/sarif_replay.rs
  • tests/fixtures/sarif_parity.json
  • tests/test_sarif_parity_fixtures.py

Comment threadrust/crates/own-diagnostics/src/diagnostic.rs
Comment threadrust/crates/own-diagnostics/src/sarif.rs
… kind_suffix
Review round on 984de7d. Every finding checked against the tree first.
The one that mattered: the `severity` override was documented backwards. The
doc claimed it "only ever raises"; measured against the reference it only ever
LOWERS.
request intrinsic Error intrinsic Warning
"error" (or anything) error warning
"warning" warning warning
Asking for "warning" turns an error into a warning; asking for "error" cannot
turn a warning into an error. The same inversion had been copied into the
fixture's case rationale, so both are corrected and the fixture regenerated.
Behaviour is unchanged — only the description of it was wrong, which for a
user-visible severity contract is worth fixing on its own.
Also:
* `[crate::sarif]` named the private module, so rustdoc emitted
private_intra_doc_links and rendered no link. Points at `crate::build_sarif`
now; `cargo doc` is clean.
* the SARIF result reused a private copy of the resource-kind suffix rule.
Calling the existing `Diagnostic::kind_suffix` removes a second owner of one
user-visible text contract — and strengthens coverage as a side effect:
breaking the emptiness rule now fails the human render as well as the parity
replay, measured.
* the volatile-field census exempted every NT drive path starting with `src`,
so `D:\src\...` and `C:\srcgen\...` would have passed as clean. Narrowed to
the one path the fixture deliberately carries; both probes now flag.
* dropped an unused `canonical()` helper from the Python generator — the Rust
replay owns canonicalization, and the rationale already lives in the docstring.
Declined, with the measurement: extending the `startLine >= 1` assertion to
relatedLocations and codeFlows. Both `emittable` and `phys` gate on `line >= 1`,
and breaking both together already fails the ingest invariants through the
primary `locations` — so the extra assertion cannot be made to fail. It would
grow the coverage claim without growing coverage.
The mutation table in the module docs was itself stale: it was measured before
the ingest-invariant test existed, and said "parity replay only" for a row that
now has two catchers. Re-measured in full and corrected. One of those re-runs
initially reported zero catchers for the severity mutation, which turned out to
be a shell-escaping typo producing a compile error rather than a surviving
mutant — re-run cleanly, it is caught.
Refs #256, #250.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM
@PhysShellClaude

Copy link
Copy Markdown
OwnerAuthor

Nitpicks: three taken, one declined with the measurement

All four checked against the tree first. Fixed in a10af9b.

Reuse Diagnostic::kind_suffix — taken.render.rs:221 already owns that rule with identical semantics (.filter(|kind| !kind.is_empty())) and the identical format string. Two owners of one user-visible text contract is exactly the drift risk you name.

It paid off beyond tidiness, which I only found by re-measuring: breaking the emptiness rule now fails two tests instead of one, because the human render shares the method. The row moved from parity replay only to parity replay and the human render. Deleting the duplicate strengthened coverage.

Narrow the nt-abs-path exemption — taken. You're right that (?!src) was far too broad. Probed before and after:

before → "D:\\src\\secret\\x.own" flagged=False ← leak, missed
"C:\\srcgen\\leak.own" flagged=False ← leak, missed
after → "D:\\src\\secret\\x.own" flagged=True
"C:\\srcgen\\leak.own" flagged=True
"C:\\src\\App\\Main.own" flagged=False ← the deliberate fixture path

I anchored the exemption as a negative lookahead on the whole prefix rather than your (?!src\\App\\) form, so the exemption is the one path the fixture actually carries and nothing that merely starts like it.

Remove the unused canonical helper — taken. Confirmed dead: only self-referential recursion, no caller in the module and no importer. The Rust replay owns canonicalization and the rationale already lives in the module docstring, so there was nothing to preserve.

Extend startLine >= 1 to the evidence projections — declined, measured.

You noted yourself that it "cannot fail" today. I tried to make it fail, and it can't be made to:

mutationresult
break emittable's line checkevidence steps appear, but phys still omits region — no bad startLine anywhere
break phys too (both gates down)code_scanning_acceptance_invariants_hold_for_every_casealready fails — via the primary locations of the file-level case

Both gates are shared between the primary and evidence paths, so any mutation that puts a bad startLine into relatedLocations puts one into locations first, where the existing assertion fires. The extension would add a line that no reachable defect can trip.

That is the specific thing P-022 rule 2 exists to refuse — an assertion that grows the coverage claim without growing coverage. Recorded in the module docs with the measurement, plus the condition that would make it worthwhile: if the evidence projection ever gets its own location builder rather than sharing phys, it stops being vacuous and should be added then.

One correction to my own work, unprompted

The mutation table in the module docs was stale. I measured it before adding the ingest-invariant test, then never re-ran it — so it claimed parity replay only for the region row that by then had two catchers. Re-measured in full and corrected.

During that re-run the severity mutation initially reported zero catchers, which would have meant a surviving mutant in the exact semantics this round is about. It was a shell-escaping typo producing a compile error; a mutation that fails to apply is indistinguishable from one nothing catches if you only grep for failing tests. Re-run cleanly, it is caught.

Gates after all of it: suite exit 0, ruff clean, mypy clean on 30 files, cargo fmt --check ok, clippy 0 items, cargo doc 0 warnings, workspace tests green.


Generated by Claude Code

@PhysShell
PhysShell merged commit 834341d into mainAug 7, 2026
41 checks passed
PhysShell pushed a commit that referenced this pull request Aug 7, 2026
Closes the 58 permissive documents and 9 category mismatches the
re-census opened. The fix is architectural: no arrangement of the
previous design could have passed.
The previous door ran `version gate -> all semantic gates -> serde for
all shapes`. That is a third check order, matching neither
implementation. BR-D1 validates each section COMPLETELY before the next
begins, interleaving shape and semantics inside it — so a `components`
shape failure outranks a `services` vocabulary failure, and within a
service `lifetime` outranks `name`. Hoisting semantics in front of serde
gets section-local controls right and every ordering control wrong.
So the strict door is now a sequential validator over the raw document:
own-ir/src/strict.rs primitives + one function per section, applied
in the reference's order
own-ir/src/protocol.rs the obligation ACCEPTANCE grammar
Not 47 transcribed `if`s — eight primitives, each encoding one of
Python's access idioms, and section validators that apply them:
objects / list d.get(k, []) as a container
name_slot isinstance(v, str) and v — a value facts join on
optional_string `is not None and not isinstance` — null tolerated
defaulted_string isinstance(d.get(k, "?"), str) — null rejected
defaulted_int int and not bool
string_array list of str
column the 1-based contract (#317), recursive over flow
sites the {type, file, line} record
The two string primitives are the place a single "policy for optional
fields" would be silently wrong: `resource` rejects an explicit null and
`source_provenance` accepts it, because the reference writes one as a
defaulted isinstance and the other as `is not None and ...`.
serde is now the CONSTRUCTOR, not the arbiter. Once the validator
accepts, a serde failure means a rule lives in the model rather than the
validator — its category and its ordering would both be accidental. That
is marked with a sentinel and asserted against by
`no_control_escapes_into_serde`, which is not decoration: five of the 27
mutations below are caught by it ALONE, because the model still enforced
the rule while the validator no longer did.
`OwnIr::validate` is now `to_value` + the same validator. It costs a
round-trip and buys single-copy-of-the-law — the property whose absence
already produced a false "mutation survived" in this PR, when a planted
mutation hit one copy of a duplicated check and the other caught it.
Protocol scope, exactly as agreed: `parse_protocol`, `parse_matcher`,
`parse_events`, `parse_method` — what the door ACCEPTS. Not the
lattice, the walker, matching, or verdicts. `protocols` and
`protocol_functions` stay raw `Value`s with pure validation beside them,
because nothing consumes a typed representation yet. Two of the grammar's
rules ("can never fire", "barrier equals opens") are well-formedness
rather than shape; they are recorded as `shape` with the taxonomy strain
written down rather than a seventh category invented unilaterally.
Final matrix over 191 controls:
agreed accept 29
agreed reject 162
python reject / rust accept 0
python accept / rust reject 0
category mismatch 0
27 mutations, all caught — one per mechanism, plus three that only
change a category while still rejecting, plus two that only change
ORDER. The order mutations matter most: they are the ones the previous
architecture could not have failed.
Also fixed: `is_none_or` needs Rust 1.82 and the workspace MSRV is 1.74;
three `private_intra_doc_links` of the same class #324 hit; and the
replay now reports all three failure rows in one assertion, since
sequential asserts showed only the first and would have made this a
two-round discovery.
Still open and deliberately not closed here: Python integers are
unbounded, so every line/column field accepts values past 64 bits and
Rust is over-strict above i64::MAX across seven field families. The
ledger pins the boundary where both agree. The range above it is a
Python-first defensive limit, not a reason to thread arbitrary-precision
integers through own-ir and the bridge.
Refs #250, #259.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM
Sign up for freeto 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.

2 participants

@PhysShell@claude