The Bug That Silently Dropped Half Your Refactor

The symptom in one sentence: run two transforms against the same file, get a success message that says both transforms applied, then open the file and only one of them is there. The transform engine reports success. The journal records both edits. The diff is a lie.

This is the bug we found during v0.2.3 integration testing for the three new type-aware TypeScript transforms. The fix landed in PR #38. Here is the full anatomy.

How the engine is shaped

Refactron's refactoring engine produces a RefactorPlan containing a list of FileChange records. Each FileChange is a complete new-content blob for one file path — not a diff, not a patch, the whole replacement text. A Refactorer (one per transform) reads the source files it cares about, decides what to change, and emits its own FileChange records into the plan. The apply-orchestrator groups by path and writes the result atomically through the temp-file-then-rename path that every Refactron write goes through.

The shape is deliberate. Whole-file content blobs are easy to verify, easy to back up, easy to roll back, and atomic on the filesystem. Diff patches are none of those things. The choice was not the bug.

The bug

Each transform built its FileChange by re-reading the original file content from disk and applying its own edits against that original. If two transforms both touched src/foo.ts, the apply-orchestrator received two FileChange records for src/foo.ts. Both records were rebuilt from the original content, before either transform's edits. The orchestrator grouped by path. It wrote the last record per path. The last record was missing the first transform's edits.

The output looked correct at every layer except the file on disk. Each transform reported "applied 1 change" — true, from its own perspective. The journal recorded two FileChanges — true, there were two FileChange records. The verification gates passed — they ran on the final-written content of each file independently, and the final-written content was self-consistent (it just contained only one transform's work). The only place the bug surfaced was the actual content of the file on disk, which contained the work of exactly one of the two transforms — usually whichever one ran second, but order-dependence varied with transform registration order, which made the bug feel non-deterministic.

A user running Refactron with autofix saw a green checkmark and a clean exit. The session journal showed two successful transforms. The before/after diff Refactron printed showed both transforms' edits, because the diff was computed from the FileChange records, not from the written file. Only opening the file in an editor revealed that half the work was missing.

Why the tests did not catch it

Every existing transform test ran one transform in isolation against a tmp directory. The integration tests had broad coverage of one-transform-per-file scenarios. None of them ran two transforms against the same file.

With ten transforms in the catalog and only a fraction of files attracting multiple transforms in any given run, the bug was statistically rare in fixtures — but on real codebases, multi-transform-per-file is the common case rather than the rare one. A single TypeScript file might be touched by var_to_const_let, indexof_to_includes, and string_concat_to_template_literal in the same session. The bug was waiting for the third transform to land before it had any chance of being triggered by the test suite, and even then only if the integration tests happened to run two of them against the same fixture.

The fix

PR #38 changed the composition contract. Each touching transform now emits its FileChange carrying the cumulative content — the file content as it stands after all prior transforms in this run have applied to that file. The apply-orchestrator still groups by path and the last record per path is what gets written, but now the last record is the correct cumulative result instead of a stale rebuild from the original.

The public FileChange shape did not change. No consumer of the engine had to update anything. The change was entirely in how the engine produces FileChange records.

The implementation is "thread the current text." Instead of every Refactorer reading source files from disk, the engine maintains an in-memory Map from path to current text, initialized from disk and updated after each Refactorer runs. When the engine asks a Refactorer to consider a file, it passes the current text — the text reflecting every prior transform's edits in this session. Order-stable, no data loss, no shared mutable state visible to the Refactorer beyond the text it is being asked to refactor.

The integration test that locks it down

A new test in tests/integration/multi-transform-composition-ts.test.ts builds a TypeScript file engineered to hit all three of the v0.2.3 type-aware transforms: indexof_to_includes, object_assign_to_spread, and string_concat_to_template_literal. The test runs the full engine end to end, then reads the written file from disk and asserts that the final content contains the rewrite from every transform — not just one of them.

The test fails against the v0.2.0, v0.2.1, and v0.2.2 engines immediately. It passes against the post-PR-38 engine. It is exactly the test we should have written before shipping v0.2.0.

The lesson

Every composable system needs a test that exercises the composition, not just the units. Transforms that look pure in isolation can interact through shared state you forgot was shared. In our case the shared state was the file on disk — invisible to the unit tests because each unit test ran in its own tmp directory with its own input file, but very real once the engine ran multiple transforms in one session against one real tree.

The deeper lesson is about contract ambiguity. The public types — RefactorPlan, FileChange — did not change in PR #38. The bug was entirely in the interpretation of FileChange.content. Did "content" mean "the new content this transform proposes against the original" or "the new content this transform proposes against the current state"? The shape was fine. The contract was ambiguous. Tests against the units cannot catch a contract ambiguity, because the units are individually consistent with both interpretations. Only a test that runs the composition can pin the contract down.