Skip to content

fix: preserve a distinct backup path for *.original.md inputs#152

Merged
rohitg00 merged 1 commit intorohitg00:mainfrom
shaun0927:fix/compress-file-original-backup-collision
Apr 16, 2026
Merged

fix: preserve a distinct backup path for *.original.md inputs#152
rohitg00 merged 1 commit intorohitg00:mainfrom
shaun0927:fix/compress-file-original-backup-collision

Conversation

@shaun0927
Copy link
Copy Markdown
Contributor

@shaun0927 shaun0927 commented Apr 16, 2026

Problem

PR #150 added automatic backup creation for memory_compress_file, but there is one filename shape where that contract breaks.

For inputs already ending in *.original.md, resolveBackupPath() returns the same path as the input file, so the function writes the backup and then immediately overwrites that same path with the compressed content.

Fix

Keep the existing behavior for ordinary Markdown files:

  • notes.md -> notes.original.md

But make already-suffixed inputs produce a distinct backup path:

  • notes.original.md -> notes.original.backup.md

This keeps the diff narrow and preserves the existing naming convention for the common case while making the advertised backup behavior true for *.original.md inputs.

Verification

  • npm test -- test/compress-file.test.ts
  • npm run build

Regression coverage added

New test case:

  • notes.original.md writes its backup to notes.original.backup.md
  • the backup contains the original Markdown
  • the input file contains the compressed Markdown

The new file-compression feature promises automatic backup creation,
but `*.original.md` inputs currently collapse the backup path onto the
source path and lose the original content. This keeps the common
`foo.md -> foo.original.md` behavior unchanged while assigning a
distinct backup name for already-suffixed inputs.

Constraint: Keep the fix narrow to backup-path generation and one regression test
Rejected: Introduce numbered backup discovery logic | larger behavior change than needed for the verified bug
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Preserve backup-path distinctness for any future suffix handling changes
Tested: npm test -- test/compress-file.test.ts; npm run build
Not-tested: Full npm test still hits an unrelated timeout in test/auto-compress.test.ts on this machine
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fd7cb9d8-078a-4976-b8eb-440126a8d82a

📥 Commits

Reviewing files that changed from the base of the PR and between 1bdeaad and 471ccbc.

📒 Files selected for processing (2)
  • src/functions/compress-file.ts
  • test/compress-file.test.ts

📝 Walkthrough

Walkthrough

The resolveBackupPath() function in compress-file is updated to generate backup filenames conditionally: if the input filename ends with .original, it appends .backup instead of .original, preventing overwrites. A test case validates this behavior.

Changes

Cohort / File(s) Summary
Backup Path Logic
src/functions/compress-file.ts
Updated resolveBackupPath() to append .backup when basename ends with .original; otherwise append .original. Prevents duplicate .original suffix collisions.
Backup Path Test
test/compress-file.test.ts
Added test case for files already ending in .original.md, verifying backup writes to .original.backup.md and original input receives compressed content.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 When .original comes to call,
We append .backup instead of all,
No overwrites shall befall,
The test hops through with ease and thrall,
A fix both simple and quite small!

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly summarizes the main change: modifying backup path generation for files already ending with .original.md to produce distinct backups instead of collisions.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@rohitg00 rohitg00 merged commit 9eabbdc into rohitg00:main Apr 16, 2026
1 check 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