Skip to content

fix: ignore stale bank selection responses - #28

Merged
chrlesur merged 1 commit into
Cloud-Temple:mainfrom
moul:codex/live-ui-stale-selectbank
Jul 8, 2026
Merged

fix: ignore stale bank selection responses#28
chrlesur merged 1 commit into
Cloud-Temple:mainfrom
moul:codex/live-ui-stale-selectbank

Conversation

@moul

@moul moul commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • capture the selected space and bank filename before loading bank content
  • ignore stale success/error responses when the user switched space or bank meanwhile
  • add a targeted static regression test for the /live bank selection race guard

Fixes #24.

Tests

  • uv run --no-sync python -m pytest tests/test_live_ui_bank_race_guard.py -q
  • uv run --no-sync python -m pytest tests/ -q

Note: the exact uv run pytest tests/ -q command currently fails before pytest because uv.lock is not parseable by this uv version (missing package version at line 431).

@chrlesur chrlesur left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review

Verdict: APPROVE — the guard correctly kills the cross-space stale render described in #24, on every post-await render path.

Checks performed

  • Diff analysis against current main (bank.js unchanged since the PR was opened; merge state CLEAN).
  • All three render paths verified behind the guard: markdown success, non-ok else, and non-Unauthorized catch. The Unauthorized path keeps its existing no-render behavior.
  • Ordering verified: app.currentBankFile = filename runs before the capture, so for two rapid clicks A→B only B renders (A is dropped by the currentBankFile comparison).
  • Space-switch interaction verified: app.js resets currentBankFile = null on space change and renderBankTabs() re-selects the first file, so a dropped stale response is always followed by a fresh render — no orphaned "Loading…" state.
  • tests/test_live_ui_bank_race_guard.py: 2 passed. It asserts both the presence AND the ordering of the guards (guard before each innerHTML write), and it fails if the guard is removed — acceptable rigor given the repo has no JS harness.
  • Full suite on the PR branch: 415 passed + 1 xfailed (baseline 413 + the 2 new tests). Zero regression.

Residual, out of scope for #24 (optional, no change requested)

  1. Same-key race: two concurrent selectBank() calls with the same space and same filename both pass the guard; the older response can land last and win. Distinct from the cross-space bug fixed here; a strict "latest request wins" would need an incremental request token. Not worth blocking this PR.
  2. Same reasoning applies to the A→B→A round-trip during flight: the render is always the right space/right file, but last-writer ordering is not guaranteed. Same fix family as point 1 if it ever matters in practice.

Note: the uv.lock parse issue mentioned in the PR body is tracked by #27 and does not affect this change (suite runs green with the repo's standard test venv).

@chrlesur
chrlesur merged commit 068060a into Cloud-Temple:main Jul 8, 2026
This was referenced Jul 8, 2026
pull Bot pushed a commit to moul/live-memory that referenced this pull request Jul 8, 2026
Bump VERSION, __version__ and version assert to 2.5.4; add CHANGELOG
entry for the batch-failure status fix (Cloud-Temple#32/PR Cloud-Temple#33), the /live stale
selectBank guard (Cloud-Temple#24/PR Cloud-Temple#28), and the LLM budget startup guard.
Refresh the stale version badge (2.5.1 → 2.5.4) in both READMEs.
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.

UI /live: stale selectBank() response can render into the wrong space (pre-existing race)

2 participants