Skip to content

audit: add build-free XAML analyzer (Phase 1, markup-only)#114

Merged
PhysShell merged 4 commits into
mainfrom
claude/oun-net-html-analyzer-drff2t
Jun 26, 2026
Merged

audit: add build-free XAML analyzer (Phase 1, markup-only)#114
PhysShell merged 4 commits into
mainfrom
claude/oun-net-html-analyzer-drff2t

Conversation

@PhysShell

@PhysShell PhysShell commented Jun 26, 2026

Copy link
Copy Markdown
Owner

Implements Phase 1 of the XAML analyzer design note as a build-free static
runner feeding the existing audit/ pipeline as a second fact source (not a
parallel linter). Pure stdlib XML via an expat tree builder, so it has no
toolchain prerequisite and runs on Linux CI like the other build-free runners.

  • audit/static/tools/xaml_check.py: line-preserving parse + rules
    XAML101/102/103/104/106/107/108/109 (virtualization-off, per-keystroke
    binding, template complexity, Freezable/x:Shared/DynamicResource/merged-
    dictionary perf), with the design note's exception lists. Emits the canonical
    SARIF record; file-level findings stay file-level (line 0). WPF-only rules
    short-circuit on Avalonia. Embedded-fixture selftest covers every rule, the
    line-preservation requirement, and the SARIF round-trip.
  • run_static.py: wire "xaml" into the build-free tier (+ integration selftest).
  • desktop-wpf profile: add xaml to build_free; category 8 (broken
    virtualization) is no longer NO-TOOL static now that XAML107/109 cover it.
  • taxonomy: map XAML rules to Plan.md §2 categories (6/8/9) + a XAML1* safety
    net; add category-8 severity (P2).
  • docs/notes/xaml-analyzer-design.md: the design note, alongside the impl.
  • ci.yml + audit/README.md: run and document the new selftest.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb

Summary by CodeRabbit

  • New Features

    • Added build-free XAML analysis coverage to the audit pipeline, with results now included in static scoring and reports.
    • Expanded support for XAML and related markup files with rule-based findings and improved handling of WPF/Avalonia markup.
  • Documentation

    • Updated audit docs with the new XAML analysis tier, supported rules, and rollout status.
  • Tests

    • Added self-tests for the new XAML analysis path in CI and local validation.

claude added 2 commits June 26, 2026 10:04
Implements Phase 1 of the XAML analyzer design note as a build-free static
runner feeding the existing audit/ pipeline as a second fact source (not a
parallel linter). Pure stdlib XML via an expat tree builder, so it has no
toolchain prerequisite and runs on Linux CI like the other build-free runners.

- audit/static/tools/xaml_check.py: line-preserving parse + rules
  XAML101/102/103/104/106/107/108/109 (virtualization-off, per-keystroke
  binding, template complexity, Freezable/x:Shared/DynamicResource/merged-
  dictionary perf), with the design note's exception lists. Emits the canonical
  SARIF record; file-level findings stay file-level (line 0). WPF-only rules
  short-circuit on Avalonia. Embedded-fixture selftest covers every rule, the
  line-preservation requirement, and the SARIF round-trip.
- run_static.py: wire "xaml" into the build-free tier (+ integration selftest).
- desktop-wpf profile: add xaml to build_free; category 8 (broken
  virtualization) is no longer NO-TOOL static now that XAML107/109 cover it.
- taxonomy: map XAML rules to Plan.md §2 categories (6/8/9) + a XAML1* safety
  net; add category-8 severity (P2).
- docs/notes/xaml-analyzer-design.md: the design note, alongside the impl.
- ci.yml + audit/README.md: run and document the new selftest.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb
… note

Folds in the cheap, markup-detectable rules surfaced by the research-comb
feedback, and pins the Phase-2 architecture decision in the design note.

New Phase-1 rules in xaml_check.py (all stdlib, all selftested):
- XAML110 ImageDecodedAtFullSize  — thumbnail Image with a plain-URI Source
  (no decode-to-size possible without BitmapImage/DecodePixelWidth). WPF-only.
- XAML111 LayoutTransformSuspicious — LayoutTransform forces a measure/arrange
  pass; RenderTransform is cheaper unless layout must reflow. WPF-only candidate.
- XAML112 TemplateBindingOpportunity — a TemplatedParent Binding with no
  converter/two-way inside a ControlTemplate could be {TemplateBinding}.
- XAML113 InlineFreezableDuplication — identical inline brush/geometry declared
  more than once; hoist to one shared keyed resource (the inline case of XAML100).

- taxonomy: map XAML110/112/113 -> cat 9, XAML111 -> cat 8 (layout cost).
- README + design-note catalogue: document the four rules.
- design note: add "Phase 2 mechanics — the binding-path join" section. It keeps
  the link-extractor in own-check (canonical in Own.NET), NOT a parallel
  OwnAudit.Xaml C# project (that is the post-lift-out product form), and ties the
  static binding-join candidate to runtime confirmation via correlate.py.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR adds a build-free XAML analyzer, maps its rule IDs into audit categories, and wires the new tier into static execution, CI selftests, and documentation.

Changes

XAML analyzer integration

Layer / File(s) Summary
Phase plan and category mapping
docs/notes/xaml-analyzer-design.md, audit/static/taxonomy/categories.yml
The phased XAML analyzer plan is documented, and XAML rule IDs are mapped into the existing audit categories and severity levels.
Parser scaffold and analysis contract
audit/static/tools/xaml_check.py
The module adds the XML tree model, line-preserving parsing, finding records, Avalonia detection, document analysis, and SARIF serialization.
Phase-1 heuristics
audit/static/tools/xaml_check.py
The markup rules for virtualization, template complexity, bindings, resource reuse, image sizing, and duplicate Freezables are implemented.
Template and layout rules
audit/static/tools/xaml_check.py
The remaining Phase-1 rules cover TemplatedParent bindings, inline Freezable duplication, LayoutTransform usage, and rule registration order.
Runner, CLI, and selftests
audit/static/tools/xaml_check.py
The analyzer scans XAML files, writes xaml-check.sarif, exposes the CLI, and includes selftests for parsing and SARIF output.
Build-free profile wiring and docs
audit/config/profiles/desktop-wpf.yml, audit/static/run_static.py, audit/README.md, .github/workflows/ci.yml
The desktop WPF profile enables the xaml tier, static aggregation consumes its SARIF, and the README and CI selftests reference the new runner.

Sequence Diagram(s)

sequenceDiagram
  participant Profile as desktop-wpf.yml
  participant Static as run_static.py
  participant Xaml as audit/static/tools/xaml_check.py
  participant Sarif as xaml-check.sarif
  Profile->>Static: enables build_free xaml
  Static->>Xaml: run_xaml_check(target, out_dir)
  Xaml->>Sarif: write SARIF results
  Static->>Static: append SARIF to sarif_inputs
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • PhysShell/Own.NET#100: Adds the static audit orchestrator, taxonomy, and the new xaml build-free tier that this PR extends with analyzer implementation and wiring.
  • PhysShell/Own.NET#101: Updates static aggregate processing, which directly consumes the SARIF output produced by xaml_check.py.

Poem

I hopped through XAML leaves so bright,
and found a build-free carrot light. 🐰
The audit burrow hummed along,
with SARIF marching brisk and strong,
thump-thump — the markup sings tonight.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding a build-free, Phase 1 markup-only XAML analyzer to audit.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check 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/oun-net-html-analyzer-drff2t

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

No behavior change — the 31-check selftest still passes. Resolves the
ruff failures on PR #114:
- B033: drop the duplicate "ListView" in ITEMS_CONTROLS
- UP037: unquote the Node self-referencing annotations (from __future__ is on)
- UP035: import Callable from collections.abc, not typing
- F841: remove the dead `flags` dict in _rule_virtualization
- E501: wrap the over-long SARIF round-trip line in the selftest

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0f0a3b552b

ℹ️ 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".

Comment thread audit/static/tools/xaml_check.py Outdated
Comment thread audit/static/tools/xaml_check.py Outdated
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{"name":"HttpError","status":500,"request":{"method":"PATCH","url":"https://api.github.com/repos/PhysShell/Own.NET/issues/comments/4808638894","headers":{"accept":"application/vnd.github.v3+json","user-agent":"octokit.js/0.0.0-development octokit-core.js/7.0.6 Node.js/24","content-type":"application/json; charset=utf-8"},"body":{"body":"<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- review_stack_entry_start -->\n\n[![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/PhysShell/Own.NET/pull/114?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)\n\n<!-- review_stack_entry_end -->\n<!-- This is an auto-generated comment: failure by coderabbit.ai -->\n\n> [!CAUTION]\n> ## Review failed\n> \n> The head commit changed during the review from 0f0a3b552b600413eeda6974ef4db14a84d24add to 6666c81f4f2061286f8e23e04f4e2883815d5c9d.\n\n<!-- end of auto-generated comment: failure by coderabbit.ai -->\n\n<!-- finishing_touch_checkbox_start -->\n\n<details>\n<summary>✨ Finishing Touches</summary>\n\n<details>\n<summary>📝 Generate docstrings</summary>\n\n- [ ] <!-- {\"checkboxId\": \"7962f53c-55bc-4827-bfbf-6a18da830691\"} --> Create stacked PR\n- [ ] <!-- {\"checkboxId\": \"3e1879ae-f29b-4d0d-8e06-d12b7ba33d98\"} --> Commit on current branch\n\n</details>\n<details>\n<summary>🧪 Generate unit tests (beta)</summary>\n\n- [ ] <!-- {\"checkboxId\": \"f47ac10b-58cc-4372-a567-0e02b2c3d479\", \"radioGroupId\": \"utg-output-choice-group-unknown_comment_id\"} -->   Create PR with unit tests\n- [ ] <!-- {\"checkboxId\": \"6ba7b810-9dad-11d1-80b4-00c04fd430c8\", \"radioGroupId\": \"utg-output-choice-group-unknown_comment_id\"} -->   Commit unit tests in branch `claude/oun-net-html-analyzer-drff2t`\n\n</details>\n\n</details>\n\n<!-- finishing_touch_checkbox_end -->\n<!-- tips_start -->\n\n---\n\n\n\n\n<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>\n\n<!-- tips_end -->"},"request":{"retryCount":3,"signal":{},"retries":3,"retryAfter":16}}}

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{"name":"HttpError","status":500,"request":{"method":"PATCH","url":"https://api.github.com/repos/PhysShell/Own.NET/issues/comments/4808638894","headers":{"accept":"application/vnd.github.v3+json","user-agent":"octokit.js/0.0.0-development octokit-core.js/7.0.6 Node.js/24","content-type":"application/json; charset=utf-8"},"body":{"body":"<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- review_stack_entry_start -->\n\n[![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/PhysShell/Own.NET/pull/114?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)\n\n<!-- review_stack_entry_end -->\n<!-- This is an auto-generated comment: review in progress by coderabbit.ai -->\n\n> [!NOTE]\n> Currently processing new changes in this PR. This may take a few minutes, please wait...\n> \n> <details>\n> <summary>⚙️ Run configuration</summary>\n> \n> **Configuration used**: Path: .coderabbit.yaml\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Pro Plus\n> \n> **Run ID**: `f1c8c199-00a4-4337-87bd-cab6eb150850`\n> \n> </details>\n> \n> <details>\n> <summary>📥 Commits</summary>\n> \n> Reviewing files that changed from the base of the PR and between 14934ae2e62d0ba961f859e27c4ad40ad91f88fb and 0f0a3b552b600413eeda6974ef4db14a84d24add.\n> \n> </details>\n> \n> <details>\n> <summary>📒 Files selected for processing (7)</summary>\n> \n> * `.github/workflows/ci.yml`\n> * `audit/README.md`\n> * `audit/config/profiles/desktop-wpf.yml`\n> * `audit/static/run_static.py`\n> * `audit/static/taxonomy/categories.yml`\n> * `audit/static/tools/xaml_check.py`\n> * `docs/notes/xaml-analyzer-design.md`\n> \n> </details>\n> \n> ```ascii\n>  ___________________________________________________________________________________\n> < The best way to predict the future is to implement it. - David Heinemeier Hansson >\n>  -----------------------------------------------------------------------------------\n>   \\\n>    \\   \\\n>         \\ /\\\n>         ( )\n>       .( o ).\n> ```\n\n<!-- end of auto-generated comment: review in progress by coderabbit.ai -->\n\n<!-- finishing_touch_checkbox_start -->\n\n<details>\n<summary>✨ Finishing Touches</summary>\n\n<details>\n<summary>📝 Generate docstrings</summary>\n\n- [ ] <!-- {\"checkboxId\": \"7962f53c-55bc-4827-bfbf-6a18da830691\"} --> Create stacked PR\n- [ ] <!-- {\"checkboxId\": \"3e1879ae-f29b-4d0d-8e06-d12b7ba33d98\"} --> Commit on current branch\n\n</details>\n<details>\n<summary>🧪 Generate unit tests (beta)</summary>\n\n- [ ] <!-- {\"checkboxId\": \"f47ac10b-58cc-4372-a567-0e02b2c3d479\", \"radioGroupId\": \"utg-output-choice-group-unknown_comment_id\"} -->   Create PR with unit tests\n- [ ] <!-- {\"checkboxId\": \"6ba7b810-9dad-11d1-80b4-00c04fd430c8\", \"radioGroupId\": \"utg-output-choice-group-unknown_comment_id\"} -->   Commit unit tests in branch `claude/oun-net-html-analyzer-drff2t`\n\n</details>\n\n</details>\n\n<!-- finishing_touch_checkbox_end -->\n<!-- tips_start -->\n\n---\n\n\n\n\n<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>\n\n<!-- tips_end -->"},"request":{"retryCount":3,"signal":{},"retries":3,"retryAfter":16}}}

…view)

Two correctness fixes from the Codex review on PR #114:

- _resource_dictionaries now also yields implicit <X.Resources> property
  elements (the common WPF syntax: keyed resources as direct children with no
  <ResourceDictionary> wrapper). Without this, XAML101/102/106 only fired on
  files that explicitly wrapped resources in <ResourceDictionary>, silently
  missing most real WPF files despite the claimed category-9 coverage. No double
  counting: when <X.Resources> wraps an explicit <ResourceDictionary>, the
  wrapper has no direct keyed children, so the inner dictionary is still the one
  that matches.
- run_xaml_check reads bytes and lets expat honor the BOM / XML-declaration
  encoding, instead of forcing read_text(utf-8). Legacy WPF XAML saved as UTF-16
  was corrupted by the forced decode, parse_xaml returned None, and the file was
  silently skipped while still counted as scanned.

Selftest grows to 34 checks: an implicit-<X.Resources> dictionary (with line
preservation) and a UTF-16 (BOM + declaration) document that must parse.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb
Sign up for free to 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