Skip to content

feat(web): server-bound /api/indexing/active for cross-tab indicator (#582)#609

Merged
memtomem merged 3 commits intomainfrom
feat/api-indexing-active
Apr 30, 2026
Merged

feat(web): server-bound /api/indexing/active for cross-tab indicator (#582)#609
memtomem merged 3 commits intomainfrom
feat/api-indexing-active

Conversation

@memtomem
Copy link
Copy Markdown
Owner

Summary

Issue #582 item 4.11 follow-up to #602. PR #602 shipped the client-side
STATE.indexing flag and #indexing-indicator pill but explicitly carved
out one remaining gap in its Limitations section:

The indicator is session-bound, not server-bound. If the user reloads
the page mid-indexing, STATE.indexing resets to false and the
indicator hides — but the indexing run continues server-side. … Closing
this gap properly needs a server endpoint like GET /api/indexing/active
that the page queries on load. Tracked as a follow-up in the umbrella;
out of scope for this PR.

This PR closes that gap. Page reload mid-index, or opening a second
browser tab while a run is in flight, now restores the header pill from
server truth and clears it once the run actually finishes.

What changed

Server

  • IndexEngine gains _active_runs: int + is_active read-only
    property. Three public entry points (index_path, index_file,
    index_path_stream) bracket their work with try/finally to inc/dec.
  • The counter is separate from _index_lock deliberately:
    index_path_stream runs outside the lock (so progress events can
    interleave), and asyncio.Lock.locked() is documented as racy. A
    counter (not boolean) lets concurrent stream + locked runs both keep
    the flag on.
  • New GET /api/indexing/active returns {"active": bool} — minimal
    shape matching the client's single-bool model. require_configured
    gate matches sibling indexing routes.

Client

  • _indexingHydrateFromServer() fires once on DOMContentLoaded and
    again on visibilitychange → visible. If the server reports active
    and the local flag is off, it calls _indexingTryStart() and starts
    polling.
  • _indexingPollUntilIdle() runs a single-flight 3 s setTimeout loop;
    on active === false it calls _indexingEnd(). _indexingEnd() also
    cancels any pending poll handle so the in-tab SSE-complete path closes
    the loop too.
  • index.html: app.js?v=78?v=79 (cache-bust).

Test plan

  • uv run pytest -m "not ollama"3337 passed, 46 deselected
  • New tests:
    • TestActiveRunsCounter × 5 — idle, during index_path, during
      index_path_stream (lock-bypass), two concurrent runs ⇒ counter==2,
      finally-decrement on exception
    • TestIndexingActive — idle / running
    • TestRequireConfigured.test_indexing_active_409_when_no_config
      — gate parity with /api/index
  • uv run ruff check packages/memtomem/src && uv run ruff format --check packages/memtomem/src
  • uv run mypy packages/memtomem/src/memtomem/indexing/engine.py packages/memtomem/src/memtomem/web/routes/system.py — clean
  • node --check packages/memtomem/src/memtomem/web/static/app.js
  • Manual verify: reload Index tab mid-run → pill reappears within ~3 s; second tab during the same run shows the pill via visibilitychange

References

🤖 Generated with Claude Code

pandas-studio and others added 3 commits April 30, 2026 16:54
…582)

Issue #582 item 4.11 follow-up. PR #602 shipped the client-side
``STATE.indexing`` flag and ``#indexing-indicator`` pill but documented
one remaining gap in its Limitations section: the indicator is session-
bound, so a page reload mid-indexing resets it to ``false`` even though
the run continues server-side, and a second tab opened during the run
shows nothing. Closing that gap is what this change does.

## Server

``IndexEngine`` gets an ``_active_runs: int`` counter and an ``is_active``
read-only property. The three public entry points (``index_path``,
``index_file``, ``index_path_stream``) bracket their work with
``try/finally`` to increment on entry and decrement on exit.

The counter is deliberately separate from ``_index_lock``:

- ``index_path_stream`` runs **outside** the lock by design (so progress
  events can interleave). A lock-only state probe would miss every SSE
  run, which is the dominant indexing surface.
- ``asyncio.Lock.locked()`` is documented as racy.
- A counter (not boolean) lets concurrent stream + locked runs both keep
  the flag on; matches the engine's actual concurrency model.

New route ``GET /api/indexing/active`` returns ``{"active": bool}`` —
intentionally minimal to match the client's single-bool ``STATE.indexing``
model. Adding ``started_at`` / progress fields later is purely additive.
``require_configured`` is wired so the gate matches sibling indexing
routes (``/index``, ``/index/stream``, ``/reindex``).

## Client

``app.js`` gains two helpers:

- ``_indexingHydrateFromServer()`` fires once on ``DOMContentLoaded`` and
  again on ``visibilitychange`` → visible. If the server reports active
  and ``STATE.indexing`` is false, it calls ``_indexingTryStart()`` and
  starts polling.
- ``_indexingPollUntilIdle()`` runs a single-flight 3 s ``setTimeout``
  loop, calling ``_indexingEnd()`` once the server reports inactive.
  ``_indexingEnd()`` also cancels any pending poll handle so the in-tab
  SSE-complete path closes the loop too.

``index.html`` bumps ``app.js?v=78`` → ``?v=79`` per the static-asset
cache-bust convention.

## Tests

Engine — ``TestActiveRunsCounter`` (5 tests):
- idle, during ``index_path`` (gated), during ``index_path_stream``
  (lock-bypass path), two concurrent ``index_path`` runs both bumping
  the counter to 2 outside the lock, and ``finally``-decrement on the
  inner work raising.

Route — ``TestIndexingActive`` (idle / running) plus a
``TestRequireConfigured`` parity test that asserts the same 409 + body
the rest of the indexing surface returns when ``mm init`` hasn't run.

3337 passed, 46 deselected. Ruff clean. mypy advisory clean on changed
files. ``node --check`` clean on ``app.js``.

## References

- Umbrella: #582 (item 4.11)
- Predecessor: #602 (client-side ``STATE.indexing`` + indicator pill)
- Plan: ``~/.claude/plans/4-11-server-bound-api-indexing-active-fo-reflective-swing.md``

Co-Authored-By: Claude <[email protected]>
…ll handle

Fixes from PR #609 self-review:

1. ``index_path_stream`` docstring no longer asserts the lock-absence is
   "deliberate" — the pre-PR code carried no comment, ADR, or commit
   message backing that intent. Reworded to state the fact (runs outside
   ``_index_lock``) without claiming design rationale we can't cite.

2. ``GET /api/indexing/active`` now returns ``JSONResponse`` with
   ``Cache-Control: no-store`` (mirrors ``/index/stream``). The endpoint
   is polled every ~3 s while a run is in flight, and a cached
   ``{"active": false}`` from an intermediary would mask the
   false→true transition the client is waiting for. New
   ``test_no_store_cache_header`` locks the header in.

3. New ``test_decrements_on_stream_aclose`` covers the realistic SSE-
   disconnect case: consumer ``aclose()`` mid-stream must trigger the
   ``finally`` and return the counter to 0. A future refactor that
   swaps ``try/finally`` for, say, an ``ExitStack`` would silently break
   cancellation cleanup without this guard.

4. ``STATE._indexingPollHandle`` now declared on the initial ``STATE``
   object alongside ``indexing`` — every other state field is declared
   upfront, so this matters for grep/discoverability even though JS
   doesn't require it.

3344 passed (+5 over previous: aclose, no-store, plus the existing 3 in
the surrounding suites). Ruff + ``node --check`` clean.

Co-Authored-By: Claude <[email protected]>
Round-2 review note: the new ``_active_runs`` counter wraps the public
``index_path`` / ``index_file`` / ``index_path_stream`` entry points, so
its scope is wider than ``STATE.indexing`` from #602 (which only covered
the three web-triggered surfaces — Index tab SSE, Sources per-dir,
Sources reindex-all). Watcher-triggered re-indexes, MCP ``mem_edit`` /
``mem_delete``, and CLI ``mm index`` runs all flip the indicator now.

That's the desired behavior (the indicator should reflect *all*
server-side indexing, not just web-triggered) but the previous
docstring still implied the narrower #602 scope. Spell it out so future
readers don't get surprised by the watcher-triggered pill flash.

Pure docstring edit; no behavior change. Ruff clean.

Co-Authored-By: Claude <[email protected]>
@memtomem memtomem merged commit f686862 into main Apr 30, 2026
7 checks passed
@memtomem memtomem deleted the feat/api-indexing-active branch April 30, 2026 08:06
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 30, 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