Skip to content

Drop a grounding read that a newer one replaced - #40

Open
ColtenOuO wants to merge 1 commit into
sysprog21:mainfrom
ColtenOuO:fix-grounding-stale-read
Open

Drop a grounding read that a newer one replaced#40
ColtenOuO wants to merge 1 commit into
sysprog21:mainfrom
ColtenOuO:fix-grounding-stale-read

Conversation

@ColtenOuO

@ColtenOuO ColtenOuO commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #39.

Sumarry

loadGroundingFile wrote its result whenever the file read finished, even if a newer action had happened in the meantime:

  • Second file on the same input: pick a slow JD, then a second JD before the first finishes. The slow read lands last and replaces the second file's snippets, so the input shows one file and the list another. If the slow read fails, it clears the good result instead.
  • Clear during a read: press Clear document grounding while a resume is still reading. When the read lands, the cleared snippets and the "Parsed locally" status come back.

Fix and Change

Each document kind keeps a read counter (groundingReads.jd / .resume). A read takes the next number before its await and gives up afterwards if the number is no longer current, so it touches neither grounding, the status line, nor the lists. Clear bumps both counters. The counters are per kind so a JD read never cancels a resume read.

New test

Two lobbyTests in tests/browser/lobby.test.js. They hold the read of a file named slow.txt open by wrapping Blob.prototype.arrayBuffer, and release it on purpose.


Summary by cubic

Fixes a race condition where a grounding file read that finished later could replace the result of a newer read, leaving the file list and snippets out of sync. Reads now use per-kind counters, and a read that is no longer the latest is dropped without touching the lists or status line.

Bug Fixes

  • Picking a second JD before the first finishes reading no longer lets the slow read overwrite the second file's snippets.
  • Clearing grounding while a resume is still reading no longer brings back the cleared snippets and "Parsed locally" status.
  • Adds two lobby tests that hold a slow read open to cover both stale-read cases.

Written for commit 0f79ad5. Summary will update on new commits.

Review in cubic

cubic-dev-ai[bot]

This comment was marked as resolved.

Comment thread web/app.js
Comment thread tests/browser/lobby.test.js Outdated
@ColtenOuO

Copy link
Copy Markdown
Contributor Author

The check failure here is not from this change. The only failing gate is cargo-audit, which flags rustls 0.23.43 under RUSTSEC-2026-0285 (TLS 1.3 handshake messages accepted across encryption level boundaries), published 2026-09-14. The advisory landed between runs: this branch passed in run 34843141378 and failed in run 34874694806 the same day.

The fix is rustls >= 0.23.45, in #42 . I will rebase this branch once that merges.

@jserv

jserv commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

The fix is rustls >= 0.23.45, in #42 . I will rebase this branch once that merges.

Go ahead.

A file read that was still in flight committed its result whenever it
landed. Picking a second JD before the first finished reading could
leave the first one's snippets under the second file's name, and a
resume read when Clear was pressed brought the cleared snippets back.
Each document kind now counts its reads and Clear bumps both, so a
read that is no longer the latest one is dropped without touching the
lists or the status line.
@ColtenOuO
ColtenOuO force-pushed the fix-grounding-stale-read branch from 0cd80d6 to 0f79ad5 Compare September 14, 2026 20:11
@ColtenOuO

Copy link
Copy Markdown
Contributor Author

rebase to the main, CI looks green!

Comment thread web/app.js
message = error.message;
}
if (read !== groundingReads[kind]) return;
if (kind === "jd") grounding.requirements = parsed?.requirements ?? [];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This guards grounding against a stale read, but the start handler still reads it across an await: selected from checkedGrounding() and consented are captured before await recordGitHubLogin(false), while selectedGroundingPacket(grounding, selected, consented) runs after it. Nothing disables the file inputs or the consent box during that round trip, so a JD picked while login is pending lands here and leaves the old indices in selected pointing at snippets the candidate never checked, and a Clear followed by a new pick ships them under a consented captured before the box was unchecked. A follow-up should build the packet before the await, the rule the handler already applies to duration and problem.

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