Stop the std_list cache drifting from the database

[boomzero]

The std_list KV cache kept losing problems that exist in std_answer. Three separate defects combined, which is why it looked intermittent and random.

1. The KV write was a floating promise

Source/Process.ts, upload path:

ThrowErrorIfFailed(await this.XMOJDatabase.Insert("std_answer", {...}));  // awaited
this.kv.put("std_list", currentStdList);                                  // NOT awaited
return new Result(true, "标程上传成功");

The D1 insert commits; the KV put has nothing keeping it alive. Source/index.ts accepts Context in fetch but never uses it — waitUntil appears nowhere outside the cron handler, and Process is constructed as new Process(RequestData, Environment), so it has no access to the execution context at all. Once the response returns, the pending KV write is not guaranteed to complete.

2. Read-modify-write over a store with no compare-and-set

Every upload did get → concat → put of the entire list. Two uploads landing close together both read the same base list, each appended only its own id, and the second put overwrote the first — that id gone permanently. KV reads are also eventually consistent, so even non-concurrent uploads from different locations could read a stale list and clobber newer entries.

3. The self-repair branch was dead code

The branch meant to backfill a missing entry, reduced to a runnable repro:

split -> ["1000","1001","1002",""]
typeof element: string | typeof ProblemID: number
branch taken for an ID that IS present (1001): false
branch taken for an ID that is MISSING (9999): false

split('\n') yields strings; CheckParams guarantees ProblemID is a number. d === Data["ProblemID"] is strict equality across types — always false, for every input. The condition was also inverted:

with the type mismatch removed:
  present (1001) -> appends again?  true
  missing (9999) -> appends?        false

It appended when the id was already there and did nothing when it was missing. Two bugs cancelling into a no-op — so fixing only the type comparison would have made it worse, appending a duplicate on every re-upload.

Net effect: 1 and 2 removed entries, 3 guaranteed they were never restored. Drift was one-directional and permanent.

The fix

Rebuild the cache wholesale from the database rather than patching entries into it.

• Reads unchanged. GetStdList is still a pure KV read costing zero database rows — that is why the cache exists.
• Insert path rebuilds from D1 and awaits the write. UploadStd early-returns when a std already exists, so a problem is inserted at most once ever; lifetime inserts are bounded by the problem count.
• Already-uploaded path stays cheap. This is the hot one, so it rebuilds only when the cached list is genuinely missing the problem. A test asserts zero DB reads and zero KV writes when the cache is already in sync.
• Daily reconciliation in the existing cron (crons = ["0 0 * * *"], which already has a proper waitUntil). One scan per day bounds any remaining drift to 24 hours instead of forever. This is the part that does not depend on every write path staying correct in the future.

Also fixes GetStdList returning a spurious trailing 0 from the trailing newline ([1000, 1001, 1002, 0]), and throwing outright if the key were ever unset.

Verification

9 tests added, written before the fix and confirmed failing against the old code (including the unset-key crash at the then-current Process.ts:1283). All pass now; full suite 56 pass, 0 fail. The changed regions are clean under tsc --noEmit; the one remaining index.ts error is pre-existing on master, verified by stashing.

Coverage includes drift healing, the empty table, the hot path doing no I/O, cache repair on re-upload, and both the legacy trailing-newline and unset-key read formats.

Note

The rebuild-on-write still has a narrow window — KV put ordering is not guaranteed, so two concurrent uploads could in principle land out of order. Each writer now writes complete state rather than a delta, so the loss is far smaller, and the daily rebuild bounds it. Closing it entirely would need the list moved into D1 or a Durable Object, which is a bigger change than this bug warrants.

🤖 Generated with Claude Code

---

Summary by cubic

Rebuilds the std_list KV cache from D1 to stop drift, awaits writes, and reconciles it daily. GetStdList now rebuilds on a missing key (not empty) and ignores trailing newlines; cron failures now propagate.

• Bug Fixes

  • Replace racy read→append→put with wholesale rebuilds; await KV writes.
  • Hot path only rebuilds when the cached list truly misses the problem.
  • GetStdList rebuilds the cache on a missing key and parses legacy trailing newlines.
  • Scheduled job hands waitUntil a real async promise and runs RebuildStdList, so failures are reported and drift heals within 24 hours.

• Refactors

  • Add StdListKey, ParseStdList, and RebuildStdList; GetStdList remains a pure KV read on hits, doing no DB/KV work when in sync.

^{Written for commit f8378f2. Summary will update on new commits.}

Summary by Sourcery

Rebuild the std_list KV cache from the database to prevent drift and ensure consistent std answer listings, with periodic reconciliation via the scheduled worker.

Bug Fixes:

• Ensure UploadStd rebuilds and awaits the std_list KV cache after inserts instead of performing racy append writes.
• Prevent UploadStd from silently ignoring already-uploaded problems that are missing from the cache by repairing the cache when it is out of sync.
• Handle unset std_list KV keys and legacy trailing-newline formats in GetStdList so it no longer crashes or returns a spurious trailing zero.
• Fix the scheduled handler so async errors propagate correctly instead of being swallowed by an improperly constructed Promise.

Enhancements:

• Introduce StdListKey, ParseStdList, and RebuildStdList helpers to centralize management of the std_list KV cache and tolerate legacy formats.
• Optimize the hot re-upload path to avoid unnecessary database reads and KV writes when the cache is already in sync.
• Add a daily reconciliation step in the scheduled job to rebuild the std_list cache from the database and bound any remaining drift.

Tests:

• Add comprehensive tests for std_list cache rebuilding, drift healing, and empty-table behavior.
• Add tests ensuring UploadStd correctly rebuilds/repairs the cache, avoids redundant I/O on the hot path, and handles unset cache keys.
• Add tests validating GetStdList parsing of cached data, distinguishing between empty and missing keys, and avoiding extra database access when the cache is valid.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[cubic-dev-ai]

2 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Source/index.ts">

<violation number="1" location="Source/index.ts:179">
P2: If the daily cache rebuild fails, the promise registered with `waitUntil` can remain pending forever because `RebuildStdList` runs inside an async Promise executor with no rejection path. Using an async IIFE directly with `waitUntil`, or explicitly propagating errors to the outer promise, would let the scheduled invocation settle and report the failure correctly.</violation>
</file>

<file name="Source/Process.ts">

<violation number="1" location="Source/Process.ts:56">
P2: When the `std_list` key is unset, `GetStdList` silently reports success with an empty list instead of surfacing the invalid cache state. That can make clients believe no standard answers exist during the drift window. Missing (`null`/`undefined`) should be distinguished from a valid empty string and raised as an error, while the empty-table cache remains valid.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[sourcery-ai]

🧙 Sourcery is reviewing your pull request!

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.