Skip to content

fix: unique ULID filename for staged records (silent-overwrite bug) - #114

Merged
ooiyeefei merged 1 commit into
mainfrom
fix/stage-filename-collision
May 30, 2026
Merged

ooiyeefei merged 1 commit into
mainfrom
fix/stage-filename-collision

Conversation

@ooiyeefei

@ooiyeefei ooiyeefei commented May 30, 2026 •

Copy link
Copy Markdown
Owner

The bug (found in real demo prep)

stage_pending_decisions named files pending_<call-timestamp>_<index>.md, where the index resets to 000 every call. The timestamp alone isn't unique, so two stage calls sharing a timestamp silently overwrote each other:

  • two calls in the same instant, or
  • re-staging a batch that already carried a fixed timestamp (e.g. copying previously-extracted records).

No error — records just disappeared. Concretely: copying 12 records that shared one extraction timestamp left only ~9 on disk.

Fix

Filename is now pending_<timestamp>_<ULID>.md:

  • ULID = globally unique per record, across calls and instants → no collision ever.
  • timestamp prefix kept → files stay chronologically sortable.
  • ULID was already a dependency (util/ulid.py).

Example: pending_2026-05-30T12-15-13.182084p00-00_01K9XT9TXEH2GVJW73WT8XR4FW.md

TDD

Regression test pins datetime.now so both stage calls get the identical timestamp — the exact collision trigger — then asserts all 4 records from two batches survive on disk.

  • RED (current code): assert 2 == 4 — batch B clobbered batch A.
  • GREEN (fixed): 4 distinct files.

(The first version of this test passed without the fix — because sequential datetime.now() calls get different microseconds — so I rewrote it to pin the timestamp and actually reproduce the bug. Anti-pattern avoided.)

Verified: full suite 379 passed, ruff clean, pyright 0 errors.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Prevented overwriting of staged pending decisions when operations share the same timestamp by using collision-resistant filenames, ensuring all staged items are preserved.
  • Tests

    • Added a regression test verifying that multiple staging operations with identical timestamps produce unique output files and do not clobber prior results.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 30, 2026 •

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Free

Run ID: 72cdb9bc-2ec3-4adf-8c2d-a3579e0e21f4

📥 Commits

Reviewing files that changed from the base of the PR and between a5bcec9 and 0dd16cb.

📒 Files selected for processing (2)
  • spanweave/learning/extractor.py
  • tests/test_extractor.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/test_extractor.py

📝 Walkthrough

Walkthrough

The pending decision extraction now uses ULID-based unique filenames to prevent overwrites when stage_pending_decisions is called multiple times with the same timestamp. The sequential batch index has been replaced with a ULID suffix. A regression test validates that two batches staged within the same timestamp produce distinct, non-colliding filenames.

Changes

Pending Decision Filename Uniqueness

Layer / File(s) Summary
ULID-based filename generation
spanweave/learning/extractor.py
ULID import added at module level. stage_pending_decisions replaces timestamp+index naming (pending_<timestamp>_<i:03d>.md) with timestamp+ULID (pending_<safe_timestamp>_<ULID()>.md) to guarantee filename uniqueness across successive calls.
Same-timestamp collision regression test
tests/test_extractor.py
New test patches datetime.now to a fixed timestamp, stages two decision batches, and asserts output paths are disjoint, all four files exist, and filenames are globally unique across both batches.

🎯 2 (Simple) | ⏱️ ~12 minutes

🐰 A ULID hops where indices stumble fast,
No collision shall come to pass,
Timestamps dance with suffixes bright,
Files remain in conflict-free sight.
Two batches staged, yet never the same! ✨


Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

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

… timestamp collision)

stage_pending_decisions named files pending_<call-timestamp>_<index>.md, where
the index reset to 000 every call. Two stage calls that shared a timestamp —
two calls in the same instant, or re-staging a batch that already carried a
fixed timestamp — reused 000/001/... and silently overwrote each other's files.
No error; records just vanished. Found in demo prep: copying 12 records that
shared an extraction timestamp left only ~9 on disk.

Fix: filename is now pending_<timestamp>_<ULID>.md — the ULID is globally
unique per record (across calls and instants) while the timestamp prefix keeps
files chronologically sortable. ULID is already a dependency (util/ulid.py).

Regression test pins datetime.now so both stage calls share one timestamp (the
exact trigger) and asserts all 4 records from two batches survive. Verified RED
(assert 2 == 4) before the fix, GREEN after. Full suite 379 passed, ruff +
pyright clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ooiyeefei
ooiyeefei force-pushed the fix/stage-filename-collision branch from a5bcec9 to 0dd16cb Compare May 30, 2026 05:48
@ooiyeefei
ooiyeefei merged commit 78b4610 into main May 30, 2026
7 checks passed
@ooiyeefei
ooiyeefei deleted the fix/stage-filename-collision branch May 30, 2026 05:49
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.

1 participant