fix(own): post-merge nits — ledger completeness, CLI guard, test robustness by PhysShell · Pull Request #3 · PhysShell/OwnAudit

PhysShell · 2026-06-25T15:39:31Z

Post-merge follow-up nits for the T4 OWN fixer (PR #2)

Three minor CodeRabbit findings landed as PR #2 was being merged, so they're collected here. All 🟡 Minor / quick wins; no behaviour change to the happy path.

own_fix.py — keep the ledger complete. The duplicate-site dedupe continued without recording anything, so a repeated finding on the same += line appeared in neither applied nor skipped — a silent drop that contradicts the function's own no-silent-drops contract (docs/fix-arm.md §8). It's now recorded as skipped with reason duplicate-site.
cli.py — fail fast on a missing after.findings.json. The CLI re-audits via the recorded after.findings.json for both appliers, but only the after/ tree was validated. A suggest-only fixture like own001-lambda (no after.findings.json) would crash later inside ReplayReaudit; now it returns 2 with a clear error.
test_own_fix.py — robust assertions. Replaced assert False (stripped under python -O, Ruff B011) with raise AssertionError, and added a ledger assertion that the duplicate finding lands in skipped.

Test plan

PYTHONPATH=fix python3 fix/tests/test_own_fix.py # 10/10
PYTHONPATH=fix python3 -O fix/tests/test_own_fix.py # 10/10 under -O (B011 guard holds)
PYTHONPATH=fix python3 fix/tests/test_orchestrate.py # 7/7

🤖 Generated with Claude Code

Generated by Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved CLI fail-fast behavior when re-audit is required but after.findings.json is missing: prints a clear error and exits with status code 2 (no stacktrace/crash later).
- No-op OWN runs now complete successfully even when after.findings.json is absent.
- Duplicate OWN findings are now recorded as skipped with reason duplicate-site, keeping the applied/skipped ledger complete.
Tests
- Added/strengthened assertions and new CLI regression tests for missing after.findings.json, plus coverage for duplicate handling and safer failure checks.

- own_fix: duplicate-site dedupe now records the dropped finding as 'duplicate-site' in the skipped ledger (preserves the no-silent-drops contract) instead of silently continuing. - cli: validate after.findings.json exists (the CLI re-audits via it for both appliers); fail fast with a clear error instead of crashing later in ReplayReaudit on suggest-only fixtures like own001-lambda. - tests: replace 'assert False' with 'raise AssertionError' (B011, survives -O); assert the duplicate finding lands in the skipped ledger. 10/10 + 7/7, incl -O. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa

coderabbitai · 2026-06-25T15:39:52Z

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: 69523042-da03-4d99-abf0-05e868af435a

📥 Commits

Reviewing files that changed from the base of the PR and between 8063e62 and aaefa85.

📒 Files selected for processing (1)

fix/tests/test_own_fix.py

🚧 Files skipped from review as they are similar to previous changes (1)

fix/tests/test_own_fix.py

📝 Walkthrough

Walkthrough

The CLI now checks for after.findings.json before reauditing a fixture, and plan_file records duplicate planned detachments as skipped entries. Tests cover the duplicate skip ledger, the explicit rejection branch, and both CLI outcomes when the fixture file is missing.

Changes

Fixarm validation and planning

Layer / File(s)	Summary
Duplicate skip ledger `fix/fixarm/own_fix.py`, `fix/tests/test_own_fix.py`	`plan_file` appends duplicate findings to the skipped ledger with reason `duplicate-site` before continuing, and the related tests assert that skipped entry along with the explicit rejection branch.
Fixture findings preflight `fix/fixarm/cli.py`, `fix/tests/test_own_fix.py`	The CLI wraps the fix run in a missing-file check for `after.findings.json`, prints a stderr error, returns exit code `2`, and the CLI regression tests cover both the no-op and reaudit-required paths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

PhysShell/OwnAudit#2: Introduced the OWN fixer and replay re-auditing flow that these CLI and planning changes extend.
The current changes build on the same fixer/replay path by adding missing-file handling and duplicate-skip tracking in that workflow.

Poem

🐰 I hopped by logs with careful feet,
Duplicate hops now leave a seat.
If findings hide, the CLI sees,
And tests all twitch in gentle breeze.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.18% 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 accurately summarizes the main follow-up changes: ledger completeness, a CLI guard, and test robustness for the OWN fixer.
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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:7bf9755e22

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

chatgpt-codex-connector · 2026-06-25T15:42:13Z

+ # The CLI re-audits via the recorded after.findings.json (both appliers), so it must
+ # exist. Suggest-only fixtures (e.g. own001-lambda) have none — they're unit-tested,
+ # not run through the CLI; fail fast rather than crash later in ReplayReaudit.
+ if not os.path.isfile(os.path.join(args.fixture, "after.findings.json")):


Defer after.findings check until re-audit is needed
When using a fixture that intentionally has only before.findings.json and --applier own, this guard now returns 2 before run_fix() can handle cases that never call ReplayReaudit, such as an absent rule (no-op, documented as exit 0 in this CLI) or a detect-only T3 rule (unfixable). For example, main(['--fixture', 'fix/fixtures/own001-lambda', '--rule', 'RCS9999', '--applier', 'own']) now fails solely because after.findings.json is missing, even though the wrapper would return a no-op ledger without reading it.
Useful? React with 👍 / 👎.

@codex Fixed in 8063e62. Removed the eager after.findings.json guard and deferred the check to when re-audit is actually reached: ReplayReaudit reads the file lazily, so no-op (absent rule) and unfixable (T3) rules — which return before re-audit — never touch it and still exit 0. A fixable rule on a fixture without after.findings.json now surfaces a clean exit-2 error (caught FileNotFoundError) instead of a stacktrace. Your exact example --fixture own001-lambda --rule RCS9999 --applier own now exits 0. Tests: test_cli_no_op_does_not_need_after_findings, test_cli_missing_after_findings_fails_cleanly_when_reaudit_needed.
Generated by Claude Code

To use Codex here, create an environment for this repo.

…needed Codex P2 on PR #3: the upfront after.findings.json guard was too eager — it returned 2 even for rules that short-circuit before re-audit (absent rule -> no-op exit 0; detect-only T3 -> unfixable). Removed the eager guard; ReplayReaudit reads the file lazily, so no-op/unfixable never touch it, and a fixable rule on a fixture without after.findings.json now surfaces a clean exit-2 error (caught FileNotFoundError) instead of a stacktrace. +2 CLI tests; 12/12 + 7/7, incl -O. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa

coderabbitai

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 `@fix/tests/test_own_fix.py`:
- Around line 224-240: The new CLI tests rely on bare assert equality checks, so
they become no-ops under optimized execution. Update
test_cli_no_op_does_not_need_after_findings and
test_cli_missing_after_findings_fails_cleanly_when_reaudit_needed to validate
the return code with an explicit conditional that raises AssertionError when
main(...) returns an unexpected value, and align the existing nearby rc checks
in the same test module that still use assert rc == ... so all CLI exit-code
tests remain -O-safe.

🪄 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: 0cec52d7-1340-4d72-9dd7-82ff7b02b202

📥 Commits

Reviewing files that changed from the base of the PR and between 7bf9755 and 8063e62.

📒 Files selected for processing (2)

fix/fixarm/cli.py
fix/tests/test_own_fix.py

🚧 Files skipped from review as they are similar to previous changes (1)

fix/fixarm/cli.py

CodeRabbit: the new CLI tests asserted rc via bare 'assert rc == N', which is stripped under python -O, so they were vacuous in optimized runs (the very mode the PR claims to support). Added an _expect(actual, expected) helper that raises AssertionError, and routed all four CLI exit-code checks through it. Verified _expect raises under -O. 12/12 normal and -O. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa

chatgpt-codex-connectorBot reviewed Jun 25, 2026
View reviewed changes

coderabbitaiBot reviewed Jun 25, 2026
View reviewed changes

Comment threadfix/tests/test_own_fix.py

PhysShell merged commit 8c3f7b5 into mainJun 25, 2026
1 check passed

Navigation Menu

fix(own): post-merge nits — ledger completeness, CLI guard, test robustness - #3

fix(own): post-merge nits — ledger completeness, CLI guard, test robustness#3
PhysShell merged 3 commits into
mainfrom
claude/sts-runtime-analysis-2mo4z9

PhysShell commented Jun 25, 2026•
edited by coderabbitai Bot
Loading

Uh oh!

coderabbitaiBot commented Jun 25, 2026•
edited
Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

chatgpt-codex-connectorBot left a comment

Uh oh!

chatgpt-codex-connectorBotJun 25, 2026

Uh oh!

PhysShellJun 25, 2026

Uh oh!

chatgpt-codex-connectorBotJun 25, 2026

Uh oh!

coderabbitaiBot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

PhysShell commented Jun 25, 2026• edited by coderabbitai BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Post-merge follow-up nits for the T4 OWN fixer (PR #2)

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitaiBot commented Jun 25, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

chatgpt-codex-connectorBot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connectorBotJun 25, 2026

Choose a reason for hiding this comment

Uh oh!

PhysShellJun 25, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connectorBotJun 25, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitaiBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

PhysShell commented Jun 25, 2026•
edited by coderabbitai Bot
Loading

coderabbitaiBot commented Jun 25, 2026•
edited
Loading