Codex final-gate found the boundary<watermark advanced signal made
sidecar_advanced_past_watermark globally true, so a stale ts>watermark suffix
row appearing in state.db BEFORE the edited checkpoint bypassed the skip and
resurrected. Gate the bypass on state_replay_idx >= len(sidecar_visible_sequence)
(checkpoint consumed); apply the same gate to the same-second CORE-B exception.
Regression test added + proven non-vacuous. All legit cases (state-only reply
after checkpoint, CORE-B same-second) still pass.
Co-authored-by: AlexeyDsov <alexeydsov@gmail.com>
Final-gate Codex repro found a non-empty-sidecar drop: when the post-edit USER
turn is checkpointed into the sidecar (ts == advanced watermark) but its
ASSISTANT reply exists only in state.db (ts > watermark), the
sidecar_advanced_past_watermark guard was False (max_sidecar_timestamp ==
watermark, not >), so the 'timestamp > watermark' skip dropped the legitimate
reply. GET /api/session returned ['first','first reply','edited prompt'] and
omitted the assistant.
Fix: treat truncation_boundary < watermark (genuinely advanced) as advanced too,
so post-watermark state-only rows merge through even when the sidecar tail only
EQUALS the watermark. Conservative skip preserved for boundary None / == watermark.
Hoisted the loop-invariant boundary check out of the per-message loop. Regression
test added + proven non-vacuous.
Co-authored-by: AlexeyDsov <alexeydsov@gmail.com>
Opus deep-gate found a latent data-loss the Codex harnesses missed: the
empty-sidecar branch kept at_or_after (ts>=watermark) UNCONDITIONALLY, which is
only valid once the watermark was ADVANCED strictly past the original cutoff
(a new turn committed -> boundary < watermark). In the not-advanced states the
content above the watermark is the deleted suffix, so it got resurrected — the
same data-loss class this fix exists to kill. Reachable via crash/cold-load
metadata-vs-sidecar divergence.
- Keep at_or_after ONLY when boundary < watermark (genuinely advanced).
- boundary == watermark (just truncated) OR boundary is None (legacy: a positive
watermark always meant 'frozen at cutoff' pre-#4767) -> conservative ts<=watermark
filter; never resurrects.
- Replaced the backward-scan heuristic with the provable boundary guard.
- Removed the now-dead 'or (not sidecar_messages and watermark>0)' clause in the
non-empty-sidecar path (unreachable; empty sidecar returns earlier).
- Added 2 regression tests (boundary==watermark, legacy None-boundary), both proven
non-vacuous; updated 2 contributor tests that assumed advanced-watermark-without-boundary
(an unreachable state) to pass the boundary the real persistence path always writes.
Co-authored-by: AlexeyDsov <alexeydsov@gmail.com>
The contributor's empty-sidecar recovery uses the persisted truncation_boundary
to distinguish a legitimate prefix from a deleted suffix, but two production
call sites still passed only truncation_watermark — the default /api/session
full reload (routes.py ~9504) and _merged_session_messages_for_display
(routes.py ~6098) — so a normal full reload fell back to the old 'drop one
turn pair' heuristic and resurrected deleted suffix turns when an OLDER message
was edited leaving >=2 later turns. Thread truncation_boundary through both.
Co-authored-by: AlexeyDsov <alexeydsov@gmail.com>
A later streaming parse-cache change dropped the #MOBILESCROLL guard from
the live-stream render tick while leaving the helper and CSS in place.
On iOS PWA, Safari can paint a transient scrollTop=0 frame during streamed
DOM rebuilds, forcing the reader to the first/oldest message.
Re-add _fixMobileScrollJank() before streaming DOM work begins and pin it
with a regression test so the third layer of the mobile scroll fix cannot
be silently dropped again.
The hardcoded /home/hermes/.local/bin/node doesn't exist in CI runners. Use
shutil.which('node') with a module-level skipif, matching the repo's other
node-driven tests.
The contributor PR had no test for the new helper. Adds Node-driven coverage:
matching-provider prefix stripped, unprefixed untouched, no/mismatched provider
untouched, empty/null passthrough. Co-authored-by: luandnh
The #4295 harness runs the extracted scroll-handler body in a Node new Function();
the body now references _lastMessageClientHeight, so it must be a param + threaded
through state (seeded null, samples keep clientHeight stable so grew stays false and
the pin-reentry assertions are unchanged).
Codex found the guard had a first-event gap: programmatic seeds of _lastScrollTop
(_setMessageScrollToBottom, scrollToBottom, snapshot restore paths) didn't seed
_lastMessageClientHeight, so the FIRST native toolbar-collapse scroll saw a null
prior height -> grew=false -> the false-unpin still fired. Now every programmatic
_lastScrollTop=el.scrollTop is paired with _lastMessageClientHeight=el.clientHeight.
iOS Safari portrait resolves its dynamic toolbar height after first paint; when the
toolbar collapses the #messages scroller grows (clientHeight up), firing a scroll
event with decreased scrollTop that was misread as an upward user scroll -> falsely
set _messageUserUnpinned, and the ResizeObserver settle then self-cancelled, leaving
the reader stranded at the TOP of a freshly-opened conversation (landscape unaffected
since it doesn't resize late). Sibling of #4701.
Fix (touch-safe, no-op on desktop/landscape where clientHeight is stable):
- Track _lastMessageClientHeight; gate movedUp on !grew so a container-height
INCREASE can't be read as an upward scroll. Reset on session switch.
- For explicit (open) settles, also observe the scroller itself so a late portrait
toolbar collapse re-anchors the bottom.
Independent of #4818 (overflow-anchor paint-gap) — different mechanism, coexists.
Diagnosis credit: investigation. Co-authored-by: reinocheong <reporter>
ModuleNotFoundError fires both when cron.jobs is absent AND when an existing
cron/jobs.py fails to import its own dependency. Catch as exc and only return the
graceful {jobs:[],cron_unavailable:true} when exc.name is 'cron'/'cron.jobs';
re-raise otherwise so a real cron bug isn't silently swallowed.
Full-suite caught 7 more per-locale count tests: open_in_vscode (->13, a
pre-existing one-locale gap unrelated to vi), usage_cache_hit_detail/usage_cached_percent
(->14), and the i18n_tooltips LOCALE_COUNT (13->14). vi carries these keys.
In split-container / minimal Docker deployments the WebUI image may not ship the
agent's cron package on its import path, so the Tasks tab's GET /api/crons did
'from cron.jobs import list_jobs' and 500'd with ModuleNotFoundError — breaking the
whole tab. Now the import is guarded and returns {jobs: [], cron_unavailable: true}
instead, so the Tasks tab loads (empty) instead of erroring. Happy path unchanged.
Co-authored-by: reporter
Maintainer-completed @thanhtantran's #4241 (Nathan approved bringing it in):
- vi locale block in static/i18n.js (contributor's 1615-line translation) rebased onto master.
- Added vi to _LOGIN_LOCALE in api/routes.py (login-page localization, issue #1442 parity).
- Bumped the per-locale hardcoded-count assertions 13->14 across the locale-parity
tests (rtl, quota_chip, hide_suggestions, virtualize_transcript, no_git, help_tab,
verdigris/zeus skins, auxiliary_models) and added 'vi' to the composer-voice-buttons
LOCALES tuples. vi already carries all current English keys (verified 14x per key).
All 497 locale/parity tests pass. Co-authored-by: thanhtantran
