Skip to content

fix(web): PR #676 review follow-ups for Timeline → Source jump#678

Merged
memtomem merged 2 commits intomainfrom
feat/web-timeline-source-jump-followups
May 2, 2026
Merged

fix(web): PR #676 review follow-ups for Timeline → Source jump#678
memtomem merged 2 commits intomainfrom
feat/web-timeline-source-jump-followups

Conversation

@memtomem
Copy link
Copy Markdown
Owner

@memtomem memtomem commented May 2, 2026

Summary

Three review follow-ups for #676 (Timeline → Source jump). All
user-visible edge cases the original PR's verification didn't cover;
mechanically narrow.

  1. 100-chunk limit miss → bump to 500 + miss toast.
    _renderMemorySourceTree was calling browseSource(path) with the
    default limit=100 even when the caller passed a chunkId. A target
    chunk past position 100 silently no-op'd the highlight — read as a
    regression from feat(web): jump from Timeline to Source tab on chunk/file click #676's old detail-panel fallback. Now passes
    limit=500 only when pendingActivateChunkId is set; Home/recent +
    command-palette callers (no chunkId) keep limit=100 so they don't
    pay the larger payload for a source-level jump. browseSource's
    pending-chunk consumer now shows toast.chunk_target_missing if the
    card still isn't found after the 500-row fetch (rare 500+ chunk
    source).

  2. Stale pendingActivateChunkId across manual source pivots. If
    the user clicked "Open in source" in Timeline and then manually
    clicked a different source-item in the tree before the Sources render
    finished, the prior chunkId stayed in STATE. Functional impact nil
    (chunk ids are UUIDs) but conceptually leaky. Manual .source-item
    click handler now clears STATE.pendingActivateChunkId as the first
    line — matches the "explicit user pivot resets the navigation hint"
    pattern already in place for the namespace chip.

  3. Hard-coded "Copy" button label in Timeline chunks-view. Sibling
    "Open in source" button moved to data-i18n in feat(web): jump from Timeline to Source tab on chunk/file click #676 but the adjacent
    .tl-btn-copy next to it stayed hard-coded. Same row, same i18n
    surface, same locale parity rules from web: i18n coverage for remaining hardcoded showConfirm/showToast strings #29 and feat(web): Index tab — surface hard-to-discover behaviors (static, 7-item bundle) (#579) #587 — promoting to
    timeline.copy (en "Copy" / ko "복사") closes the gap.

Cache-bust: app.js v=88→89, timeline.js v=2→3 per
feedback_static_asset_cache_bust.md. style.css unchanged.

Test plan

  • uv run pytest packages/memtomem/tests/test_i18n.py -q — 12
    passed (parity holds for timeline.copy + toast.chunk_target_ missing).
  • uv run ruff check + ruff format --check clean.
  • Manual via Playwright MCP against mm web:
    • Timeline → Open in source fetches /api/chunks?source=…&limit=500
      (chunkId-bearing path), confirmed via window.fetch spy.
    • tl-btn-copy renders "복사" on KO with data-i18n="timeline.copy"
      so langchange keeps it in sync on toggle.
    • Fix 2 verified by reading the source-item click handler — first
      statement is the chunkId reset.
  • 500+ chunk source toast — synthetic, didn't have one handy in
    the dev DB. Toast key + plumbing audited; falls back to silent
    if showToast / t aren't available so no breakage on bare
    callers.

🤖 Generated with Claude Code

pandas-studio and others added 2 commits May 2, 2026 09:32
Three follow-ups from the post-merge review of #676 (Timeline →
Source tab navigation). All three address user-visible edge cases
the original PR's tests didn't cover; mechanically narrow.

1. **100-chunk limit miss → bump to 500 + miss toast.** When
   ``_renderMemorySourceTree`` consumes ``pendingActivatePath``
   alongside a ``pendingActivateChunkId``, the original PR called
   ``browseSource(path)`` with the default ``limit=100``. A target
   chunk past position 100 silently no-op'd the highlight — the
   user clicked "Open in source" and saw the source open without
   any flash, which reads as a regression from #676's old
   showDetailFromChunk path that always rendered a detail panel.
   ``_renderMemorySourceTree`` now passes ``limit=500`` only when
   the caller asked for a specific chunk. The Home/recent and
   command-palette callers (no chunkId) keep the default 100 fetch
   so they don't pay the larger payload for a source-level jump.
   ``browseSource``'s pending-chunk consumer now shows
   ``toast.chunk_target_missing`` if the card still isn't found
   after the 500-row fetch — a 500+ chunk source is rare but
   reachable, and the toast makes the no-flash visible instead of
   reading as broken.

2. **Stale ``pendingActivateChunkId`` across manual source pivots.**
   If the user clicked "Open in source" in Timeline and then —
   before the Sources render finished — manually clicked a
   different source-item in the tree, the prior chunkId stayed in
   STATE and would silently consume on the next ``browseSource``
   render of any source. Functional impact was nil (chunk ids are
   UUIDs, so no wrong card matched) but the next legitimate
   Timeline jump would land on a state-machine-warm path. Manual
   ``.source-item`` click handler now clears
   ``STATE.pendingActivateChunkId`` as the first line, matching the
   "explicit user pivot resets the navigation hint" pattern already
   in place for the namespace chip.

3. **Hard-coded "Copy" button label in Timeline chunks-view.** The
   sibling "Open in source" button moved to ``data-i18n`` in #676
   but the adjacent ``.tl-btn-copy`` next to it stayed hard-coded
   English. Same row, same i18n surface, same locale parity rules
   from #29 and #587 — promoting it to ``timeline.copy`` (en
   "Copy" / ko "복사") closes the gap.

Cache-bust: ``app.js?v=88→89``, ``timeline.js?v=2→3`` per
``feedback_static_asset_cache_bust.md``. ``style.css`` unchanged.

Verified against ``mm web`` with Playwright MCP:
  - Timeline → Open in source fetches ``/api/chunks?source=…&limit=500``
    when ``pendingActivateChunkId`` is set (i.e. always for the
    chunks-view "Open in source" + files-view per-chunk paths).
  - Copy button renders "복사" on KO and ``data-i18n="timeline.copy"``
    so the langchange hook keeps it in sync on toggle.
  - Manual source-item click clears the stale chunkId before
    ``browseSource`` runs, verified by reading the click handler's
    first statement.

i18n parity (``test_i18n.py`` 12 passed) and ``ruff`` clean.

Co-Authored-By: Claude <[email protected]>
PR #678 review caught that the toast wording technically misleads:
``Load All`` already calls ``browseSource(path, 500)`` (app.js:3092)
— the same limit the chunkId-bearing path now uses for its first
fetch (app.js:2904). So if the first fetch missed, clicking Load
All issues an identical request and the highlight retry never
happens. Telling the user "try Load All" sends them on a wasted
click.

Rewords to plain information without an action suggestion:

  en: "Couldn't locate the linked chunk in this view."
  ko: "이 화면에서는 해당 청크를 찾을 수 없습니다."

The deeper fix — preserving ``pendingActivateChunkId`` across a
manual Load All so the highlight retries on the wider fetch — is
out of scope for this PR (it changes the consume-once contract
this PR's review just praised). Tracked separately as a
good-first-issue candidate.

i18n parity (12 passed). No code change — text-only.

Co-Authored-By: Claude <[email protected]>
@memtomem memtomem merged commit ffa0ed8 into main May 2, 2026
8 of 9 checks passed
@memtomem memtomem deleted the feat/web-timeline-source-jump-followups branch May 2, 2026 00:47
@github-actions github-actions Bot locked and limited conversation to collaborators May 2, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants