Skip to content

fix: treat an empty index.db-wal as missing in the store fingerprint - #79

Open
shahmir-oscilar wants to merge 1 commit into
pnpm:mainfrom
shahmir-oscilar:empty-wal-fingerprint
Open

shahmir-oscilar wants to merge 1 commit into
pnpm:mainfrom
shahmir-oscilar:empty-wal-fingerprint

Conversation

@shahmir-oscilar

@shahmir-oscilar shahmir-oscilar commented Oct 14, 2026 •

Copy link
Copy Markdown

Closes #78.

In jobs without a container, the store has an empty index.db-wal when fingerprintRestoredStore runs. The project install opens and closes the database, SQLite deletes the empty log, and the post step's fingerprint no longer matches, so the store is saved again on every run.

fingerprintStore now treats a 0-byte index.db-wal as missing. A non-empty log still changes the fingerprint, and a checkpoint that copies pages into index.db still changes its size or mtime.

Validation:

  • New test: an empty write-ahead log has the same fingerprint as none. It fails without the change. All 104 tests and the TypeScript check pass.

  • dist/index.js rebuilt with pnpm run build; rebuilding main without the change reproduces the committed bundle byte for byte, so the bundle diff comes only from this change.

  • End to end in shahmir-oscilar/pnpm-setup-store-skip-repro. Its empty-wal-fingerprint branch runs every job on this commit; main runs v3.1.1. Same push of both:

    Job v3.1.1 (run) this branch (run)
    runner-check (no container) Cache saved with the key: … The store has not changed since it was restored, not saving cache.
    container-check The store has not changed … The store has not changed …
    runner-split-check (no container, install as its own step) Cache saved with the key: … The store has not changed …

Drafted by Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Empty write-ahead log files no longer affect store fingerprints; non-empty files continue to be included.

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4ae5f0b6-70b4-4c6d-ade8-25fca079137c
📥 Commits

Reviewing files that changed from the base of the PR and between a088197 and f285b2f.

⛔ Files ignored due to path filters: 1
  • dist/index.js is excluded by !**/dist/**
📒 Files selected for processing: 2
  • src/store-fingerprint/index.test.mjs
  • src/store-fingerprint/index.ts

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📜 Recent review details
🔇 Additional comments: 2
src/store-fingerprint/index.ts: 1

10-11: LGTM!

src/store-fingerprint/index.test.mjs: 1

33-40: LGTM!

📝 Walkthrough

fingerprintStore now treats a zero-byte index.db-wal as absent when it computes the store fingerprint. A test compares fingerprints with the empty WAL present and removed.

Changes: Store Fingerprinting

Layer / File(s) Summary
Empty WAL fingerprint behavior
src/store-fingerprint/index.ts, src/store-fingerprint/index.test.mjs
fingerprintStore clears the stat entry for a zero-byte WAL file. The test checks that its presence does not change the fingerprint.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Severity of issue fixed: Low

Suggested reviewers: ehaby

Merge Risk: ⚪ Minimal · up to f285b

The action bundle includes the change intended to prevent redundant cache saves when an empty WAL file appears; no actionable merge risk remains.

Pre-merge checks: Passed 4 · Failed 1

Failed 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed 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 clearly and concisely describes the primary change: treating an empty index.db-wal file as missing during store fingerprinting.
Linked Issues check Passed Issue #78 requires fingerprintStore to treat a 0-byte index.db-wal as missing. The source change implements this behavior. The added test compares the fingerprint with an empty WAL to the fingerpr…
Out of Scope Changes check Passed The reviewed changes are limited to the fingerprintStore implementation and its focused automated test. Both changes directly support issue #78. The reported dist/index.js rebuild is excluded from…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit checks the WAL at night,
Finds zero bytes, and sets things right.
The fingerprint stays the same,
When empty logs leave the frame.
The tests hop through and cheer.

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

@shahmir-oscilar

Copy link
Copy Markdown
Author

@zkochan Could you take a look when you get a chance? Thank you so much!

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.

Store cache is re-saved on every run in jobs without a container

1 participant