Skip to content

oracle: golden arch-graph fixture — run arch/ on real oracle-shaped data - #20

Merged
PhysShell merged 2 commits into
mainfrom
claude/sts-runtime-analysis-2mo4z9
Jun 26, 2026
Merged

oracle: golden arch-graph fixture — run arch/ on real oracle-shaped data#20
PhysShell merged 2 commits into
mainfrom
claude/sts-runtime-analysis-2mo4z9

Conversation

@PhysShell

@PhysShell PhysShell commented Jun 26, 2026

Copy link
Copy Markdown
Owner

What

The in-scope half of слой 2: validate the architecture pass (arch/) on a real, faithful graph of the LeakyOracle app instead of synthetic fixtures, and pin the exact graph.json contract the Own.NET-side Roslyn extractor (слой 2 proper) must emit for it.

Files (oracle/fixtures/)

  • graph.json — faithful arch-graph of the oracle: 8 internal types + the framework types they touch, real dependency edges (ownAudit/arch-graph/v1). This is the extractor contract.
  • rules.json — an MVVM layering profile (the default arch/rules.json is STS-specific). Allowed: Views → ViewModels → Services; reverse forbidden. Shows the rules engine is configurable per codebase, not STS-hardcoded.
  • test_oracle_arch.py — wired into CI (normal + -O).

What it proves

  • Clean faithful graph → 0 findings. The oracle is well-layered MVVM, so arch/ correctly stays silent — its real smells are lifetime/heap (OWN001, string-dup, XAML107), not architecture. Guards against false positives on clean code.
  • One planted MVVM inversion (WatchlistViewModel → MainWindow) → 3 rules fire: ARCH-MVVM-VM-VIEW (forbidden direction) + ARCH-CYCLE-TYPE ({MainWindow, WatchlistViewModel}) + ARCH-CYCLE-NS ({ViewModels, Views}). One edge, three distinct rule classes.
  • Drift guard: every internal graph node must point at a file that still exists under oracle/, so the golden can't silently diverge from the app it describes.

The degraded graph is kept in-memory (clean graph + one documented edge) — single source of truth, no duplicated fixture to drift.

Scope

Pure Python + fixtures, fully in-scope. When слой 2 lands in Own.NET, its extractor over LeakyOracle should reproduce graph.json (up to ids/loc), and this same pass runs on extracted data — the contract makes that swap mechanical.

🤖 Generated with Claude Code


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added new architecture fixture data (golden graph and rule set) for exercising dependency and MVVM layering expectations.
    • Introduced documentation describing the “Oracle arch fixtures” and the expected clean vs. failing outcomes.
  • Tests

    • Added an architecture test runner that validates findings, including planted inversion behavior and consistent result shape.
    • Updated CI to execute the new test in both standard and optimized (-O) modes.

The in-scope half of слой 2: validate the architecture pass on a real, faithful
graph of the LeakyOracle app (8 internal types + the framework types they touch)
instead of synthetic fixtures, and pin the graph.json contract the Own.NET-side
Roslyn extractor must emit for the oracle.

 - oracle/fixtures/graph.json — faithful arch-graph of the oracle (ownAudit/arch-graph/v1).
 - oracle/fixtures/rules.json — an MVVM layering profile (Views -> ViewModels ->
   Services; reverse forbidden), showing the rules engine is configurable per
   codebase, not STS-hardcoded.
 - test_oracle_arch.py: clean faithful graph -> 0 findings (arch/ doesn't cry wolf
   on well-layered MVVM; the oracle's smells are lifetime/heap, not architecture).
   One planted MVVM inversion (WatchlistViewModel -> MainWindow) -> exactly
   ARCH-MVVM-VM-VIEW + ARCH-CYCLE-TYPE + ARCH-CYCLE-NS. Also guards the graph
   against drift (every internal node must point at a real file under oracle/).

Wired into CI (normal + -O). All suites green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
@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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: eeffda6d-46df-4c2b-be96-e9a4f7ad86a5

📥 Commits

Reviewing files that changed from the base of the PR and between 2f388d2 and 0955244.

📒 Files selected for processing (2)
  • oracle/fixtures/README.md
  • oracle/fixtures/graph.json
✅ Files skipped from review due to trivial changes (1)
  • oracle/fixtures/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • oracle/fixtures/graph.json

📝 Walkthrough

Walkthrough

This PR adds Oracle architecture fixtures, a dedicated golden test module, documentation for the fixture contract and usage, and CI steps that run the new test script in normal and optimized Python mode.

Changes

Oracle arch fixtures and CI

Layer / File(s) Summary
Graph and rules fixtures
oracle/fixtures/graph.json, oracle/fixtures/rules.json
Defines the oracle graph schema, node and edge data, and the MVVM/cycle/coupling rule fixture set.
Architecture test script
oracle/fixtures/test_oracle_arch.py
Loads the fixtures, runs the rules engine, checks the clean graph result, validates oracle node/file coverage, and asserts the planted inversion produces the expected findings and JSON shape.
Docs, entrypoint, and CI wiring
oracle/fixtures/README.md, oracle/fixtures/test_oracle_arch.py, .github/workflows/ci.yml
Documents the fixture contract and usage, adds the script entrypoint, and runs the test script in both normal and -O CI passes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PhysShell/OwnAudit#9: Adds the same Oracle architecture test script to CI and reruns it under python -O.

Poem

I hopped through graphs with ears aloft,
And found the MVVM path quite soft.
One back edge twitched; the tests all sang,
While CI ran twice with a cheerful bang. 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 summarizes the main change: adding a golden arch-graph fixture and running arch on real oracle-shaped data.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/sts-runtime-analysis-2mo4z9

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

@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: 2f388d2137

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

{ "from": "T:LeakyOracle.Views.MainWindow", "to": "T:LeakyOracle.ViewModels.QuoteRow", "kind": "depends" },
{ "from": "T:LeakyOracle.Views.MainWindow", "to": "T:Avalonia.Controls.Window", "kind": "depends" },

{ "from": "T:LeakyOracle.Services.MarketDataService", "to": "T:System.EventHandler`1", "kind": "depends" },

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add the missing System.String dependencies

Under the current extractor contract this fixture is not reproducible: docs/arch-graph.md:134-136 says method parameter/return types and generic type arguments are dependencies, but MarketDataService records only EventHandler<T> here while the real source has EventHandler<string> and Tick(string symbol). A Roslyn extractor that follows the documented contract will emit a T:System.String external node/edge, so this “exact” golden graph will immediately drift or force the extractor to drop valid dependencies.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fair and correct — fixed in 0955244. You caught a framing bug: I called this an "exact" golden while hand-authoring can't be byte-exact on BCL leaves, and I'd inconsistently kept EventHandler<T> but dropped its <string> arg. Two-part fix: (1) added System.String (external node) + edges from the six types that reference it via EventHandler<string>/Tick(string)/QuoteRow(string)/string[] — and String earns its place since string duplication is one of the oracle's own smells; (2) reframed the contract — the internal type graph is what the extractor must reproduce; external nodes list the meaningful framework types, and pure-infrastructure leaves (Int32/Console/GC/attributes/arrays) are elided, so a faithful extractor emits a superset. The test already pins only the internal subgraph, not external completeness. Still 0 findings on the clean graph; 4/4 normal + -O.


Generated by Claude Code

@coderabbitai coderabbitai 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.

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 `@oracle/fixtures/graph.json`:
- Line 2: The fixture comment in the graph JSON references a non-existent
graph.degraded.json, which conflicts with the documented in-memory degraded
variant. Update the _comment text to remove that file reference and instead
point readers to the in-memory regression variant described in the repo docs,
keeping the wording aligned with the intent of Oracle/LeakyOracle and the Roslyn
extractor contract.
🪄 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: 912eff5e-6590-4c47-9aa0-2c52bd6cafc8

📥 Commits

Reviewing files that changed from the base of the PR and between 3aaeea7 and 2f388d2.

📒 Files selected for processing (5)
  • .github/workflows/ci.yml
  • oracle/fixtures/README.md
  • oracle/fixtures/graph.json
  • oracle/fixtures/rules.json
  • oracle/fixtures/test_oracle_arch.py

Comment thread oracle/fixtures/graph.json Outdated
… P2, CodeRabbit)

Codex: the documented extractor contract (docs/arch-graph.md) counts param/return
types and generic args as dependencies, so a faithful extractor emits System.String
edges for EventHandler<string>/Tick(string)/QuoteRow(string)/string[] — which the
golden omitted. Add System.String (an external node) + its edges from the six types
that reference it. String earns its place: string duplication is one of the oracle's
own smells.

Reframe to match reality: hand-authoring can't be byte-exact on BCL leaves, so the
INTERNAL type graph is the contract the extractor must reproduce; external nodes list
the meaningful framework types, and pure-infrastructure leaves (Int32, Console, GC,
attributes, arrays) are elided — a faithful extractor emits a superset. The test
already pins only the internal subgraph, not external completeness.

CodeRabbit: the _comment's stale reference to a non-existent graph.degraded.json is
removed by the same reframe (the regression variant is in-memory in the test).

Still 0 findings on the clean graph; test 4/4 normal + -O.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
@PhysShell
PhysShell merged commit 247bda0 into main Jun 26, 2026
2 of 3 checks passed
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