feat(sources): real-time file progress for sync and upload — background upload with success toast

2026-09-01 23:51:43 -04:00
parent cddc84c7db
commit 4677d86f49
103 changed files with 5914 additions and 456 deletions
@@ -13,12 +13,31 @@ client-side hard timeout), the page's #sync-result line +
 §7.4 never-stale CSS (spin + reduced-motion opt-out,
 disabled state, 44px floor, contrast pair) — so a silent regression is
 caught without a browser.
+
+Phase 64 (task 02): unit coverage of the ``app/api/sync.py`` status
+contract — the idle response dict is pinned in full (every pre-existing
+key unchanged, plus ``current_file``/``files_done``/``files_total`` as
+null/0/0); mid-run the status reports the file the (mock) import is
+processing, through the runner's own hook closure (no file yet during
+the clone/pull phase — A4); the terminal states clear
+``current_file`` while keeping the run's final counts.
 """
 from __future__ import annotations

+import asyncio
 import re
+import threading
+from collections.abc import Callable, Iterator
 from pathlib import Path

+import pytest
+
+from app.api import sync as sync_api
+from app.config import Settings
+from app.models import GitSource
+from app.rag.importer import ImportSummary
+from app.rag.llm import EmbeddingError
+
 FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
 ASSETS = FRONTEND / "assets"

@@ -178,12 +197,15 @@ def test_sources_js_polls_every_2000ms() -> None:
 def test_sources_js_adopts_409_and_starts_on_202() -> None:
    """202 (started) and 409 (a run started elsewhere — e.g. a second
    tab) both enter the running state and start polling: the UI never
-    starts a second run, it adopts the in-flight one."""
+    starts a second run, it adopts the in-flight one.
+
+    Phase 64 (task 04): the entry is the sync-kind live label with no
+    file yet (the run is just starting — bare "Syncing…", A4)."""
    js = _text(SOURCES_JS)
    assert "r.status === 202 || r.status === 409" in js
    idx = js.find("r.status === 202 || r.status === 409")
-    branch = js[idx : idx + 400]
-    assert "enterSyncRunningState()" in branch
+    branch = js[idx : idx + 600]
+    assert "enterSyncRunningState(\"sync\", null, 0, 0)" in branch
    assert "startSyncPolling()" in branch


@@ -202,19 +224,29 @@ def test_sources_js_hides_the_button_on_403() -> None:

 def test_sources_js_running_state_is_never_stale() -> None:
    """Entering the running state disables the button, sets aria-busy,
-    spins the icon, and swaps the label to 'Syncing…' (the §7.4
-    feedback while the poll waits) — and a fresh run starts clean: the
-    previous failure's title / aria-label / .is-error come off NOW,
-    not when the run settles."""
+    spins the icon, and swaps the label to the live file label
+    (fmtSyncLabel — the §7.4 feedback while the poll waits) — and a
+    fresh run starts clean: the previous failure's title / aria-label /
+    .is-error come off NOW, not when the run settles.
+
+    Phase 64 (task 04): the button title carries the FULL untruncated
+    current file (removed when null — no file yet) and #sync-result
+    (the aria-live announcer) carries the same untruncated label."""
    js = _text(SOURCES_JS)
    body = _body(js, "enterSyncRunningState")
    assert "syncBtn.disabled = true" in body
    assert 'syncBtn.setAttribute("aria-busy", "true")' in body
-    assert 'syncBtn.removeAttribute("title")' in body
+    assert "syncBtn.title = currentFile" in body, "the full path on hover"
+    assert 'syncBtn.removeAttribute("title")' in body, "removed when no file yet"
+    title_if = body.find("if (currentFile) syncBtn.title = currentFile")
+    title_else = body.find("syncBtn.removeAttribute(\"title\")")
+    assert -1 < title_if < title_else, "title is set, not removed, only when a file exists"
    assert 'syncBtn.setAttribute("aria-label", "Sync sources")' in body
    assert "syncBtn.classList.remove(\"is-error\")" in body
    assert "syncIcon.classList.add(\"is-spinning\")" in body
-    assert '"Syncing…"' in body
+    assert "fmtSyncLabel(kind, currentFile, done, total)" in body
+    assert "syncLabel.textContent = label" in body
+    assert "syncResult.textContent = label" in body, ("the announcer reads the full live path")


 def test_sources_js_terminal_states() -> None:
@@ -595,3 +627,276 @@ def test_sync_modal_respects_reduced_motion() -> None:
    )
    assert reduced, "the backdrop fade must opt out under prefers-reduced-motion"
    assert "transition: none" in reduced.group(1)
+
+
+# ---------- phase 64 (task 02): per-file progress on the sync status ----------
+#
+# The GET /api/sync/status contract in app/api/sync.py (task 04 renders
+# the live file label on the button — its pins live in
+# tests/unit/test_frontend_sync_upload.py). The runner's seams are
+# monkeypatched on ``app.api.sync`` (the house mock-import pattern from
+# tests/integration/test_sync_api.py); the background task runs on a
+# worker thread's own loop so the test can read the status mid-run.
+# No DB, no HTTP: every seam is faked.
+
+
+@pytest.fixture()
+def fresh_sync_status() -> Iterator[None]:
+    """The module-level status object + task are process-global: reset
+    them before AND after every progress test (the integration suite's
+    ``_fresh_sync_state`` pattern)."""
+    sync_api._status = sync_api.SyncStatus()
+    sync_api._task = None
+    yield
+    sync_api._status = sync_api.SyncStatus()
+    sync_api._task = None
+
+
+class _GatedClone:
+    """A ``clone_or_pull`` that parks between start and finish on a
+    threading gate, so the test can read the status during the
+    clone/pull phase (A4: no file yet)."""
+
+    def __init__(self, started: threading.Event, release: threading.Event) -> None:
+        self.started = started
+        self.release = release
+        self.calls: list[tuple[str, Path]] = []
+
+    def __call__(self, url: str, dest: Path | str) -> Path:
+        dest = Path(dest)
+        self.calls.append((url, dest))
+        dest.mkdir(parents=True, exist_ok=True)
+        (dest / "notes.md").write_text("# repo\ncontent for the KB\n", encoding="utf-8")
+        self.started.set()
+        self.release.wait()  # blocking is fine: the clone is a sync call
+        return dest
+
+
+class _GatedImport:
+    """The mock import: fires the runner's OWN progress hook once (the
+    progress-shaped call goes through the real hook closure — the
+    closure under test), parks on a threading gate so the test can read
+    the status mid-run, then returns the canned summary (or raises
+    ``fail``)."""
+
+    def __init__(
+        self,
+        summary: ImportSummary,
+        started: threading.Event,
+        release: threading.Event,
+        fail: BaseException | None = None,
+    ) -> None:
+        self.summary = summary
+        self.started = started
+        self.release = release
+        self.fail = fail
+        self.hook_calls: list[tuple[str, str, int, int]] = []
+        self.prune_flags: list[bool] = []
+
+    async def __call__(
+        self,
+        sources: list[Path],
+        llm: object,
+        *,
+        prune: bool = False,
+        limit: int | None = None,
+        session: object = None,
+        progress: Callable[[str, str, int, int], None] | None = None,
+    ) -> ImportSummary:
+        self.prune_flags.append(prune)
+        if progress is not None:
+            progress("repo", "notes/deep.md", 1, 3)
+            self.hook_calls.append(("repo", "notes/deep.md", 1, 3))
+        self.started.set()
+        await asyncio.to_thread(self.release.wait)  # park without freezing the worker loop
+        if self.fail is not None:
+            raise self.fail
+        return self.summary
+
+
+def _patch_sync_seams(
+    monkeypatch: pytest.MonkeyPatch,
+    tmp_path: Path,
+    fake_import: _GatedImport,
+    fake_clone: _GatedClone,
+) -> None:
+    """The runner's seams, monkeypatched on ``app.api.sync`` (the house
+    mock-import pattern): fresh settings (no ``.env`` leak), a no-op
+    model probe, a sentinel LLM client, one git row, the gated clone +
+    import, a no-op overview, and the DB-free sources-version step
+    (dummy session + pinned counters)."""
+    monkeypatch.setattr(
+        sync_api,
+        "get_settings",
+        lambda: Settings(_env_file=None, sources_dir=str(tmp_path / "bor")),  # pyright: ignore[reportCallIssue]
+    )
+
+    async def fake_probe(llm: object) -> None:
+        pass
+
+    monkeypatch.setattr(sync_api, "check_models", fake_probe)
+    monkeypatch.setattr(sync_api, "LLMClient", lambda: object())
+    monkeypatch.setattr(
+        sync_api,
+        "effective_sources",
+        lambda session: (
+            # kind explicit: the Python-side default applies at INSERT
+            # flush, not on an in-memory instance
+            [GitSource(url="https://git.example.com/repo.git", kind="git")],
+            "db",
+        ),
+    )
+    monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone)
+    monkeypatch.setattr(sync_api, "import_sources", fake_import)
+
+    async def fake_overview(llm: object, session: object = None) -> bool:
+        return False
+
+    monkeypatch.setattr(sync_api, "regenerate_overview", fake_overview)
+
+    class _DummySession:
+        def close(self) -> None:
+            pass
+
+        def commit(self) -> None:
+            pass
+
+    monkeypatch.setattr(sync_api, "SessionLocal", _DummySession)
+    monkeypatch.setattr(sync_api, "bump_sources_version", lambda session: 1)
+    monkeypatch.setattr(sync_api, "current_sources_version", lambda session: 1)
+
+
+def _start_run() -> tuple[threading.Thread, list[BaseException]]:
+    """Run the module-level runner on a worker thread's own event loop
+    (the house background-task pattern), capturing any unexpected
+    exception — the runner is supposed to die in state, never raise."""
+    errors: list[BaseException] = []
+
+    def _run() -> None:
+        try:
+            asyncio.run(sync_api._run_sync())
+        except BaseException as e:  # noqa: BLE001 — surfaced to the test
+            errors.append(e)
+
+    thread = threading.Thread(target=_run, daemon=True)
+    thread.start()
+    return thread, errors
+
+
+def test_idle_status_pins_full_shape_including_progress_keys(
+    fresh_sync_status: None,
+) -> None:
+    """Idle: the three phase-64 progress keys ride along as null/0/0,
+    and EVERY pre-existing key is unchanged — the full response dict is
+    pinned, so the current UI and every existing consumer keep working."""
+    assert sync_api.sync_status() == {
+        "state": "idle",
+        "started_at": None,
+        "finished_at": None,
+        "detail": {},
+        "error": None,
+        "current_file": None,
+        "files_done": 0,
+        "files_total": 0,
+    }
+
+
+def test_mid_run_status_reports_current_file(
+    fresh_sync_status: None,
+    monkeypatch: pytest.MonkeyPatch,
+    tmp_path: Path,
+) -> None:
+    """Mid-run: the status carries the file the (mock) import is
+    processing — assigned through the runner's own hook closure; during
+    the clone/pull phase (A4) no file is reported yet. The success
+    terminal clears ``current_file`` but keeps the final counts."""
+    clone = _GatedClone(threading.Event(), threading.Event())
+    fake_import = _GatedImport(
+        ImportSummary(files=3, added=1, updated=1, unchanged=1),
+        threading.Event(),
+        threading.Event(),
+    )
+    _patch_sync_seams(monkeypatch, tmp_path, fake_import, clone)
+
+    thread, errors = _start_run()
+    try:
+        assert clone.started.wait(5.0), "the run never reached the clone phase"
+        # Clone/pull phase: running — but no file yet (A4: bare "Syncing…").
+        s = sync_api.sync_status()
+        assert s["state"] == "running"
+        assert s["current_file"] is None
+        assert s["files_done"] == 0 and s["files_total"] == 0
+        clone.release.set()
+        assert fake_import.started.wait(5.0), "the run never reached the import"
+        # Import phase: the hook's file is live on the status.
+        s = sync_api.sync_status()
+        assert s["state"] == "running"
+        assert s["current_file"] == "repo/notes/deep.md"
+        assert s["files_done"] == 1
+        assert s["files_total"] == 3
+        fake_import.release.set()
+    finally:
+        clone.release.set()
+        fake_import.release.set()
+        thread.join(10.0)
+    assert not thread.is_alive()
+    assert errors == [], f"the runner raised: {errors!r}"
+    # Wiring: prune=True is preserved, and the hook fired through the
+    # runner's own closure (the recorded call is what the closure
+    # assigned to the status above).
+    assert fake_import.prune_flags == [True]
+    assert fake_import.hook_calls == [("repo", "notes/deep.md", 1, 3)]
+    # Success terminal: current_file null, final counts retained.
+    s = sync_api.sync_status()
+    assert s["state"] == "success"
+    assert s["current_file"] is None
+    assert s["files_done"] == 1 and s["files_total"] == 3
+    assert s["error"] is None
+    assert s["started_at"] is not None and s["finished_at"] is not None
+
+
+def test_failed_terminal_clears_current_file_keeps_counts(
+    fresh_sync_status: None,
+    monkeypatch: pytest.MonkeyPatch,
+    tmp_path: Path,
+) -> None:
+    """Terminal (failed): a run that dies inside the import clears
+    ``current_file`` but keeps the hook's final counts — the last
+    position is useful context next to the (sanitized) error."""
+    clone = _GatedClone(threading.Event(), threading.Event())
+    fake_import = _GatedImport(
+        ImportSummary(),
+        threading.Event(),
+        threading.Event(),
+        fail=EmbeddingError(
+            "embeddings request to https://u:p@aipi.example.com/v1 "
+            "failed: connection refused"
+        ),
+    )
+    _patch_sync_seams(monkeypatch, tmp_path, fake_import, clone)
+
+    thread, errors = _start_run()
+    try:
+        assert clone.started.wait(5.0), "the run never reached the clone phase"
+        clone.release.set()
+        assert fake_import.started.wait(5.0), "the progress hook never fired"
+        # While the (about-to-fail) import is parked: the file is live.
+        s = sync_api.sync_status()
+        assert s["state"] == "running"
+        assert s["current_file"] == "repo/notes/deep.md"
+        fake_import.release.set()
+    finally:
+        clone.release.set()
+        fake_import.release.set()
+        thread.join(10.0)
+    assert not thread.is_alive()
+    assert errors == []
+    s = sync_api.sync_status()
+    assert s["state"] == "failed"
+    assert s["current_file"] is None  # cleared in the terminal state
+    assert s["files_done"] == 1 and s["files_total"] == 3  # final counts kept
+    assert s["detail"] == {}
+    error = s["error"] or ""
+    assert "*****@aipi.example.com" in error  # credentials masked
+    assert "u:p" not in error
+    assert "connection refused" in error  # the reason survives