Skip to content

docs: Arm 3 (Fix) design — wire mass appliers + audit-grade safety + OWN gap - #1

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

docs: Arm 3 (Fix) design — wire mass appliers + audit-grade safety + OWN gap#1
PhysShell merged 4 commits into
mainfrom
claude/sts-runtime-analysis-2mo4z9

Conversation

@PhysShell

@PhysShell PhysShell commented Jun 25, 2026

Copy link
Copy Markdown
Owner

Grounds the fix arm in the real STS run (72569 findings): 85% come from
analyzers shipping code fixes, so we wire roslynator fix / dotnet format
rather than build an engine. Documents the off-the-shelf landscape
(appliers vs OpenRewrite/Copilot-Autofix + why they don't fit), the
risk-tier gate, the safety contract (dry-run -> diff -> re-audit
no-new-findings -> tier gate), the OWN001/OWN014 bespoke fixer, the
MSBuild build-wall + fix-spike, and a CI/Linux-native first slice.

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

Summary by CodeRabbit

  • New Features

    • Added a guided fix workflow that reviews changes before applying them, checks for regressions, and blocks unsafe outcomes.
    • Introduced risk-based handling so some fixes can be auto-applied while others require review.
  • Documentation

    • Expanded project docs with a new “fix” design guide and updated navigation links.
  • Tests

    • Added end-to-end coverage for fix success, rejection, no-op, and review routing scenarios.

claude added 3 commits June 25, 2026 14:15
…OWN gap

Grounds the fix arm in the real STS run (72569 findings): 85% come from
analyzers shipping code fixes, so we wire roslynator fix / dotnet format
rather than build an engine. Documents the off-the-shelf landscape
(appliers vs OpenRewrite/Copilot-Autofix + why they don't fit), the
risk-tier gate, the safety contract (dry-run -> diff -> re-audit
no-new-findings -> tier gate), the OWN001/OWN014 bespoke fixer, the
MSBuild build-wall + fix-spike, and a CI/Linux-native first slice.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
The audit-grade wrapper from docs/fix-arm.md §4, end to end on IDISP001:
select -> dry-run -> diff -> apply -> re-audit -> assert no new findings ->
tier gate. We don't ship a fix engine; roslynator fix / dotnet format are
the appliers behind a pluggable interface (Replay adapters for CI fixtures,
real Roslynator/DotnetFormat/ScriptReaudit adapters for the .NET stand).

- fixarm/tiers.py     risk tiers T1 auto / T2 review / T3 unfixable / T4 bespoke
- fixarm/orchestrate.py  select, diff two audit runs, no-new-findings gate, ledger
- fixarm/appliers.py  Replay* (Linux) + Roslynator/DotnetFormat/ScriptReaudit (stand)
- fixarm/cli.py       run a fixture through the wrapper
- fixtures + tests    golden before/after; 6/6 on bare python3

Crux test: a fix that removes the target but introduces a new finding is
rejected, never committed. CI/Linux-native; run-over-STS stays Windows-bound.

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

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds the fix-arm documentation, tiering rules, orchestration wrapper, replay and real applier adapters, a CLI entrypoint, IDISP001 fixtures, and orchestration tests.

Changes

Fix arm workflow

Layer / File(s) Summary
Design docs and package framing
docs/fix-arm.md, fix/README.md, README.md, PLAN.md, fix/fixarm/__init__.py
Repo docs and the package docstring describe the fix arm, its placement, workflow, and rollout status.
Tier routing rules
fix/fixarm/tiers.py
Tier constants, rule classification, and gate mapping define T1-T4 routing and the available exit gates.
Orchestration core
fix/fixarm/orchestrate.py
Finding, JSON ingestion, diffing, result codes, and run_fix() implement the rule-scoped select -> dry-run -> apply -> re-audit flow.
Replay adapters
fix/fixarm/appliers.py
Replay appliers compare fixture before and after trees, apply fixture output to a workdir, and replay recorded findings JSON.
Real adapters and CLI
fix/fixarm/appliers.py, fix/fixarm/cli.py
Roslynator, dotnet format, and PowerShell re-audit adapters are wired into the CLI entrypoint.
Fixtures and tests
fix/fixtures/idisp001-*/..., fix/tests/test_orchestrate.py
IDISP001 fixtures and orchestration tests cover tier mapping, diffing, result states, and the script runner.

Sequence Diagram(s)

sequenceDiagram
  participant fixarm_cli_main as "fixarm.cli.main"
  participant run_fix as "run_fix"
  participant ReplayApplier as "ReplayApplier"
  participant ReplayReaudit as "ReplayReaudit"

  fixarm_cli_main->>fixarm_cli_main: load before.findings.json
  fixarm_cli_main->>fixarm_cli_main: seed temporary workdir
  fixarm_cli_main->>run_fix: run_fix(before, workdir, rule, ReplayApplier, ReplayReaudit)
  run_fix->>ReplayApplier: dry_run(workdir, rule)
  run_fix->>ReplayApplier: apply(workdir, rule)
  run_fix->>ReplayReaudit: load after.findings.json
  ReplayReaudit-->>run_fix: parsed findings
  run_fix-->>fixarm_cli_main: FixResult and ledger()
  fixarm_cli_main->>fixarm_cli_main: print ledger and rejection details
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

I hopped through tiers with whiskers bright,
Then checked each diff by moonbeam light.
Dry runs, re-audits, soft and neat,
Kept no-new-findings in my feet.
Hooray, the fix arm twinkles white!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.87% 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 matches the PR’s main theme: Arm 3 fix-arm design plus mass appliers, safety gating, and OWN-specific gaps.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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/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: 6a7c984e38

ℹ️ 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 fix/fixarm/orchestrate.py Outdated
Comment on lines +158 to +160
if introduced:
return FixResult(rule, REJECTED, tier, gate, targeted_removed, introduced,
diff, len(selected))

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 Revert rejected fixes before returning

When the real applier introduces a new finding, this path returns REJECTED after applier.apply() has already mutated workdir, but nothing rolls the checkout back. On the Windows stand this leaves the rejected patch in the target tree, so a later rule run or a manual commit can include the regression even though the ledger says it was rejected/never committed; please restore the tree before returning REJECTED or apply in a disposable worktree.

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.

@codex Fixed in 56ec0db. Added an Applier.revert hook; run_fix now restores the worktree on every non-success path — REJECTED, NO_EFFECT, and a re-audit that throws (Replay restores before/, real adapters do git checkout -- . + git clean -fd). FixResult.reverted records it in the ledger. New test: test_regression_is_rejected_and_tree_reverted asserts the rejected patch is gone and the file is back to its before/ state.


Generated by Claude Code

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment thread fix/fixarm/orchestrate.py Outdated
"""Identity for set-diffing two audit runs. Mirrors the audit's own
matching: same rule + same basename + line within tolerance. With
line_tol>0 the line is bucketed so a fix that shifts lines still matches."""
ln = self.line if line_tol <= 0 else self.line // (line_tol + 1)

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 Implement line tolerance by distance

When --line-tol is used, bucketing with line // (tol + 1) does not mean “within tolerance”: for example, with line_tol=1, the same finding moving from line 1 to line 2 gets different keys and is reported as removed plus introduced. Any valid fix that shifts a diagnostic across a bucket boundary will be rejected as a new finding despite being within the configured tolerance; compare line distances during matching instead of hashing lines into buckets.

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.

@codex Fixed in 56ec0db. Replaced the line // (line_tol + 1) bucketing with true within-distance matching: within each (rule, basename) group, before/after findings are matched greedily by absolute line distance ≤ line_tol, so a diagnostic that shifts by ≤ tol is the same site, not removed + introduced. Test: test_line_tolerance_matches_within_distance (covers the line 11→12, tol=1 boundary case).


Generated by Claude Code

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

🤖 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 `@fix/fixarm/appliers.py`:
- Around line 59-63: The apply method in Applier only copies the after_dir
overlay, so files present only in before_dir are never removed from workdir and
the replayed tree can drift from dry_run() results. Update Applier.apply to
reconcile deletions by identifying paths that exist in before_dir but not
after_dir and removing them from workdir before or during the copy loop, while
preserving the existing copy behavior for files from _walk(self.after_dir).

In `@fix/fixarm/cli.py`:
- Around line 26-34: The seeded temporary workspace created by _seed_workdir()
is never cleaned up, so repeated runs leak temp directories. Update main() to
track the directory returned by _seed_workdir() and remove it in a finally block
after the fixture run completes, ensuring cleanup happens even on errors. Use
the existing _seed_workdir() and main() flow to locate where the temp tree is
created and where teardown should be added.

In `@fix/fixarm/orchestrate.py`:
- Around line 32-37: The line tolerance logic in Finding.key() is using bucketed
division, which does not actually preserve “within tolerance” matching and can
split nearby lines into different identities. Update the diffing flow in
Finding.key() and diff_findings() so findings are compared within each (rule,
basename) group using absolute line-distance checks against line_tol rather than
encoding the line into the key. Ensure the matching logic keeps close lines
grouped together without fabricating introduced findings at bucket boundaries.
- Around line 94-96: The non-OK apply paths in the fix-arm flow leave the
working tree mutated, so make the apply operation disposable or add rollback to
the Applier path. Update the orchestration around the apply/audit logic in
orchestrate.py so that the REJECTED and NO_EFFECT branches, plus any re-audit
failure after apply, restore workdir before returning a FixResult. Use the
existing status symbols OK, REJECTED, and NO_EFFECT to ensure every non-success
exit from the apply flow triggers cleanup.

In `@fix/README.md`:
- Around line 7-9: The README snippet is using a bare fenced code block, which
triggers markdownlint MD040 because the content is shell syntax. Update the
fenced block around the pipeline in README.md to use a Bash language tag, and
keep the existing content under the same block so the shell command is clearly
identified. Use the fenced snippet near the select(rule) → dry-run → diff →
apply → re-audit → assert NO new findings → tier gate text to locate it.
- Around line 13-16: The README demo command is not runnable from the repo root
because the module path for fixarm is unresolved; update the documented command
in the README snippet to use a root-compatible invocation, or explicitly include
the package path setup needed to find fixarm from the checkout root. Refer to
the demo command example in the README and make sure the instructions clearly
work without requiring the user to manually cd into fix/ first.

In `@fix/tests/test_orchestrate.py`:
- Around line 42-54: The `_run` helper in test_orchestrate leaks the temporary
workdir created by `_workdir()` because the `finally` block is a no-op and the
comment about OS cleanup is incorrect. Update `_run` to guarantee cleanup of
`wd` after `run_fix` completes, either by deleting the directory in the
`finally` path or by turning `_run` into a context manager so callers can still
inspect the tree before it is removed. Keep the fix localized to `_run`,
`_workdir`, and the `tempfile.mkdtemp()`-backed workdir handling.
🪄 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: 00eb3550-50c0-4eb1-a044-3577968830ea

📥 Commits

Reviewing files that changed from the base of the PR and between 4dce713 and 6a7c984.

📒 Files selected for processing (19)
  • .gitignore
  • PLAN.md
  • README.md
  • docs/fix-arm.md
  • fix/README.md
  • fix/fixarm/__init__.py
  • fix/fixarm/appliers.py
  • fix/fixarm/cli.py
  • fix/fixarm/orchestrate.py
  • fix/fixarm/tiers.py
  • fix/fixtures/idisp001-clean/after.findings.json
  • fix/fixtures/idisp001-clean/after/Core/Mail.cs
  • fix/fixtures/idisp001-clean/before.findings.json
  • fix/fixtures/idisp001-clean/before/Core/Mail.cs
  • fix/fixtures/idisp001-regress/after.findings.json
  • fix/fixtures/idisp001-regress/after/Core/Mail.cs
  • fix/fixtures/idisp001-regress/before.findings.json
  • fix/fixtures/idisp001-regress/before/Core/Mail.cs
  • fix/tests/test_orchestrate.py

Comment thread fix/fixarm/appliers.py
Comment on lines +59 to +63
def apply(self, workdir: str, rule: str) -> None:
for rel in _walk(self.after_dir):
dst = os.path.join(workdir, rel)
os.makedirs(os.path.dirname(dst), exist_ok=True)
shutil.copy2(os.path.join(self.after_dir, rel), dst)

@coderabbitai coderabbitai Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Replay apply cannot model file deletions.

dry_run() diffs the union of the before/ and after/ trees, so fixtures can represent removed files, but apply() only overlays after/. Files that exist only in before/ stay in workdir, so the replayed tree can diverge from the recorded after.findings.json.

Suggested fix
 def apply(self, workdir: str, rule: str) -> None:
-    for rel in _walk(self.after_dir):
+    before_files = set(_walk(self.before_dir))
+    after_files = set(_walk(self.after_dir))
+    for rel in before_files - after_files:
+        stale = os.path.join(workdir, rel)
+        if os.path.exists(stale):
+            os.remove(stale)
+    for rel in sorted(after_files):
         dst = os.path.join(workdir, rel)
         os.makedirs(os.path.dirname(dst), exist_ok=True)
         shutil.copy2(os.path.join(self.after_dir, rel), dst)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def apply(self, workdir: str, rule: str) -> None:
for rel in _walk(self.after_dir):
dst = os.path.join(workdir, rel)
os.makedirs(os.path.dirname(dst), exist_ok=True)
shutil.copy2(os.path.join(self.after_dir, rel), dst)
def apply(self, workdir: str, rule: str) -> None:
before_files = set(_walk(self.before_dir))
after_files = set(_walk(self.after_dir))
for rel in before_files - after_files:
stale = os.path.join(workdir, rel)
if os.path.exists(stale):
os.remove(stale)
for rel in sorted(after_files):
dst = os.path.join(workdir, rel)
os.makedirs(os.path.dirname(dst), exist_ok=True)
shutil.copy2(os.path.join(self.after_dir, rel), dst)
🤖 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 `@fix/fixarm/appliers.py` around lines 59 - 63, The apply method in Applier
only copies the after_dir overlay, so files present only in before_dir are never
removed from workdir and the replayed tree can drift from dry_run() results.
Update Applier.apply to reconcile deletions by identifying paths that exist in
before_dir but not after_dir and removing them from workdir before or during the
copy loop, while preserving the existing copy behavior for files from
_walk(self.after_dir).

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.

@coderabbitai Applied in 56ec0db. ReplayApplier.apply now removes before-only files before overlaying after/, so a fix that deletes a file is modelled and the replayed tree matches after.findings.json. The same reconciliation backs the new revert.


Generated by Claude Code

Comment thread fix/fixarm/cli.py
Comment on lines +26 to +34
def _seed_workdir(before_dir: str) -> str:
d = tempfile.mkdtemp(prefix="fixarm-")
for dp, _, names in os.walk(before_dir):
for n in names:
full = os.path.join(dp, n)
dst = os.path.join(d, os.path.relpath(full, before_dir))
os.makedirs(os.path.dirname(dst), exist_ok=True)
shutil.copy2(full, dst)
return d

@coderabbitai coderabbitai Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Clean up the seeded temp workspace.

_seed_workdir() creates a fresh mkdtemp(), but main() never removes it. Repeated fixture runs will leak temp trees in CI and local dev.

Suggested fix
-    before = load_findings(os.path.join(args.fixture, "before.findings.json"))
-    wd = _seed_workdir(os.path.join(args.fixture, "before"))
-    res = run_fix(
-        before=before, workdir=wd, rule=args.rule,
-        applier=ReplayApplier(args.fixture),
-        reaudit=ReplayReaudit(os.path.join(args.fixture, "after.findings.json")),
-        line_tol=args.line_tol,
-    )
-
-    print(json.dumps(res.ledger(), indent=2))
-    if res.status == REJECTED:
-        print(f"\nREJECTED — fix introduced {len(res.introduced)} new finding(s):", file=sys.stderr)
-        for f in res.introduced:
-            print(f"  + {f.rule} {f.path}:{f.line}  {f.message}", file=sys.stderr)
-    elif res.status == OK and (args.show_diff or res.gate != "auto-commit"):
-        print(f"\n--- reviewable patch (gate: {res.gate}) ---\n{res.diff}")
-
-    return {OK: 0, NO_OP: 0, UNFIXABLE: 0, REJECTED: 2, NO_EFFECT: 3}.get(res.status, 0)
+    before = load_findings(os.path.join(args.fixture, "before.findings.json"))
+    wd = _seed_workdir(os.path.join(args.fixture, "before"))
+    try:
+        res = run_fix(
+            before=before, workdir=wd, rule=args.rule,
+            applier=ReplayApplier(args.fixture),
+            reaudit=ReplayReaudit(os.path.join(args.fixture, "after.findings.json")),
+            line_tol=args.line_tol,
+        )
+
+        print(json.dumps(res.ledger(), indent=2))
+        if res.status == REJECTED:
+            print(f"\nREJECTED — fix introduced {len(res.introduced)} new finding(s):", file=sys.stderr)
+            for f in res.introduced:
+                print(f"  + {f.rule} {f.path}:{f.line}  {f.message}", file=sys.stderr)
+        elif res.status == OK and (args.show_diff or res.gate != "auto-commit"):
+            print(f"\n--- reviewable patch (gate: {res.gate}) ---\n{res.diff}")
+
+        return {OK: 0, NO_OP: 0, UNFIXABLE: 0, REJECTED: 2, NO_EFFECT: 3}.get(res.status, 0)
+    finally:
+        shutil.rmtree(wd, ignore_errors=True)

Also applies to: 45-62

🤖 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 `@fix/fixarm/cli.py` around lines 26 - 34, The seeded temporary workspace
created by _seed_workdir() is never cleaned up, so repeated runs leak temp
directories. Update main() to track the directory returned by _seed_workdir()
and remove it in a finally block after the fixture run completes, ensuring
cleanup happens even on errors. Use the existing _seed_workdir() and main() flow
to locate where the temp tree is created and where teardown should be added.

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.

@coderabbitai Fixed in 56ec0db. main() now seeds the workdir inside a try and shutil.rmtrees it in finally, so repeated runs don't leak temp trees.


Generated by Claude Code

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@PhysShell, thanks — that addresses the temp workspace leak cleanly.

Comment thread fix/fixarm/orchestrate.py Outdated
Comment thread fix/fixarm/orchestrate.py
Comment on lines +94 to +96
OK = "ok" # fix applied, no regression; gate decides commit vs review
REJECTED = "rejected" # fix introduced new findings -> reverted, never committed
NO_EFFECT = "no-effect" # applier ran but the targeted finding survived

@coderabbitai coderabbitai Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Non-OK apply paths leave the worktree mutated.

After Line 150, the REJECTED and NO_EFFECT branches only return a FixResult; nothing restores workdir. A regressing or ineffective fix can therefore leak into later steps if the caller reuses the same tree. Apply into a disposable copy, or add a rollback hook to Applier and invoke it on every post-apply non-success path, including re-audit failures. This contradicts the documented fix-arm safety flow.

Also applies to: 149-162

🤖 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 `@fix/fixarm/orchestrate.py` around lines 94 - 96, The non-OK apply paths in
the fix-arm flow leave the working tree mutated, so make the apply operation
disposable or add rollback to the Applier path. Update the orchestration around
the apply/audit logic in orchestrate.py so that the REJECTED and NO_EFFECT
branches, plus any re-audit failure after apply, restore workdir before
returning a FixResult. Use the existing status symbols OK, REJECTED, and
NO_EFFECT to ensure every non-success exit from the apply flow triggers cleanup.

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.

@coderabbitai Fixed in 56ec0db. The REJECTED and NO_EFFECT branches (and a throwing re-audit) now revert the tree via a new Applier.revert hook before returning; FixResult.reverted surfaces it in the ledger. This restores the documented §4 safety flow. Real adapters revert with git checkout+clean; the Replay adapter restores before/.


Generated by Claude Code

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@PhysShell, thanks — that addresses my concern. Adding Applier.revert for the non-success and re-audit-failure paths brings the flow back in line with the documented safety contract, and surfacing reverted in the ledger is a nice touch for observability.

🐇✅

Comment thread fix/README.md Outdated
Comment on lines +7 to +9
```
select(rule) → dry-run → diff → apply → re-audit → assert NO new findings → tier gate
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add a fence language.

This is shell syntax, and the bare fence will trigger MD040 in markdownlint.

Suggested fix
-```
+```bash
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
```
select(rule) → dry-run → diff → apply → re-audit → assert NO new findings → tier gate
```
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 7-7: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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 `@fix/README.md` around lines 7 - 9, The README snippet is using a bare fenced
code block, which triggers markdownlint MD040 because the content is shell
syntax. Update the fenced block around the pipeline in README.md to use a Bash
language tag, and keep the existing content under the same block so the shell
command is clearly identified. Use the fenced snippet near the select(rule) →
dry-run → diff → apply → re-audit → assert NO new findings → tier gate text to
locate it.

Source: Linters/SAST tools

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.

@coderabbitai Done in 56ec0db — the pipeline fence is now tagged text (it's a flow diagram, not shell), clearing MD040.


Generated by Claude Code

Comment thread fix/README.md
Comment on lines +13 to +16
```bash
python3 fix/tests/test_orchestrate.py # the contract tests
python3 -m fixarm.cli --fixture fix/fixtures/idisp001-clean --rule IDISP001 # demo
```

@coderabbitai coderabbitai Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the demo command runnable from the repo root.

fixarm lives under fix/, so python3 -m fixarm.cli will not resolve from the root checkout unless you add fix/ to PYTHONPATH or cd fix first.

Suggested fix
- python3 fix/tests/test_orchestrate.py                              # the contract tests
- python3 -m fixarm.cli --fixture fix/fixtures/idisp001-clean --rule IDISP001    # demo
+ PYTHONPATH=fix python3 fix/tests/test_orchestrate.py               # the contract tests
+ PYTHONPATH=fix python3 -m fixarm.cli --fixture fix/fixtures/idisp001-clean --rule IDISP001    # demo
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
```bash
python3 fix/tests/test_orchestrate.py # the contract tests
python3 -m fixarm.cli --fixture fix/fixtures/idisp001-clean --rule IDISP001 # demo
```
🤖 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 `@fix/README.md` around lines 13 - 16, The README demo command is not runnable
from the repo root because the module path for fixarm is unresolved; update the
documented command in the README snippet to use a root-compatible invocation, or
explicitly include the package path setup needed to find fixarm from the
checkout root. Refer to the demo command example in the README and make sure the
instructions clearly work without requiring the user to manually cd into fix/
first.

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.

@coderabbitai Done in 56ec0db — both commands are now prefixed with PYTHONPATH=fix so they run from the repo root without a cd fix.


Generated by Claude Code

Comment thread fix/tests/test_orchestrate.py Outdated
- rollback on every non-success path: REJECTED/NO_EFFECT and a throwing
  re-audit now revert the worktree via a new Applier.revert hook (Replay
  restores before/; real adapters git checkout+clean). FixResult.reverted
  surfaces it in the ledger. (Codex + CodeRabbit: tree left mutated)
- line_tol is now true within-distance matching per (rule, basename) group,
  not line//(tol+1) bucketing — no more false regressions at bucket edges.
  (Codex + CodeRabbit)
- ReplayApplier.apply models file deletions (before-only files removed), so
  the replayed tree matches after.findings.json. (CodeRabbit)
- cli.py and tests reap their temp workdirs (finally / context manager);
  removed the wrong 'OS reaps tmp' comment. (CodeRabbit)
- README: fence language for MD040 + PYTHONPATH=fix so commands run from root.
  (CodeRabbit)
- tests: +within-tolerance, +reject-reverts-tree; 7/7 on bare python3.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
@PhysShell
PhysShell merged commit b0f754b into main Jun 25, 2026
PhysShell pushed a commit that referenced this pull request Jun 26, 2026
Killer feature #1: where phase-2 diffs *findings* (a violation only shows once it
crosses a rule threshold), drift diffs the *structure* between two runs (baseline =
main's graph, PR = current) and reports what moved — before it trips a rule.

- arch/drift.py: snapshot(g) -> compact, committable {component metrics + namespace
  dependency surface (incl. external framework edges) + cycle set}. diff(base, cur)
  -> risk-tagged items: new/resolved cycles (type/ns/asm), new/removed dependencies,
  Ce coupling jumps, instability shifts. gate(d, level) for a PR ratchet. A new
  dependency into a sensitive_targets layer (SQL/WPF) is High.
- arch/drift_cli.py: --save-snapshot, then --baseline (snapshot OR raw graph.json) +
  --graph -> drift.json + a PR-friendly drift.md grouped by 🔴/🟠/🔵. --gate-level
  fails (exit 2) on drift at/above a risk; report-only otherwise.
- arch/rules.json: `drift` config block (thresholds + sensitive_targets).
- docs: phase-4 section marked engine-ready with a worked example.
- tests: +10 (identical=empty, new cycle=high, new SQL dep=high, plain dep=medium,
  coupling jump, resolved cycle=info, gate high-only, raw-graph baseline, CLI
  snapshot->gate) -> 41/41, -O safe.

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