Protocol B append (phases 141-145) for the five TODO items: the read-range dedupe false "Already in context" bug (141), the stale KB overview recency ordering + honest intro (142), the mobile keyboard pop after answer completion (143), the document modal back-button history entry (144), and the git original-source links in answers + the doc modal (145). TODO.md cleared (items now live in .agents/phases/todo/). LLM-Generated: true
7.8 KiB
Phase 144 — the browser back button closes the document modal, not the view behind it
Source: TODO.md L6 — "back button doesn't close the modal of you're looking at but instead navigates back behind the modal which can lead to confusion"
Story: n/a (TODO-derived phase, Protocol B append).
Context: frontend/assets/document-modal.js — the single owner of #doc-modal (the near-fullscreen document viewer; openDocumentModal L141+ — trigger memory, the Escape/Tab capture, the fetchSeq stale-guard; closeDocumentModal — hide, drop the capture, restore focus to the trigger; the close affordances: #doc-modal-close, backdrop click, Escape); frontend/assets/router.js — the shell's popstate (L283–286: reads the PATHNAME only — a state-only entry's pop is a no-op for it); frontend/assets/sources.js — the existing STATE-ONLY-entry convention (L237–260, L1539: history.pushState({ view: "rag", kb: target }, "") — the second arg "" keeps the current URL) and its popstate adopt/reset (L1949–1954: adopts the popped entry's kb or resets the drill — a pop onto the SAME state the view already shows is invisible); tests/unit/test_remove_confirm_modal.py (the source-level modal-pin precedent); the phase-99 D2 "the browser owns its entries" convention.
Locked (owner-confirmed 2026-09-30, chat): scope = the document modal (#doc-modal) only — the Sources-page admin confirm dialogs (the remove-confirm / ignore-editor <dialog> elements) are quick confirmations, not "modals you look at", and stay as-is. Back closes the modal via a state-only history entry; forward re-opens the modal (the browser owns its entries).
Objective
On mobile and desktop alike, the browser Back button while the document modal is open CLOSES the modal and leaves the user exactly where they were (same view, same URL, same Sources drill) — instead of navigating the view behind the still-open modal. A Back-then-Forward re-opens the same document; a button/Escape/backdrop close leaves the history exactly as if the modal had never been opened.
Dependencies
143_mobile_keyboard_after_answer(todo) — pipeline order only; no code dependency.
Design (shared by all tasks — the executor reads this, not the chat)
The modal becomes history-aware with ONE state-only entry per open — the sources.js convention, keyed on a modal state key the router (pathname-only) and the sources drill (kb key) both ignore:
- Open (
openDocumentModal, after the skeleton guard — thewindow.openfallback path pushes NOTHING): remember the entry we are sitting on (prev = history.state), then — IF AND ONLY IF the current entry is not already a modal entry —history.pushState({ modal: "doc", source, path }, "")(state-only: the URL does not move). The "not already a modal entry" rule has exactly two callers: the normal open (sitting on the view's entry → push) and the forward re-open below (sitting on the modal's own entry → adopt, no push). If the modal is ALREADY open (defensive — the overlay makes it unreachable by pointer, but the programmatic contract must not stack entries),replaceStatethe existing modal entry in place (the rememberedprevis kept) instead of pushing. - Back (the popstate handler, armed once at module top level like the other bindings):
popstatefires with the state of the entry we LANDED ON. The modal is open and the landed state is NOT amodalentry → the browser moved off the modal's entry → that is the back press: rememberlastPrev = the landed state(the entry below the modal's — needed for the forward re-open's restore), close the modal (aclosingViaBackflag around the call so the close does not alsoreplaceState). The router's popstate is a no-op (same pathname →name === current); the sources drill re-adopts the SAME state it was already showing (itskbkey is the state we sat on before the push — invisible). Net: the modal closes, nothing else moves. - Forward onto the modal's entry (modal closed,
event.state.modal === "doc"): the browser owns its entries (phase-99 D2) — re-open the document the entry names:openDocumentModal(st.source, st.path, null)(trigger null — focus goes to the content target, the normal open path). The open sees the current entry IS a modal entry → ADOPTS it instead of pushing:entry = { prev: lastPrev, source, path }(lastPrev— the below-entry remembered at the back-close; it is always available: a modal entry can only be in the forward stack because a back-close popped it first). - Button / Escape / backdrop close (not a back press): the modal entry is still the current entry — remove it IN PLACE with
history.replaceState(entry.prev, "")(no popstate fires, no other listener runs, the view state is restored byte-for-byte) — so the NEXT back behaves exactly as if the modal had never been opened. Guard: run thereplaceStateonly whenhistory.stateis still a modal entry (the invariant — the three affordances are reachable only while open, and open means sitting on the modal entry; the check is the fail-safe if the invariant ever breaks: do nothing to the history rather than corrupt the current entry). - Module state:
let entry = null—{ prev, source, path }while the modal is open (theprevto restore, the doc the entry names);let lastPrev = null— the below-entry remembered at each back-close (the adopt'sprev); bothnull/stale otherwise. The popstate handler and the open/close paths are the ONLY writers. Known edge (documented, accepted): if the user opens the modal, backs out, switches views, then FORWARDS onto the modal entry, the modal re-opens over the new view — the browser owns its entries (the sources drill does the same with itskbentries); the modal is always one close-away.
Tasks
01_history_aware_modal.md— the push/replace/popstate mechanics indocument-modal.js(open, back, forward, the three close affordances).02_unit_pins.md— the source-level unit pins (thetest_remove_confirm_modal.pypattern) for the entry lifecycle and the listener interplay.03_e2e_back_closes_modal.md— the dedicated E2E: back closes the modal (URL + view + drill untouched), button-close keeps history sane, forward re-opens.
Testing & Quality
- Unit: the source-level pins (the push on open, the
closingViaBack-guarded close, thereplaceState(prev)on non-back close, the popstate handler's two branches, the re-open suppression). - E2E: one dedicated Playwright file, run in isolation, mock-only —
page.goBack()/page.goForward()drive the real history. - Regression:
tests/e2e/test_document_back_navigation.py(the phase-13 /document.html back-nav story — the FULL-PAGE viewer, a different page: it must stay green) and the phase-99 Sources drill E2E (the kb back/forward story — the interplay must not leak). - Coverage: >90% on new/modified code (frontend — the
app/gate stays green).
Completion Criteria
- with the modal open: Back → the modal closes, the URL is unchanged, the view is unchanged, the Sources drill (when opened from Sources) is unchanged; a SECOND Back then leaves the view as usual
- Back → Forward → the same document re-opens in the modal (no duplicate history entry)
- button / Escape / backdrop close → the next Back behaves exactly as if the modal had never been opened (no dead no-op back step)
uv run pytestgreen;uv run pytest --cov=app --cov-report=term-missing—app/coverage >90%;uv run pytest tests/e2e/test_modal_back_button.py -v --no-covgreen in isolation; the two regression E2Es still green in isolation;uv run ruff check . && uv run pyrightclean- no behavior change in completed phases (the router's popstate, the sources drill's adopt/reset, the modal's focus trap / Escape / fetch-seq contracts, the admin confirm dialogs)