phase: 98_sync_summary_visibility
All verification complete. Final report: **Phase 98 — Sync summary visibility: final verification pass** (all 5 tasks already complete; implementation verified against the design, no defects found, no code changes needed) - **Implementation checked:** `SyncStatus` phase machine (4 new keys, terminal-keep counts), `on_progress` hook in `generate_folder_summaries`, `summary_pending` on `KbTreeSource`/`KbTreeFolder` + D3 rule in `build_kb_tree`, phase-aware sync labels + pending UI in `sources.js`, `.kb-summary-pending` CSS — all match decisions D1–D5. - **Unit + integration:** `uv run pytest` → 2184 tests, 0 failed/errors (exit 0) - **Coverage:** `uv run pytest --cov=app --cov-report=term-missing` → **99%** on `app/` (criterion >90% ✓; `app/api/sync.py` and `app/rag/folder_summaries.py` at 100%) - **Lint/types:** `uv run ruff check .` → All checks passed; `uv run pyright` → 0 errors, 0 warnings - **Phase E2E (isolation):** `uv run pytest tests/e2e/test_sync_summary_visibility.py -v --no-cov` → **3 passed** (phase machine, live label, pending markers + gap-fill self-heal) - **Regression suites (each isolated, `--no-cov`):** test_kb_tree ✓, test_ls_tree_drilldown 3 ✓, test_sync_button 3 ✓, test_sync_upload_progress 4 ✓, test_oneshot_llm_retry 2 ✓, test_local_directory_sources 3 ✓ - **Completion criteria:** all 7 verified green — status phase fields + terminal semantics; `Writing KB overview…`/`Summarizing folders… (n/m)` labels (title + aria-live); pending set == `missing_folder_summaries` (integration cross-check pinned at `test_docs_api.py:428`); CLI/`ls` byte-identity (no changes to those paths, pins green); suite/coverage/lint gates; dedicated + regression E2E. Commit left to the harness per protocol (no `git add`/`commit` run). - **Decisions/deviations:** none — no fixes were required this pass. - **Next pending phase:** `99_kb_tree_table_and_back_nav`.
This commit is contained in:
@@ -69,6 +69,7 @@ from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
import threading
|
||||
import time
|
||||
from collections.abc import Callable, Iterator
|
||||
from datetime import UTC, datetime
|
||||
@@ -315,16 +316,29 @@ class FakeFolderSummaries:
|
||||
generator with ``only_missing=True`` — the fake records the flag
|
||||
the same way it records ``skip`` (the real gap probe,
|
||||
``missing_folder_summaries``, runs against the real tables).
|
||||
Phase 98 (task 01): records the ``on_progress`` hook the runner
|
||||
wires into the generation branches (a live closure while the
|
||||
wiring holds, None if it regresses); a canned *progress* list of
|
||||
``(done, total, source, folder_path)`` steps is fired through the
|
||||
hook when given — the same way the import's file counter is driven
|
||||
through its hook, so the status's summary counters are testable
|
||||
deterministically.
|
||||
"""
|
||||
|
||||
ZERO = {"generated": 0, "failed": 0, "pruned": 0}
|
||||
|
||||
def __init__(self, stats: dict[str, int] | None = None) -> None:
|
||||
def __init__(
|
||||
self,
|
||||
stats: dict[str, int] | None = None,
|
||||
progress: list[tuple[int, int, str, str]] | None = None,
|
||||
) -> None:
|
||||
self.stats = stats if stats is not None else dict(self.ZERO)
|
||||
self.progress_steps = list(progress or [])
|
||||
self.llms: list[LLMClient] = []
|
||||
self.sessions: list[Session] = []
|
||||
self.skip_flags: list[bool] = []
|
||||
self.only_missing_flags: list[bool] = []
|
||||
self.progress_hooks: list[Callable[[int, int, str, str], None] | None] = []
|
||||
|
||||
async def __call__(
|
||||
self,
|
||||
@@ -333,13 +347,18 @@ class FakeFolderSummaries:
|
||||
*,
|
||||
skip: bool = False,
|
||||
only_missing: bool = False,
|
||||
on_progress: Callable[[int, int, str, str], None] | None = None,
|
||||
) -> dict[str, int]:
|
||||
self.skip_flags.append(skip)
|
||||
self.only_missing_flags.append(only_missing)
|
||||
self.progress_hooks.append(on_progress)
|
||||
if skip:
|
||||
return dict(self.ZERO)
|
||||
self.llms.append(llm)
|
||||
self.sessions.append(db)
|
||||
for done, total, source, folder_path in self.progress_steps:
|
||||
if on_progress is not None:
|
||||
on_progress(done, total, source, folder_path)
|
||||
return dict(self.stats)
|
||||
|
||||
|
||||
@@ -394,7 +413,13 @@ def test_admin_sync_success_reports_full_detail(
|
||||
monkeypatch.setattr(sync_api, "import_sources", fake_import)
|
||||
fake_overview = FakeOverview(ok=True)
|
||||
monkeypatch.setattr(sync_api, "regenerate_overview", fake_overview)
|
||||
monkeypatch.setattr(sync_api, "generate_folder_summaries", FakeFolderSummaries())
|
||||
# Phase 98 (task 01): the canned summary steps fire through the
|
||||
# runner's live hook closure — the terminal's summary counters
|
||||
# (kept per the keep-final-counts convention) are pinned below.
|
||||
fake_folders = FakeFolderSummaries(
|
||||
progress=[(1, 2, "repo", ""), (2, 2, "repo", "a")]
|
||||
)
|
||||
monkeypatch.setattr(sync_api, "generate_folder_summaries", fake_folders)
|
||||
|
||||
_login(sync_client)
|
||||
assert sync_client.get("/api/sync/status").json() == {
|
||||
@@ -407,6 +432,11 @@ def test_admin_sync_success_reports_full_detail(
|
||||
"current_file": None,
|
||||
"files_done": 0,
|
||||
"files_total": 0,
|
||||
# Phase 98 (task 01): the phase-machine keys — null/0/0/0 idle.
|
||||
"phase": None,
|
||||
"current_summary": None,
|
||||
"summaries_done": 0,
|
||||
"summaries_total": 0,
|
||||
}
|
||||
|
||||
r = sync_client.post("/api/sync")
|
||||
@@ -445,6 +475,15 @@ def test_admin_sync_success_reports_full_detail(
|
||||
# Phase 41: the probe ran first and got the very client the import
|
||||
# and the overview reuse.
|
||||
assert probe_seen == [fake_import.llms[0]]
|
||||
# Phase 98 (task 01): the summary hook was wired into the
|
||||
# changed-KB branch and fired the canned steps — the terminal
|
||||
# clears phase + current_summary and KEEPS the hook's final
|
||||
# summary counts (the phase-64 keep-final-counts convention).
|
||||
assert len(fake_folders.progress_hooks) == 1
|
||||
assert fake_folders.progress_hooks[0] is not None
|
||||
assert body["phase"] is None
|
||||
assert body["current_summary"] is None
|
||||
assert body["summaries_done"] == 2 and body["summaries_total"] == 2
|
||||
|
||||
|
||||
def test_unchanged_kb_skips_overview_refresh(
|
||||
@@ -518,10 +557,279 @@ def test_double_trigger_while_running_returns_409(
|
||||
assert body["started_at"] is not None
|
||||
assert body["finished_at"] is None
|
||||
assert body["error"] is None
|
||||
# Phase 98 (task 01): the in-flight run (parked in the fake
|
||||
# import's delay) reports the import phase — the summary span has
|
||||
# not started, so its counters are 0/0.
|
||||
assert body["phase"] == "import"
|
||||
assert body["current_summary"] is None
|
||||
assert body["summaries_done"] == 0 and body["summaries_total"] == 0
|
||||
|
||||
# The (single) run completes; the import ran exactly once.
|
||||
_poll(sync_client, "success")
|
||||
body = _poll(sync_client, "success")
|
||||
assert len(fake_import.sources) == 1
|
||||
# Phase 98 (task 01): the terminal cleared the phase keys (the
|
||||
# fake folder step fired no hook — nothing to keep).
|
||||
assert body["phase"] is None and body["current_summary"] is None
|
||||
assert body["summaries_done"] == 0 and body["summaries_total"] == 0
|
||||
|
||||
|
||||
# --- phase 98 (task 01): the status's phase machine --------------------
|
||||
# The background task runs on the app's event loop, so every gate below
|
||||
# parks AWAY from the loop (``asyncio.to_thread(event.wait)``) — a
|
||||
# blocking wait on the loop thread would deadlock the very status
|
||||
# endpoint the test is polling.
|
||||
|
||||
|
||||
class _GatedImport:
|
||||
"""An import that fires the runner's progress hook once, then
|
||||
parks on a threading gate — the test reads the status mid-import
|
||||
(the phase ``"import"`` + the file hook)."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
summary: ImportSummary,
|
||||
started: threading.Event,
|
||||
release: threading.Event,
|
||||
) -> None:
|
||||
self.summary = summary
|
||||
self.started = started
|
||||
self.release = release
|
||||
self.hook_calls: list[tuple[str, str, int, int]] = []
|
||||
|
||||
async def __call__(
|
||||
self,
|
||||
sources: list[Path],
|
||||
llm: LLMClient,
|
||||
*,
|
||||
prune: bool = False,
|
||||
limit: int | None = None,
|
||||
session: Session | None = None,
|
||||
progress: Callable[[str, str, int, int], None] | None = None,
|
||||
ignore_by_root: dict[str, list[str]] | None = None,
|
||||
include_hidden_by_root: dict[str, bool] | None = None,
|
||||
) -> ImportSummary:
|
||||
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)
|
||||
return self.summary
|
||||
|
||||
|
||||
class _GatedOverview:
|
||||
"""A KB-overview step that parks on a threading gate once it
|
||||
starts — the test reads the status mid-overview (phase
|
||||
``"overview"``)."""
|
||||
|
||||
def __init__(self, started: threading.Event, release: threading.Event) -> None:
|
||||
self.started = started
|
||||
self.release = release
|
||||
|
||||
async def __call__(self, llm: LLMClient, session: Session | None = None) -> bool:
|
||||
self.started.set()
|
||||
await asyncio.to_thread(self.release.wait)
|
||||
return True
|
||||
|
||||
|
||||
class _GatedFolderSummaries:
|
||||
"""A folder-summary step that fires the runner's progress hook
|
||||
(canned steps), parks mid-span — the (long) phase the user
|
||||
reported — then fires the final steps and returns canned stats.
|
||||
The test reads the status mid-span (the phase ``"summaries"`` +
|
||||
the hook's folder + counters)."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
started: threading.Event,
|
||||
release: threading.Event,
|
||||
steps_before: list[tuple[int, int, str, str]],
|
||||
steps_after: list[tuple[int, int, str, str]],
|
||||
) -> None:
|
||||
self.started = started
|
||||
self.release = release
|
||||
self.steps_before = steps_before
|
||||
self.steps_after = steps_after
|
||||
self.hook: Callable[[int, int, str, str], None] | None = None
|
||||
|
||||
async def __call__(
|
||||
self,
|
||||
db: Session,
|
||||
llm: LLMClient,
|
||||
*,
|
||||
skip: bool = False,
|
||||
only_missing: bool = False,
|
||||
on_progress: Callable[[int, int, str, str], None] | None = None,
|
||||
) -> dict[str, int]:
|
||||
assert not skip, "these runs take a generation branch, never --limit"
|
||||
self.hook = on_progress
|
||||
for done, total, source, folder_path in self.steps_before:
|
||||
if on_progress is not None:
|
||||
on_progress(done, total, source, folder_path)
|
||||
self.started.set()
|
||||
await asyncio.to_thread(self.release.wait)
|
||||
for done, total, source, folder_path in self.steps_after:
|
||||
if on_progress is not None:
|
||||
on_progress(done, total, source, folder_path)
|
||||
return {"generated": 3, "failed": 0, "pruned": 0, "kept_manual": 0}
|
||||
|
||||
|
||||
def test_status_reports_the_phase_machine_across_the_run(
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, db: Session, tmp_path: Path
|
||||
) -> None:
|
||||
"""Phase 98 (task 01), the full state machine: idle reports the
|
||||
four keys null/0/0/0; the model-check + clone/pull prelude
|
||||
reports ``phase: null`` (D1 — the bare label's pin); the import
|
||||
reports ``"import"`` with the file hook (the phase-64 shape,
|
||||
unchanged); the KB-overview step reports ``"overview"``; the
|
||||
folder-summary span reports ``"summaries"`` with the hook's
|
||||
folder + done/total (the import's final file position kept — the
|
||||
pause the user reported); the success terminal clears ``phase`` +
|
||||
``current_summary`` and KEEPS the hook's final summary counts
|
||||
(the phase-64 keep-final-counts convention)."""
|
||||
repo_url = f"file://{tmp_path / 'repo.git'}"
|
||||
_seed(db, repo_url)
|
||||
_stub_env(monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
lambda: _settings(sources_dir=str(tmp_path / "bor")),
|
||||
)
|
||||
_, fake_clone = _fake_clone()
|
||||
monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone)
|
||||
_stub_probe(monkeypatch)
|
||||
gated_import = _GatedImport(
|
||||
ImportSummary(files=3, added=1, updated=1, unchanged=1),
|
||||
threading.Event(),
|
||||
threading.Event(),
|
||||
)
|
||||
monkeypatch.setattr(sync_api, "import_sources", gated_import)
|
||||
gated_overview = _GatedOverview(threading.Event(), threading.Event())
|
||||
monkeypatch.setattr(sync_api, "regenerate_overview", gated_overview)
|
||||
gated_folders = _GatedFolderSummaries(
|
||||
threading.Event(),
|
||||
threading.Event(),
|
||||
steps_before=[(1, 3, "repo", "")],
|
||||
steps_after=[(3, 3, "repo", "notes")],
|
||||
)
|
||||
monkeypatch.setattr(sync_api, "generate_folder_summaries", gated_folders)
|
||||
|
||||
_login(sync_client)
|
||||
# Idle: the four phase keys ride along as null/0/0/0.
|
||||
body = sync_client.get("/api/sync/status").json()
|
||||
assert body["state"] == "idle"
|
||||
assert body["phase"] is None
|
||||
assert body["current_summary"] is None
|
||||
assert body["summaries_done"] == 0 and body["summaries_total"] == 0
|
||||
|
||||
assert sync_client.post("/api/sync").status_code == 202
|
||||
try:
|
||||
assert gated_import.started.wait(5.0), "the run never reached the import"
|
||||
s = sync_client.get("/api/sync/status").json()
|
||||
assert s["state"] == "running"
|
||||
assert s["phase"] == "import"
|
||||
assert s["current_file"] == "repo/notes/deep.md" # the file hook as today
|
||||
assert s["files_done"] == 1 and s["files_total"] == 3
|
||||
assert s["current_summary"] is None
|
||||
assert s["summaries_done"] == 0 and s["summaries_total"] == 0
|
||||
gated_import.release.set()
|
||||
assert gated_overview.started.wait(5.0), "the run never reached the overview"
|
||||
s = sync_client.get("/api/sync/status").json()
|
||||
assert s["state"] == "running"
|
||||
assert s["phase"] == "overview"
|
||||
assert s["current_summary"] is None
|
||||
gated_overview.release.set()
|
||||
assert gated_folders.started.wait(5.0), "the run never reached the summaries"
|
||||
s = sync_client.get("/api/sync/status").json()
|
||||
assert s["state"] == "running"
|
||||
assert s["phase"] == "summaries"
|
||||
# The hook's first step: the source-root row (the bare source
|
||||
# name — folder_path "" never gets a slash).
|
||||
assert s["current_summary"] == "repo"
|
||||
assert s["summaries_done"] == 1 and s["summaries_total"] == 3
|
||||
# The import's final position is kept through the summary span
|
||||
# (the "number pauses" span — the user's report).
|
||||
assert s["files_done"] == 1 and s["files_total"] == 3
|
||||
gated_folders.release.set()
|
||||
finally:
|
||||
gated_import.release.set()
|
||||
gated_overview.release.set()
|
||||
gated_folders.release.set()
|
||||
|
||||
body = _poll(sync_client, "success")
|
||||
# Terminal: phase + current_summary cleared ... (the final step
|
||||
# fired post-park, so the kept counts are its position).
|
||||
assert body["phase"] is None
|
||||
assert body["current_summary"] is None
|
||||
assert body["summaries_done"] == 3 and body["summaries_total"] == 3
|
||||
# Wiring: the summary hook was the runner's live closure (the
|
||||
# counters above came through it); the file hook fired once.
|
||||
assert gated_folders.hook is not None
|
||||
assert gated_import.hook_calls == [("repo", "notes/deep.md", 1, 3)]
|
||||
|
||||
|
||||
def test_failed_terminal_after_the_summary_hook_clears_phase_keeps_counts(
|
||||
sync_client: TestClient, monkeypatch: pytest.MonkeyPatch, db: Session, tmp_path: Path
|
||||
) -> None:
|
||||
"""Phase 98 (task 01), the except terminal path: a run that dies
|
||||
AFTER the summary hook has fired (here: the folder step raises
|
||||
post-hook) clears ``phase`` + ``current_summary`` in the failed
|
||||
terminal AND keeps the hook's final summary counts next to the
|
||||
error — the same keep-final-counts convention as success (D1:
|
||||
BOTH terminal paths)."""
|
||||
repo_url = f"file://{tmp_path / 'repo.git'}"
|
||||
_seed(db, repo_url)
|
||||
_stub_env(monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
sync_api,
|
||||
"get_settings",
|
||||
lambda: _settings(sources_dir=str(tmp_path / "bor")),
|
||||
)
|
||||
_, fake_clone = _fake_clone()
|
||||
monkeypatch.setattr(sync_api, "clone_or_pull", fake_clone)
|
||||
_stub_probe(monkeypatch)
|
||||
fake_import = FakeImportSources(
|
||||
ImportSummary(files=3, added=1, updated=1, unchanged=1)
|
||||
)
|
||||
monkeypatch.setattr(sync_api, "import_sources", fake_import)
|
||||
monkeypatch.setattr(sync_api, "regenerate_overview", FakeOverview(ok=True))
|
||||
|
||||
class _HookThenFail:
|
||||
"""Fires two progress steps through the runner's hook, then
|
||||
raises — the failure lands AFTER the hook moved the
|
||||
counters (the keep-counts path the success test pins for the
|
||||
happy terminal)."""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.hook: Callable[[int, int, str, str], None] | None = None
|
||||
|
||||
async def __call__(
|
||||
self,
|
||||
db: Session,
|
||||
llm: LLMClient,
|
||||
*,
|
||||
skip: bool = False,
|
||||
only_missing: bool = False,
|
||||
on_progress: Callable[[int, int, str, str], None] | None = None,
|
||||
) -> dict[str, int]:
|
||||
self.hook = on_progress
|
||||
assert on_progress is not None, "the runner must wire the hook"
|
||||
on_progress(1, 3, "repo", "")
|
||||
on_progress(2, 3, "repo", "notes")
|
||||
raise RuntimeError("simulated post-hook failure (test sentinel)")
|
||||
|
||||
failing = _HookThenFail()
|
||||
monkeypatch.setattr(sync_api, "generate_folder_summaries", failing)
|
||||
|
||||
_login(sync_client)
|
||||
assert sync_client.post("/api/sync").status_code == 202
|
||||
body = _poll(sync_client, "failed")
|
||||
|
||||
assert "simulated post-hook failure" in body["error"]
|
||||
assert body["phase"] is None # cleared in the failed terminal
|
||||
assert body["current_summary"] is None
|
||||
# The hook's FINAL summary counts survive the failure (D1).
|
||||
assert body["summaries_done"] == 2 and body["summaries_total"] == 3
|
||||
assert failing.hook is not None # the wiring held
|
||||
|
||||
|
||||
# --- admin: failures -------------------------------------------------------
|
||||
@@ -560,6 +868,15 @@ def test_git_failure_marks_failed_and_skips_import(
|
||||
assert "fatal: repository not found" in body["error"]
|
||||
assert body["detail"] == {}
|
||||
assert body["finished_at"] is not None
|
||||
# Phase 98 (task 01): the failed terminal (died in the clone/pull
|
||||
# prelude — ``phase`` was still null) clears the phase keys; the
|
||||
# run never reached the import or summary spans, so all counters
|
||||
# are 0/0.
|
||||
assert body["phase"] is None
|
||||
assert body["current_summary"] is None
|
||||
assert body["current_file"] is None
|
||||
assert body["files_done"] == 0 and body["files_total"] == 0
|
||||
assert body["summaries_done"] == 0 and body["summaries_total"] == 0
|
||||
assert fake_import.sources == [] # no partial import
|
||||
assert fake_overview.llms == []
|
||||
# Phase 53: a FAILED sync never bumps — the version is untouched.
|
||||
@@ -847,6 +1164,12 @@ def test_import_error_is_reported_with_credentials_masked(
|
||||
assert "*****@aipi.reeseapps.com" in body["error"] # credentials masked
|
||||
assert "user:secret" not in body["error"]
|
||||
assert "connection refused" in body["error"] # the reason survives
|
||||
# Phase 98 (task 01): the failed terminal (died mid-import) clears
|
||||
# phase + current_summary; the run never reached the summary span,
|
||||
# so those counters are 0/0.
|
||||
assert body["phase"] is None
|
||||
assert body["current_summary"] is None
|
||||
assert body["summaries_done"] == 0 and body["summaries_total"] == 0
|
||||
|
||||
|
||||
# --- phase 41: model probe (fail fast before any clone) --------------------
|
||||
|
||||
Reference in New Issue
Block a user