perf(fixer): skip defensive deep copy when caller owns document

[erraggy]

Summary

• Fixer.Fix() now sets MutableInput=true with defer-based save/restore before calling FixParsed(), since it always owns the parsed document it creates internally
• CLI stdin path sets f.MutableInput=true — the parse result is locally scoped and discarded after fixing
• CLI source-map path passes WithMutableInput(true) — same ownership reasoning
• Bonus: isFixEnabled modernized to use slices.Contains (gopls hint)

Why

The fixer defensively deep-copies the entire parsed document before mutating it, protecting callers who reuse the input afterward. But in all three CLI code paths (file, stdin, source-map), the parsed document is never reused — it's consumed by the fixer and discarded. Skipping the copy eliminates an unnecessary O(n) allocation on every fix invocation.

Test plan

• All TestMutableInput* tests pass (6/6)
• Fixer corpus tests pass (exercises Fix() path)
• CLI fix command tests pass
• go_diagnostics clean on both modified files
• PR review: 3 agents confirmed correctness; defer pattern adopted per silent-failure-hunter recommendation

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added a configurable option to control input mutability for fix operations, covering stdin and source-map file flows.

• Bug Fixes

  • Ensures mutable-input mode is applied during fixes and reliably restored after the operation.

• Tests

  • Added tests validating mutable-input is toggled during the operation, preserved on success, and unchanged on parse errors.

• Documentation

  • Guidance updated to prefer generated DeepCopy() methods and note when skipping copies is safe.

[coderabbitai]

Warning

Rate limit exceeded

@erraggy has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 17 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

Adds a MutableInput mode to the fixer: new MutableInput field and public WithMutableInput(bool) option; Fix() temporarily forces MutableInput = true during execution and restores it; stdin and source-map flows enable mutable input; tests and docs updated to prefer DeepCopy() or WithMutableInput(true).

Changes

Cohort / File(s)	Summary
Command Handler cmd/oastools/commands/fix.go	Enables mutable input for stdin and source-map paths by setting MutableInput = true and passing WithMutableInput(true) when constructing the fixer.
Fixer Core fixer/fixer.go, fixer/...	Adds/uses MutableInput bool on Fixer, introduces public option WithMutableInput(bool), and makes Fix() temporarily force MutableInput = true (restores original afterwards). Replaces manual fix-type checks with slices.Contains.
Tests fixer/fixer_test.go	Adds TestFix_SetsInternalMutableInput validating that Fix() forces and restores/preserves MutableInput across success and parse-error scenarios.
Documentation & Guidance .github/copilot-instructions.md, .claude/docs/oas-concepts.md, AGENTS.md, CONTRIBUTORS.md, .claude/agents/developer.md	Replaces JSON marshal/unmarshal deep-copy guidance with generated DeepCopy() usage; documents WithMutableInput(true) as an alternative when caller owns the document. Minor clarifications to parse/version docs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 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 performance optimization: enabling MutableInput to skip defensive deep copying in the fixer when the caller owns the document.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch perf/skip-defensive-copy-in-fixer

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.62%. Comparing base (a6ba2b4) to head (b02fc5f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #297      +/-   ##
==========================================
+ Coverage   84.59%   84.62%   +0.02%     
==========================================
  Files         158      158              
  Lines       25125    25125              
==========================================
+ Hits        21255    21261       +6     
+ Misses       2625     2618       -7     
- Partials     1245     1246       +1     

Files with missing lines	Coverage Δ
fixer/fixer.go	91.32% <100.00%> (+3.46%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@fixer/fixer_test.go`:
- Around line 604-615: Add a subtest to TestFix_SetsInternalMutableInput that
sets f.MutableInput = true before calling f.Fix(...) and then asserts the
original value is restored after Fix returns; specifically, create a
t.Run("starts true") block that creates f := New(), sets f.MutableInput = true,
calls result, err := f.Fix("../testdata/bench/small-oas3.yaml"),
require.NoError(t, err), assert.True(t, result.Success), and finally
assert.True(t, f.MutableInput, "MutableInput should be restored to true after
Fix()") to ensure the save/restore (defer) logic in Fix handles both starting
true and false cases.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@fixer/fixer_test.go`:
- Around line 604-627: Add a subtest in TestFix_SetsInternalMutableInput that
verifies MutableInput is restored when Fix() returns an error: create f :=
New(), set f.MutableInput to true (and another case false if desired), call
f.Fix() with an input that provably causes an error (e.g., invalid path or
malformed test fixture) and require an error, then assert f.MutableInput equals
the original value; ensure the test exercises the same defer/restore path used
by Fix() so the panic/error-safe restore is validated.
- Around line 604-627: The test relies on Fix() toggling MutableInput to skip
defensive copies, which violates the "always deep copy" rule; change Fix() so it
never mutates or relies on the MutableInput flag internally—instead always
perform a defensive deep copy of the input document (use JSON marshal/unmarshal)
before any modifications and ensure MutableInput remains unchanged by Fix();
then update or keep TestFix_SetsInternalMutableInput to assert that
f.MutableInput is unchanged before and after Fix() and that Fix() still succeeds
(refer to the Fix(), MutableInput field, New(), and
TestFix_SetsInternalMutableInput symbols to locate the code).