fix: make AdaptiveStore.save() atomic via tempfile + os.replace (#138)

[devkamani9313-lab]

Summary

Makes AdaptiveStore.save() atomic by writing to a temporary file (tempfile.mkstemp) and then renaming it over the target with os.replace().

Fixes #138

Problem

The current save() method uses Path.write_text(), which is a non-atomic write:

• A crash mid-write leaves adaptive.json truncated/corrupt
• Two concurrent scrapers can interleave and clobber each other
• _load() silently returns {} on corrupt JSON, wiping all saved fingerprints

Changes

src/pyscrappy/generic/adaptive_store.py

• Added import tempfile (stdlib, no new dependencies)
• Replaced self.path.write_text(json.dumps(...)) with:
  1. tempfile.mkstemp(dir=self.path.parent) — temp file in same directory
  2. json.dump(data, f) — write to temp file
  3. os.replace(tmp, self.path) — atomic rename (POSIX + Windows)
  4. except BaseException: os.unlink(tmp); raise — cleanup on failure

tests/test_generic/test_adaptive.py

• Added test_save_atomic_no_corruption_on_write_failure:
  • Saves initial data, then simulates a failed write via mock.patch("os.replace")
  • Asserts original data survives intact
  • Asserts failed key does not appear
  • Asserts no leftover .tmp files

Checklist

• ruff check src/ passes
• All lines ≤ 100 characters
• No new dependencies (uses stdlib tempfile + os)
• Existing tests unmodified and expected to pass
• New test covers the failure scenario

[Copilot]

Pull request overview

This PR improves the resilience of the adaptive-selector fingerprint persistence by making AdaptiveStore.save() write updates atomically (write to a temp file, then swap into place) and adds a regression test to ensure failed writes don’t corrupt existing stored data.

Changes:

• Updated AdaptiveStore.save() to write JSON to a temp file and atomically replace the target via os.replace.
• Added a test that simulates a failure during the replace step and verifies the original store remains intact and no temp files are left behind.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/pyscrappy/generic/adaptive_store.py	Switches store persistence to a temp-write + atomic replace approach to avoid partial/corrupt writes.
tests/test_generic/test_adaptive.py	Adds a failure-mode test to validate atomic-save behavior and cleanup.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[devkamani9313-lab]

Hi @mldsveda 👋

I've addressed the automated review feedback in the latest commit:

1. Safe Resource Cleanup & Non-Masking Exceptions: Wrapped fd closure and tmp unlinking in safe, non-masking try...except OSError blocks so primary exceptions are never overridden and file descriptors don't leak.
2. Module-Scoped Mocking: Scoped the os.replace patch specifically to pyscrappy.generic.adaptive_store.os.replace to prevent side-effects in concurrent test environments.
3. Lint Compliance: Checked and verified with ruff (all lines ≤ 100 chars).

Ready for review/merge when you have a moment! 🚀

[devkamani9313-lab]

Hi @mldsveda 👋

I've updated the PR with all feedback addressed:

1. Atomic Write: AdaptiveStore.save() now writes to a temp file and renames via os.replace().
2. Safe Resource Cleanup: Added non-masking try...except OSError blocks for fd and tmp file cleanup on errors.
3. Module-Scoped Mocking: Scoped os.replace patch specifically to pyscrappy.generic.adaptive_store.os.replace.
4. Ruff Formatting: Applied ruff format to ensure 100% style compliance.

Ready for review & merge when you have a moment! 🚀

[vedaant00]

Thanks @devkamani9313-lab, the atomic save is done right, temp file in the same dir, handle closed before os.replace, and failure cleanup that unlinks the temp file without masking the original exception. The failure-mode test is a good guard too. One thing: CI lint stayed red because ruff format still flagged adaptive_store.py and test_adaptive.py on your latest, likely a ruff version mismatch with CI, so your format commits did not fully clear it. I pushed a ruff format commit on top to green it, no logic change. 480 passed. Nice contribution.

[devkamani9313-lab]

Thank you so much @vedaant00 and @mldsveda for the super fast review and merge! Glad to contribute to PyScrappy! 🎉🙌