diff --git a/.agents/phases/todo/98_sync_summary_visibility/00_phase.md b/.agents/phases/complete/98_sync_summary_visibility/00_phase.md similarity index 100% rename from .agents/phases/todo/98_sync_summary_visibility/00_phase.md rename to .agents/phases/complete/98_sync_summary_visibility/00_phase.md diff --git a/.agents/phases/todo/98_sync_summary_visibility/01_summary_phase_status.md b/.agents/phases/complete/98_sync_summary_visibility/01_summary_phase_status.md similarity index 100% rename from .agents/phases/todo/98_sync_summary_visibility/01_summary_phase_status.md rename to .agents/phases/complete/98_sync_summary_visibility/01_summary_phase_status.md diff --git a/.agents/phases/todo/98_sync_summary_visibility/02_sync_label_summary_phases.md b/.agents/phases/complete/98_sync_summary_visibility/02_sync_label_summary_phases.md similarity index 100% rename from .agents/phases/todo/98_sync_summary_visibility/02_sync_label_summary_phases.md rename to .agents/phases/complete/98_sync_summary_visibility/02_sync_label_summary_phases.md diff --git a/.agents/phases/todo/98_sync_summary_visibility/03_tree_pending_flag.md b/.agents/phases/complete/98_sync_summary_visibility/03_tree_pending_flag.md similarity index 100% rename from .agents/phases/todo/98_sync_summary_visibility/03_tree_pending_flag.md rename to .agents/phases/complete/98_sync_summary_visibility/03_tree_pending_flag.md diff --git a/.agents/phases/todo/98_sync_summary_visibility/04_tree_pending_ui.md b/.agents/phases/complete/98_sync_summary_visibility/04_tree_pending_ui.md similarity index 100% rename from .agents/phases/todo/98_sync_summary_visibility/04_tree_pending_ui.md rename to .agents/phases/complete/98_sync_summary_visibility/04_tree_pending_ui.md diff --git a/.agents/phases/todo/98_sync_summary_visibility/05_e2e_summary_visibility.md b/.agents/phases/complete/98_sync_summary_visibility/05_e2e_summary_visibility.md similarity index 100% rename from .agents/phases/todo/98_sync_summary_visibility/05_e2e_summary_visibility.md rename to .agents/phases/complete/98_sync_summary_visibility/05_e2e_summary_visibility.md diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__00_phase.a1.err b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__00_phase.a1.md b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__00_phase.a1.md new file mode 100644 index 0000000..61f8f66 --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__00_phase.a1.md @@ -0,0 +1,13 @@ +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`. diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__00_phase.a1.validate b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__00_phase.a1.validate new file mode 100644 index 0000000..fbaf60f --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__00_phase.a1.validate @@ -0,0 +1,98 @@ +........................................................................ [ 3%] +........................................................................ [ 6%] +........................................................................ [ 9%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 19%] +........................................................................ [ 23%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 33%] +........................................................................ [ 36%] +........................................................................ [ 39%] +........................................................................ [ 43%] +........................................................................ [ 46%] +........................................................................ [ 49%] +........................................................................ [ 53%] +........................................................................ [ 56%] +........................................................................ [ 59%] +........................................................................ [ 62%] +........................................................................ [ 66%] +........................................................................ [ 69%] +........................................................................ [ 72%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 82%] +........................................................................ [ 86%] +........................................................................ [ 89%] +........................................................................ [ 92%] +........................................................................ [ 96%] +........................................................................ [ 99%] +............ [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 181 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 94 0 100% +app/api/docs.py 134 1 99% +app/api/git_sources.py 232 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 137 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 140 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 21 0 100% +app/main.py 66 0 100% +app/models.py 126 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 316 1 99% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 200 3 98% +app/rag/llm.py 243 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 88 0 100% +app/rag/retriever.py 150 3 98% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 313 0 100% +-------------------------------------------------- +TOTAL 3903 15 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__01_summary_phase_status.a1.err b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__01_summary_phase_status.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__01_summary_phase_status.a1.md b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__01_summary_phase_status.a1.md new file mode 100644 index 0000000..5aa15f2 --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__01_summary_phase_status.a1.md @@ -0,0 +1,18 @@ +Everything is green. The diff matches the task spec exactly, `scripts/import_docs.py` is untouched, and all gates pass. + +## Report + +**Implemented (phase 98, task 01 — sync status phases + generator progress hook):** +- `app/api/sync.py`: `SyncStatus` gains `phase` / `current_summary` / `summaries_done` / `summaries_total` (defaults null/0); `/status` returns them; `_run_sync` resets all four at run start, sets `"import"` before `import_sources`, `"overview"` inside the changed-KB gate, `"summaries"` + `_summary_hook` on BOTH generation branches (changed-KB regen + gap-fill; no-gap skip stays `"import"`); both terminal paths clear `phase`/`current_summary`, keep final summary counts; module + dataclass + endpoint docstrings carry the phase machine (D1). +- `app/rag/folder_summaries.py`: `generate_folder_summaries(..., on_progress=None)` — hook fires per sorted candidate BEFORE the attempt (manual-skip and failed keys advance; `total = len(keys)` = missing count under `only_missing`), `None` guarded as zero-cost no-op; docstrings updated. `scripts/import_docs.py` verified untouched (no keyword at call site). +- Tests: unit `on_progress` call matrix (6 new tests incl. order/values, skip/fail advance, `only_missing` total, `skip=True`, `None` byte-identical); status-shape pins extended (idle nulls, prelude-null, import-phase, terminals); integration phase-machine test with gated fakes (import→overview→summaries→terminal, keep-final-counts) + post-hook-failure terminal test; hook-firing pins on changed-KB / gap-fill / fail-soft branches and 0/0 on the no-gap skip. +- Fixed pre-existing test-helper defect the matrix exposed: `_FakeLLM` fail-folder matcher was substring-based (`Folder: FSU/a` shadowed `Folder: FSU/a/b`) — now header-line exact. + +**Results:** +- `uv run pytest --cov=app --cov-report=term-missing` → **2156 passed**, TOTAL **99%** (>90% gate; `sync.py` + `folder_summaries.py` both 100%) +- `uv run ruff check .` → All checks passed +- `uv run pyright` → 0 errors, 0 warnings + +**Decisions:** hook recorded via `progress_hooks` + canned steps in `FakeFolderSummaries` (mirrors the file-hook convention); integration gates park via `asyncio.to_thread` (blocking the app loop would deadlock the status endpoint). No deviations from D1/D5. + +**Next pending task:** `02_sync_label_summary_phases.md` (sync button phase-aware labels). diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__01_summary_phase_status.a1.validate b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__01_summary_phase_status.a1.validate new file mode 100644 index 0000000..db76295 --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__01_summary_phase_status.a1.validate @@ -0,0 +1,97 @@ +........................................................................ [ 3%] +........................................................................ [ 6%] +........................................................................ [ 10%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 20%] +........................................................................ [ 23%] +........................................................................ [ 26%] +........................................................................ [ 30%] +........................................................................ [ 33%] +........................................................................ [ 36%] +........................................................................ [ 40%] +........................................................................ [ 43%] +........................................................................ [ 46%] +........................................................................ [ 50%] +........................................................................ [ 53%] +........................................................................ [ 56%] +........................................................................ [ 60%] +........................................................................ [ 63%] +........................................................................ [ 66%] +........................................................................ [ 70%] +........................................................................ [ 73%] +........................................................................ [ 76%] +........................................................................ [ 80%] +........................................................................ [ 83%] +........................................................................ [ 86%] +........................................................................ [ 90%] +........................................................................ [ 93%] +........................................................................ [ 96%] +.................................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 181 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 94 0 100% +app/api/docs.py 134 1 99% +app/api/git_sources.py 232 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 137 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 140 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 21 0 100% +app/main.py 66 0 100% +app/models.py 126 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 316 1 99% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 200 3 98% +app/rag/llm.py 243 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 88 0 100% +app/rag/retriever.py 150 3 98% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 311 0 100% +-------------------------------------------------- +TOTAL 3901 15 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__02_sync_label_summary_phases.a1.err b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__02_sync_label_summary_phases.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__02_sync_label_summary_phases.a1.md b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__02_sync_label_summary_phases.a1.md new file mode 100644 index 0000000..791747b --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__02_sync_label_summary_phases.a1.md @@ -0,0 +1,18 @@ +Everything is green. Task 02 is complete. + +## Report + +**Implemented (phase 98, task 02 — phase-aware sync-button labels):** +- `frontend/assets/sources.js`: new `fmtSyncPhaseLabel(status)` with the exact D2 copy — `"overview"` → `Writing KB overview…`; `"summaries"` → `Summarizing folders… (n/m)` (folder part omitted while `current_summary` is null); anything else → byte-identical phase-64 `fmtSyncLabel("sync", …)` fall-through +- `enterSyncRunningState` gains a 5th `status` arg: sync job + status → phase label; upload job and the 202/409 click keep the bare label; A4 title rule adjusted — the full untruncated label now rides the button `title` for **every** running label (file-only rule removed) + `#sync-result` +- Threading: the poll's sync-running branch and `initSyncButton`'s running re-attach pass the whole status object (mid-summaries reload re-enters with the summaries label); upload call sites untouched; module docstring + decision-tree comments updated +- Pins: `tests/unit/test_frontend_sync_upload.py` (exact D2 strings, ±folder, byte-identical fall-through, status threading, upload negatives) + the adjusted title-rule pin in `tests/unit/test_sync_button.py` + +**Test / lint / coverage results:** +- `uv run pytest` → 2159 passed; `uv run pytest --cov=app` → 99% (gate >90%) +- `uv run ruff check . && uv run pyright` → clean (0 errors) +- E2E regressions, in isolation: `test_sync_upload_progress` 4 ✓, `test_sync_button` 3 ✓, `test_local_directory_sources` 3 ✓, `test_kb_tree` 8 ✓, `test_ls_tree_drilldown` 3 ✓, `test_oneshot_llm_retry` 2 ✓ + +**Decisions:** kept `fmtSyncLabel` untouched (fall-through/upload) and added a separate builder, so all existing byte-identity pins hold; no CSS change (existing `.sync-label` ellipsis covers the longer copy). No E2E file added (deferred to task 05 per the task note). + +**Next pending task:** `.agents/phases/todo/98_sync_summary_visibility/03_tree_pending_flag.md` diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__02_sync_label_summary_phases.a1.validate b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__02_sync_label_summary_phases.a1.validate new file mode 100644 index 0000000..d97530c --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__02_sync_label_summary_phases.a1.validate @@ -0,0 +1,97 @@ +........................................................................ [ 3%] +........................................................................ [ 6%] +........................................................................ [ 10%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 20%] +........................................................................ [ 23%] +........................................................................ [ 26%] +........................................................................ [ 30%] +........................................................................ [ 33%] +........................................................................ [ 36%] +........................................................................ [ 40%] +........................................................................ [ 43%] +........................................................................ [ 46%] +........................................................................ [ 50%] +........................................................................ [ 53%] +........................................................................ [ 56%] +........................................................................ [ 60%] +........................................................................ [ 63%] +........................................................................ [ 66%] +........................................................................ [ 70%] +........................................................................ [ 73%] +........................................................................ [ 76%] +........................................................................ [ 80%] +........................................................................ [ 83%] +........................................................................ [ 86%] +........................................................................ [ 90%] +........................................................................ [ 93%] +........................................................................ [ 96%] +....................................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 181 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 94 0 100% +app/api/docs.py 134 1 99% +app/api/git_sources.py 232 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 137 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 140 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 21 0 100% +app/main.py 66 0 100% +app/models.py 126 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 316 1 99% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 200 3 98% +app/rag/llm.py 243 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 88 0 100% +app/rag/retriever.py 150 3 98% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 311 0 100% +-------------------------------------------------- +TOTAL 3901 15 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__03_tree_pending_flag.a1.err b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__03_tree_pending_flag.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__03_tree_pending_flag.a1.md b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__03_tree_pending_flag.a1.md new file mode 100644 index 0000000..f5b6b03 --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__03_tree_pending_flag.a1.md @@ -0,0 +1,15 @@ +All checks pass. Task complete — final report: + +**Task 03 — `summary_pending` on the tree (phase 98)** + +- `app/schemas.py`: `KbTreeFolder` + `KbTreeSource` gain `summary_pending: bool = False` (wire-additive, `KbTreeFile` untouched); `KbTree` + both node docstrings carry the D3 rule +- `app/api/docs.py`: `MIN_DOCS_PER_FOLDER` imported next to `folder_of`; `_level_children` sets `counts[sub] >= MIN and (source, sub) not in summaries`; `_source_node` sets `len(rows) >= MIN and (source, "") not in summaries`; `build_kb_tree` docstring pins the rule to `missing_folder_summaries` (one concept). Endpoint fetches untouched +- Unit: 6 new tests in `test_kb_tree_builder.py` — full pending matrix (2-doc no row→true; any stored row→false; 1-doc never; name-collision recursive-count edge with nested-row isolation; source root pending + 0-doc source never; two-source independence; file nodes flag-free) +- Integration: 2 new tests in `test_docs_api.py` — pending shape on source + folder nodes (both values, nested); the D3 cross-check: partial summary table (rows deleted) → tree pending set == `set(missing_folder_summaries(db))` == `{("Alpha","a"),("Beta","")}` +- One pre-existing exact key-set assertion (`set(homelab)`) extended to include the new additive key — unavoidable consequence of a wire-additive field; all other shape assertions pass unchanged + +**Results:** `uv run pytest --cov=app` → 2167 tests, exit 0, **99%** on `app/` (>90% gate); `uv run ruff check .` → all checks passed; `uv run pyright` → 0 errors; `uv run pytest tests/e2e/test_kb_tree.py -v --no-cov` → 8 passed (tree E2E regression) + +**Decisions:** none — implementation follows the task spec verbatim. No commits made (working tree left for the harness). + +**Next pending task:** `.agents/phases/todo/98_sync_summary_visibility/04_tree_pending_ui.md` (the row-cell + level-block pending markers + CSS + source pins) diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__03_tree_pending_flag.a1.validate b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__03_tree_pending_flag.a1.validate new file mode 100644 index 0000000..25bf945 --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__03_tree_pending_flag.a1.validate @@ -0,0 +1,98 @@ +........................................................................ [ 3%] +........................................................................ [ 6%] +........................................................................ [ 9%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 19%] +........................................................................ [ 23%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 33%] +........................................................................ [ 36%] +........................................................................ [ 39%] +........................................................................ [ 43%] +........................................................................ [ 46%] +........................................................................ [ 49%] +........................................................................ [ 53%] +........................................................................ [ 56%] +........................................................................ [ 59%] +........................................................................ [ 63%] +........................................................................ [ 66%] +........................................................................ [ 69%] +........................................................................ [ 73%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 83%] +........................................................................ [ 86%] +........................................................................ [ 89%] +........................................................................ [ 93%] +........................................................................ [ 96%] +........................................................................ [ 99%] +....... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 181 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 94 0 100% +app/api/docs.py 134 1 99% +app/api/git_sources.py 232 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 137 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 140 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 21 0 100% +app/main.py 66 0 100% +app/models.py 126 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 316 1 99% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 200 3 98% +app/rag/llm.py 243 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 88 0 100% +app/rag/retriever.py 150 3 98% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 313 0 100% +-------------------------------------------------- +TOTAL 3903 15 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__04_tree_pending_ui.a1.err b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__04_tree_pending_ui.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__04_tree_pending_ui.a1.md b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__04_tree_pending_ui.a1.md new file mode 100644 index 0000000..7755605 --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__04_tree_pending_ui.a1.md @@ -0,0 +1,19 @@ +Task 04 complete. Report: + +**Implemented (phase 98, task 04 — "Summary pending" markers):** +- `sources.js` `makeDescCell`: three-state Description cell — stored summary → text (never marker); no summary + `summary_pending` → span gets `kb-summary-pending` class, text `Summary pending`, D4 title (textContent only); neither → empty cell. Edit button unchanged/unconditional +- `wireDescriptionEdit` success path: `node.summary_pending = false` right after `node.summary = data.summary` (in-place clear, no re-fetch) +- `renderLevel`: level block shows when `node.summary || node.summary_pending`, with the exact D4 pending note as fall-through; neither stays hidden (ls rule) +- `styles.css`: `.kb-summary-pending { color: var(--ink-soft); }` in the phase-97 block (AA, no font/height overrides, no new hue) +- Module docstring: Phase 98 (task 04) section (surfaces + in-place clear) +- Source pins in `test_kb_tree_ui.py`: 4 new tests (marker branch + exact D4 strings, in-place flag clear, CSS class, docstring) + updated 3 existing pins broken by design + +**Results (all green):** +- `uv run pytest` → 2171 passed +- `uv run pytest --cov=app --cov-report=term-missing` → TOTAL 99% (>90% gate) +- `uv run ruff check . && uv run pyright` → clean (0 errors) +- `uv run pytest tests/e2e/test_kb_tree.py -v --no-cov` → 8 passed (phase-97 regression) + +**Decisions:** flag clear is the unconditional single assignment the task specifies (a cleared-then-refetched state self-heals on the next load, same safety-net pattern as the summary itself); `onCleared` guard left untouched since the flag clear keeps it consistent. + +**Next pending task:** `05_e2e_summary_visibility.md` (E2E suite + regression sweep + atomic commit). diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__04_tree_pending_ui.a1.validate b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__04_tree_pending_ui.a1.validate new file mode 100644 index 0000000..b597aaf --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__04_tree_pending_ui.a1.validate @@ -0,0 +1,98 @@ +........................................................................ [ 3%] +........................................................................ [ 6%] +........................................................................ [ 9%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 19%] +........................................................................ [ 23%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 33%] +........................................................................ [ 36%] +........................................................................ [ 39%] +........................................................................ [ 43%] +........................................................................ [ 46%] +........................................................................ [ 49%] +........................................................................ [ 53%] +........................................................................ [ 56%] +........................................................................ [ 59%] +........................................................................ [ 63%] +........................................................................ [ 66%] +........................................................................ [ 69%] +........................................................................ [ 72%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 82%] +........................................................................ [ 86%] +........................................................................ [ 89%] +........................................................................ [ 92%] +........................................................................ [ 96%] +........................................................................ [ 99%] +........... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 181 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 94 0 100% +app/api/docs.py 134 1 99% +app/api/git_sources.py 232 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 137 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 140 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 21 0 100% +app/main.py 66 0 100% +app/models.py 126 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 316 1 99% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 200 3 98% +app/rag/llm.py 243 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 88 0 100% +app/rag/retriever.py 150 3 98% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 313 0 100% +-------------------------------------------------- +TOTAL 3903 15 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__05_e2e_summary_visibility.a1.err b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__05_e2e_summary_visibility.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__05_e2e_summary_visibility.a1.md b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__05_e2e_summary_visibility.a1.md new file mode 100644 index 0000000..02381f5 --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__05_e2e_summary_visibility.a1.md @@ -0,0 +1,10 @@ +All green. Final report: + +**Phase 98 task 05 — complete** + +- **`tests/e2e/test_sync_summary_visibility.py`** (new, A16 suite): 3 tests — (1) the endpoint phase machine via the ~100 ms tight-poll recorder (prelude null → `import` with live file label → `overview` → `summaries` with `current_summary` bare-source/folder + `summaries_done` 1→2→3, every summaries tick `files_done==files_total`; terminal clears phase/folder, keeps `summaries_done==summaries_total==3`); (2) the UI's own 2 s poll renders `Summarizing folders… [folder] (n/3)` (atomic in-browser label+title check, A4), then the phase-32 `Synced HH:MM` settle + counts; (3) fast-mock leg: no markers after sync → delete bravo + root rows → `Summary pending` (copy/class/D4 title, Edit kept) on both rows + pending notes in both level blocks → manual save clears the marker in place → unchanged sync's gap-fill heals the root row, manual row untouched. +- **Defect found & fixed (task-04 work):** `closeEditor` left the stale `kb-summary-pending` class + D4 tooltip after a save (and cancel lost the marker text). Both surfaces now re-derive display state from the node (`pendingText`/`pendingTitle` per surface); `renderLevel` manages the level `

` class (block is reused). New unit pins in `test_kb_tree_ui.py` + level-block pin. +- **Timing:** `SLOW_DELAY_S=1.5` (15× the 100 ms cadence per the task spec) — also makes the ~4.5 s summaries span outlive the UI's 2 s poll period, so the rendered-label assertion is deterministic. Two module apps: slow (proxy 8904, app 8142) for tests 1–2, fast (mock direct, app 8143) for test 3. +- **Gates:** `uv run pytest --cov=app --cov-report=term-missing` → 2172 passed, TOTAL 99% (>90%); `uv run ruff check .` → clean; `uv run pyright` → 0 errors/0 warnings; new E2E `… test_sync_summary_visibility.py -v --no-cov` → 3 passed (×2 runs, ~40 s); regressions in isolation: kb_tree 8, ls_tree_drilldown 3, sync_button 3, sync_upload_progress 4, oneshot_llm_retry 2, local_directory_sources 3 — all passed. +- **Deviation:** no `git add`/commit and no phase-dir move (harness owns the atomic commit, per pipeline rules overriding the task's step 4). +- **Next pending:** `99_kb_tree_table_and_back_nav` (phase 98 finishes once the harness moves task 05). diff --git a/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__05_e2e_summary_visibility.a1.validate b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__05_e2e_summary_visibility.a1.validate new file mode 100644 index 0000000..fbaf60f --- /dev/null +++ b/.agents/reports/98_sync_summary_visibility/98_sync_summary_visibility__05_e2e_summary_visibility.a1.validate @@ -0,0 +1,98 @@ +........................................................................ [ 3%] +........................................................................ [ 6%] +........................................................................ [ 9%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 19%] +........................................................................ [ 23%] +........................................................................ [ 26%] +........................................................................ [ 29%] +........................................................................ [ 33%] +........................................................................ [ 36%] +........................................................................ [ 39%] +........................................................................ [ 43%] +........................................................................ [ 46%] +........................................................................ [ 49%] +........................................................................ [ 53%] +........................................................................ [ 56%] +........................................................................ [ 59%] +........................................................................ [ 62%] +........................................................................ [ 66%] +........................................................................ [ 69%] +........................................................................ [ 72%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 82%] +........................................................................ [ 86%] +........................................................................ [ 89%] +........................................................................ [ 92%] +........................................................................ [ 96%] +........................................................................ [ 99%] +............ [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 181 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 94 0 100% +app/api/docs.py 134 1 99% +app/api/git_sources.py 232 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 137 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 140 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 21 0 100% +app/main.py 66 0 100% +app/models.py 126 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 316 1 99% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 200 3 98% +app/rag/llm.py 243 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 88 0 100% +app/rag/retriever.py 150 3 98% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 313 0 100% +-------------------------------------------------- +TOTAL 3903 15 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/app/api/docs.py b/app/api/docs.py index b2e7c0a..113b628 100644 --- a/app/api/docs.py +++ b/app/api/docs.py @@ -40,7 +40,7 @@ from app.core.auth import require_admin, require_user from app.db import get_db from app.models import Chunk, Document, FolderSummary from app.rag.agent import list_source_names -from app.rag.folder_summaries import folder_of +from app.rag.folder_summaries import MIN_DOCS_PER_FOLDER, folder_of from app.rag.importer import match_extension from app.rag.llm import EmbeddingError, LLMClient from app.schemas import ( @@ -376,6 +376,11 @@ def _level_children( :func:`app.rag.agent.group_folder_listing` level-for-level; the file list is NOT capped (the ``ls`` 50-line cap is a model-context budget — the UI is for humans). Recurses one level per call. + + Each folder node also carries the phase-98 D3 ``summary_pending`` + flag: the recursive count ≥ :data:`MIN_DOCS_PER_FOLDER` AND no + stored ``folder_summaries`` row for ``(source, sub)`` — the same + rule the source node applies (see :func:`build_kb_tree`). """ children: list[KbTreeFolder | KbTreeFile] = [] for sub in sorted(g for g in folders if folder_of(g) == folder): @@ -384,6 +389,8 @@ def _level_children( path=sub, documents=counts[sub], summary=summaries.get((source, sub)), + summary_pending=counts[sub] >= MIN_DOCS_PER_FOLDER + and (source, sub) not in summaries, children=_level_children(source, sub, folders, counts, rows, summaries), ) ) @@ -431,7 +438,22 @@ def build_kb_tree( shape. * **File nodes** — direct files only, in input (catalog) order; ``path`` source-relative; ``title`` / ``chunks`` / ``indexed_at`` - verbatim from the catalogue row. + verbatim from the catalogue row. File nodes carry NO pending + flag (the file table has no description column). + * **Pending** — ``summary_pending`` on the SOURCE and every FOLDER + node (phase 98, decision D3 — ONE concept): true iff the node's + recursive ``documents`` count ≥ + :data:`app.rag.folder_summaries.MIN_DOCS_PER_FOLDER` (2) AND it + has NO stored ``folder_summaries`` row (AI or manual — any row; + the builder sees stored rows only). That is EXACTLY + :func:`app.rag.folder_summaries.missing_folder_summaries`'s + candidate set (phase 96's gap-fill regenerates precisely those + keys on the next sync — the marker is honest: "waiting to + generate", and the integration cross-check pins the tree's + pending set to that function so the marker can never drift from + the gap-fill). A < 2-document folder is NEVER pending (it never + gets a summary — its one file line IS its description), and a + registered 0-document source never is. ONE concept end to end: the builder reuses :func:`app.rag.folder_summaries.folder_of` and the phase-94 @@ -471,13 +493,19 @@ def _source_node( count (every one of its documents, the set its stored ``(source, "")`` summary describes). A source with no rows lists ``documents: 0`` and no children (the registered 0-document source - — the phase-70/72 invariant, extended by the superset rule). + — the phase-70/72 invariant, extended by the superset rule) and + is never ``summary_pending`` (0 < the minimum). + + ``summary_pending`` (phase 98, D3): the whole-source count ≥ + :data:`MIN_DOCS_PER_FOLDER` AND no stored ``(source, "")`` row — + the source-root arm of the rule :func:`build_kb_tree` documents. """ folders, counts = _folder_counts(rows) return KbTreeSource( name=source, documents=len(rows), summary=summaries.get((source, "")), + summary_pending=len(rows) >= MIN_DOCS_PER_FOLDER and (source, "") not in summaries, children=_level_children(source, "", folders, counts, rows, summaries), ) diff --git a/app/api/sync.py b/app/api/sync.py index 5fd2cc3..a22c497 100644 --- a/app/api/sync.py +++ b/app/api/sync.py @@ -75,6 +75,23 @@ carries the phase-64 per-file progress — ``current_file`` (the (clone/pull reports no file yet) and in terminal states, which clear ``current_file`` but keep the run's final counts. +Phase 98 (task 01) adds the run's PHASE machine — ``phase`` is the +pipeline stage the run is in: the model-check + clone/pull prelude +reports ``null`` (the bare "Syncing…" label stays), then ``"import"`` +(set immediately before ``import_sources``), ``"overview"`` (set +before ``regenerate_overview`` — a changed KB only), and +``"summaries"`` (set before a folder-summary generation — the +changed-KB full regeneration or the unchanged-walk gap-fill; the +no-gap skip sets NO phase and the run stays ``"import"``). While in +``"summaries"`` the generator's ``on_progress`` hook fills +``current_summary`` (the ``source`` / ``source/folder_path`` being +summarized — the bare source name for the source-root row) plus +``summaries_done`` / ``summaries_total`` — the per-folder position +through the long summary span where the file count sits still. +Terminal states (success AND failed) clear ``phase`` + +``current_summary`` but keep the run's final ``summaries_done`` / +``summaries_total`` (the phase-64 keep-final-counts convention). + The ``failed`` state's ``error`` string is masked by the shared sanitizer — the ``user:pass@`` masker now lives in :mod:`app.core.errors` (imported here under the private name ``_sanitize_error``). @@ -127,6 +144,16 @@ class SyncStatus: after); ``files_done`` / ``files_total`` carry the hook's done/total position and survive a terminal state (the run's last position is useful context next to the error). + + Phase 98 (task 01) phase fields: ``phase`` is the pipeline stage + (null in the model-check + clone/pull prelude and in terminal + states — the module docstring's phase machine); while in the + ``"summaries"`` phase, ``current_summary`` is the source / + ``source/folder_path`` the generator is summarizing right now and + ``summaries_done`` / ``summaries_total`` carry the generator's + progress hook's position. Terminal states clear ``phase`` + + ``current_summary`` but keep the run's final summary counts (the + phase-64 keep-final-counts convention). """ state: Literal["idle", "running", "success", "failed"] = "idle" @@ -139,6 +166,13 @@ class SyncStatus: current_file: str | None = None files_done: int = 0 files_total: int = 0 + # Phase 98 (task 01): the phase machine — the pipeline stage the + # run is in and, while in the folder-summary phase, the folder + # being summarized plus the hook's done/total position. + phase: Literal["import", "overview", "summaries"] | None = None + current_summary: str | None = None + summaries_done: int = 0 + summaries_total: int = 0 _status = SyncStatus() @@ -153,7 +187,11 @@ def sync_status() -> dict[str, Any]: ``current_file`` (phase 64) is the ``source/relative/path`` the import is processing right now — null during the clone/pull phase and in terminal states; ``files_done`` / ``files_total`` carry the - hook's position (0/0 idle). + hook's position (0/0 idle). ``phase`` (phase 98) is the pipeline + stage — null in the prelude and terminal states; while in + ``"summaries"``, ``current_summary`` + ``summaries_done`` / + ``summaries_total`` carry the generator's position (the terminal + keeps the final summary counts). """ return { "state": _status.state, @@ -164,6 +202,13 @@ def sync_status() -> dict[str, Any]: "current_file": _status.current_file, "files_done": _status.files_done, "files_total": _status.files_total, + # Phase 98 (task 01): the phase machine — null/0/0 idle (the + # dataclass defaults) and in terminal states (which clear + # phase + current_summary but keep the final summary counts). + "phase": _status.phase, + "current_summary": _status.current_summary, + "summaries_done": _status.summaries_done, + "summaries_total": _status.summaries_total, } @@ -202,6 +247,13 @@ async def _run_sync() -> None: _status.current_file = None _status.files_done = 0 _status.files_total = 0 + # Phase 98 (task 01): the phase fields reset with the run — no + # phase until the import starts (the model-check + clone/pull + # prelude reports null), no folder until the summary span starts. + _status.phase = None + _status.current_summary = None + _status.summaries_done = 0 + _status.summaries_total = 0 try: settings = get_settings() # Step 1 (phase 41): fail fast — verify both models the sync @@ -270,12 +322,17 @@ async def _run_sync() -> None: _status.files_done = done _status.files_total = total + # Phase 98 (task 01): the phase machine — the import phase + # starts NOW (the model-check + clone/pull prelude above + # reported ``phase: null``). + _status.phase = "import" summary: ImportSummary = await import_sources( sources, llm, prune=True, progress=_hook, ignore_by_root=ignore_by_root, include_hidden_by_root=include_hidden_by_root, ) overview = False if summary.added + summary.updated > 0: + _status.phase = "overview" # phase 98 (task 01) overview = await regenerate_overview(llm) # Phase 94 (task 02), phase 96 (task 03): the folder # summaries — the drill-down ls's per-level descriptions. A @@ -290,19 +347,42 @@ async def _run_sync() -> None: # outage never flips the run to failed) and only flushes: this # run's own short-lived session commits (the phase-53 # convention), so the step-6 bump stays change-gated on the KB, - # not on the summaries. No status-surface change: the stats - # are log-only (the detail shape is untouched). + # not on the summaries. The status surface is the PHASE + # machine (phase 98, task 01): the stats dict itself stays + # log-only (the detail shape is untouched). + # Phase 98 (task 01): the folder-summary progress hook — the + # closure-captures-``_status`` convention the file's ``_hook`` + # above already uses; the generator's ``total`` (the loop-start + # candidate count) and the bare source name for the root row + # come straight from the hook's arguments (D5's shape). + def _summary_hook(done: int, total: int, source: str, folder_path: str) -> None: + _status.current_summary = ( + source if folder_path == "" else f"{source}/{folder_path}" + ) + _status.summaries_done = done + _status.summaries_total = total + fs_db = SessionLocal() try: if summary.added + summary.updated > 0: - folder_stats = await generate_folder_summaries(fs_db, llm) + # Changed KB: full regeneration — the summaries phase + # starts now, and the hook reports the per-folder + # position through the (long) span. + _status.phase = "summaries" + folder_stats = await generate_folder_summaries( + fs_db, llm, on_progress=_summary_hook + ) fs_db.commit() logger.info("sync: folder_summaries stats=%s", folder_stats) else: missing = missing_folder_summaries(fs_db) if missing: + # Unchanged-walk gap-fill: the SAME summaries + # phase + hook (the fill's total is the missing + # count — the hook's loop-start ``total``). + _status.phase = "summaries" folder_stats = await generate_folder_summaries( - fs_db, llm, only_missing=True + fs_db, llm, only_missing=True, on_progress=_summary_hook ) fs_db.commit() logger.info( @@ -310,6 +390,9 @@ async def _run_sync() -> None: len(missing), folder_stats, ) else: + # The no-gap skip sets NO phase — the run stays + # "import" through to the terminal (the phase-64 + # bare-label pin holds for this shape). logger.info("sync: folder_summaries skipped (KB unchanged)") finally: fs_db.close() @@ -336,6 +419,8 @@ async def _run_sync() -> None: _status.state = "success" _status.finished_at = datetime.now(UTC) _status.current_file = None # phase 64: keep the final counts + _status.phase = None # phase 98: keep the final summary counts + _status.current_summary = None _status.detail = { "files": summary.files, "added": summary.added, @@ -356,3 +441,5 @@ async def _run_sync() -> None: _status.finished_at = datetime.now(UTC) _status.error = _sanitize_error(str(e)) _status.current_file = None # phase 64: keep the final counts + _status.phase = None # phase 98: keep the final summary counts + _status.current_summary = None diff --git a/app/rag/folder_summaries.py b/app/rag/folder_summaries.py index df79317..5cb6c5b 100644 --- a/app/rag/folder_summaries.py +++ b/app/rag/folder_summaries.py @@ -42,7 +42,12 @@ Chat turns never generate folder summaries — the agent's ``ls`` output (phase 94, task 03) only reads the stored rows. Generation is the caller's job at sync time (phase 94, task 02), and :func:`generate_ folder_summaries` only flushes — the sync path owns the transaction -(the phase-53 ``bump_sources_version`` convention). +(the phase-53 ``bump_sources_version`` convention). The sync path +additionally passes the optional ``on_progress`` hook (phase 98, +task 01) so the sync status can report the folder being summarized; +the ``scripts/import_docs.py`` CLI passes none — the hook defaults to +``None`` and is a zero-cost no-op, leaving the CLI's log-only stats +contract untouched. Self-heal (phase 96): an exhausted one-shot retry can still leave a candidate folder without a row. :func:`missing_folder_summaries` @@ -55,7 +60,7 @@ byte-identical (text AND ``updated_at``), the prune pass still runs. from __future__ import annotations import logging -from collections.abc import Sequence +from collections.abc import Callable, Sequence from datetime import UTC, datetime from typing import Protocol @@ -390,6 +395,7 @@ async def generate_folder_summaries( *, skip: bool = False, only_missing: bool = False, + on_progress: Callable[[int, int, str, str], None] | None = None, ) -> dict[str, int]: """Regenerate the stored folder summaries for the current catalogue. @@ -436,6 +442,17 @@ async def generate_folder_summaries( unchanged catalogue it is a no-op (the invariant kept), and it still drops rows whose folder fell below the minimum. + Progress (phase 98, task 01): when *on_progress* is given, it is + called once per candidate with ``(done, total, source, + folder_path)`` in the same sorted ``(source, folder_path)`` order, + BEFORE the attempt — so an instant manual skip and a failed folder + BOTH advance the counter (the UI's position moves on either), and + ``total`` is ``len(keys)`` at loop start (under ``only_missing + =True`` that is the MISSING count, not the full candidate count). + ``None`` — the ``scripts/import_docs.py`` CLI path — is a + zero-cost no-op (guarded at the call site; nothing observable + changes without it). + Only flushes — the CALLER commits (the phase-53 ``bump_sources_version`` convention: the sync path owns the transaction, so a failed sync rolls the summaries back with it). @@ -463,8 +480,15 @@ async def generate_folder_summaries( if only_missing: keys = [key for key in keys if key not in existing] - for key in keys: + for i, key in enumerate(keys): source, folder_path = key + # Phase 98 (task 01): the optional progress hook fires BEFORE + # the attempt — instant manual skips and failed folders both + # advance the counter (D5), and ``total`` is the loop-start + # count (the missing count under ``only_missing``). ``None`` + # (the CLI path) is a zero-cost no-op. + if on_progress is not None: + on_progress(i + 1, len(keys), source, folder_path) stored = existing.get(key) if stored is not None and stored.manually_edited: # Owner-edited description (phase 97, task 01): NEVER diff --git a/app/schemas.py b/app/schemas.py index ccb03e2..5c059fb 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -274,12 +274,21 @@ class KbTreeFolder(BaseModel): (path order) followed by the direct files (catalog order) — the recursive union (Pydantic v2 resolves it with ``from __future__ import annotations``). + + ``summary_pending`` (phase 98, D3 — ONE concept): true iff this + folder's recursive count ≥ ``MIN_DOCS_PER_FOLDER`` (2) AND it has + NO stored ``folder_summaries`` row (AI or manual — any row) — + exactly the candidate the sync-time gap-fill regenerates (the + :func:`app.rag.folder_summaries.missing_folder_summaries` set). + A < 2-document folder is never pending (it never gets a summary — + its one file line IS its description). """ kind: Literal["folder"] = "folder" path: str documents: int = Field(ge=0) summary: str | None = None + summary_pending: bool = False children: list[KbTreeFolder | KbTreeFile] = Field(default_factory=list) @@ -294,11 +303,21 @@ class KbTreeSource(BaseModel): ``(source, "")`` source-root row or null; ``children`` are the source's direct subfolders + direct files (same shape as a folder node's). + + ``summary_pending`` (phase 98, D3 — ONE concept): true iff the + source's recursive ``documents`` count ≥ ``MIN_DOCS_PER_FOLDER`` + (2) AND no stored ``(source, "")`` row (AI or manual — any row) — + exactly the source-root candidate the sync-time gap-fill + regenerates (the + :func:`app.rag.folder_summaries.missing_folder_summaries` set). + A registered 0-document source is never pending (there is nothing + to summarize). """ name: str documents: int = Field(ge=0) summary: str | None = None + summary_pending: bool = False children: list[KbTreeFolder | KbTreeFile] = Field(default_factory=list) @@ -308,6 +327,13 @@ class KbTree(BaseModel): The FULL recursive tree in ONE fetch — the RAG view (admin) drills client-side, zero per-level fetches (the ``00_phase.md`` "The tree endpoint" contract). + + Every SOURCE and FOLDER node carries ``summary_pending`` (phase + 98, D3): true iff its recursive document count ≥ + ``MIN_DOCS_PER_FOLDER`` (2) AND it has no stored ``folder_summaries`` + row — exactly ``missing_folder_summaries``'s candidate set (the + marker never drifts from the gap-fill); FILE nodes carry no flag + (the file table has no description column). """ sources: list[KbTreeSource] diff --git a/frontend/assets/sources.js b/frontend/assets/sources.js index b1f2005..3ebe7bd 100644 --- a/frontend/assets/sources.js +++ b/frontend/assets/sources.js @@ -129,7 +129,11 @@ * summary null) → the text goes away (level block hidden / row * cell emptied) + "Description cleared."; the in-memory kbTree * node's summary is updated IN PLACE (no re-fetch — the tree state - * stays coherent; the re-fetch is the safety net). Failure + * stays coherent; the re-fetch is the safety net) — and, since + * Phase 98 (task 04), its summary_pending flag is cleared IN + * PLACE on the same success path (a created/updated description + * is no longer pending — the D4 marker clears where the edit + * happened, no re-fetch). Failure * (non-2xx / network) → neutral retry copy (the phase-55 * convention), the editor stays open with the user's text, the * stored text untouched. Cancel → restore the text node. @@ -141,6 +145,64 @@ * • every editor part is static createElement; the description text * is a text node (textContent / .value) — this module never builds * HTML from document-derived data (the house rule, unchanged). + * + * Phase 98 (task 02) — the sync button's phase-aware labels: the + * post-import span of a sync (the KB overview + the long + * folder-summary span) no longer reads as a stuck file count. The + * SYNC job's running label is now phase-aware (fmtSyncPhaseLabel — + * the section header below carries the decision tree): "overview" → + * "Writing KB overview…", "summaries" → "Summarizing folders… + * (n/m)" (the folder part omitted while + * current_summary is null — the phase's first poll), anything else + * (the null prelude, "import") → today's "Syncing… (n/m)" + * byte-identical. The UPLOAD job (its status has no phase) keeps the + * bare label. The untruncated label rides the button title (now set + * for EVERY running label, not just file labels) + #sync-result as + * before (A4). + * + * Phase 98 (task 04) — the "Summary pending" markers: the catalog + * tells the owner a summary is WAITING instead of showing an empty + * cell that reads as "missed". A source/folder node is + * summary_pending when its summary is due (recursive count >= 2 — + * the D3 rule, exactly the next sync's gap-fill set) but no stored + * description exists yet. The marker has TWO surfaces, both driven + * by the same node flag: + * + * • the row's Description cell (makeDescCell) — when the stored + * summary is empty AND node.summary_pending, the text span + * carries the class kb-summary-pending (the muted ink-soft pair, + * AA on --surface — text + color, never color alone; the row + * cell's font-size/line-height apply — no row-height change) + + * the text "Summary pending" + the D4 title, verbatim: + * "No stored description yet — the next sync will generate one." + * A stored summary + * shows the stored text (NEVER the marker); neither stored nor + * pending shows the empty cell (the ls rule, unchanged). The + * Edit button is UNCHANGED — always present (a manual save + * creates the row). + * • the level block (renderLevel) — #kb-level shows when the + * current level has a stored description (as today) OR is + * pending: title as today (the full source-relative path), and + * #kb-level-summary shows the stored text, or — when pending — + * the D4 pending note, verbatim: + * "No description stored yet — the next sync will generate one. (You can write one yourself.)" + * Neither stored nor pending stays hidden (the ls rule, + * unchanged). The block's + * Edit button (write one manually now) already ships in the + * static markup (phase 97, task 05). + * • the in-place clear: wireDescriptionEdit's success path sets + * node.summary_pending = false right after + * node.summary = data.summary — a created/updated description is + * no longer pending; the marker clears IN PLACE in the surface + * where the edit happened, no re-fetch (the re-fetch stays the + * safety net). closeEditor re-renders the display state from the + * node with the SAME three-state rule the surfaces use: the + * pending marker's muted class + tooltip CANNOT survive a save + * (a stale "next sync" tooltip under a just-created description), + * and a cancel restores the surface's pending display (each + * surface passes its pending copy via pendingText — the row's + * "Summary pending" marker, the level's D4 note — and its + * tooltip via pendingTitle, the row only). */ import { fetchIsAdmin } from "./header.js"; @@ -187,7 +249,15 @@ function mkBtn(cls, label) { * itself (the ls rule); a row cell just goes empty (the always-present * button stays). Returns a handle whose reset() tears down an OPEN * editor before a re-render (PLAN §7.4 — never a stale open editor). */ -function wireDescriptionEdit({ editBtn, textEl, container, getTarget, onCleared }) { +function wireDescriptionEdit({ + editBtn, + textEl, + container, + getTarget, + onCleared, + pendingText, + pendingTitle, + }) { const actions = document.createElement("div"); actions.className = "kb-summary-actions"; const saveBtn = mkBtn("kb-summary-save", "Save"); @@ -200,15 +270,32 @@ function wireDescriptionEdit({ editBtn, textEl, container, getTarget, onCleared let editor = null; let isOpen = false; - /* Back to the display state: the text element re-rendered from the - * node (the CURRENT stored description, textContent only), the Edit - * button available again. A cleared node empties the text and, for - * the level block, hides the whole block via onCleared. */ + /* Back to the display state: the display state re-rendered from the + * node with the SAME three-state rule the surfaces use (makeDescCell + * / renderLevel — textContent/class/title only, the house rule): + * a stored summary → the stored text with NO marker (the in-place + * clear — the success path cleared node.summary_pending, so the + * muted pending style + the stale "next sync" tooltip cannot survive + * a saved description); no stored text but PENDING → the surface's + * pending display (pendingText — the row's "Summary pending" marker + * + its D4 tooltip, the level's D4 note — a cancel restores exactly + * the pre-edit state, including the marker); neither → the empty + * text. The Edit button is available again. A cleared node empties + * the text and, for the level block, hides the whole block via + * onCleared. */ function closeEditor(message) { const target = getTarget(); const node = target ? target.node : null; - const value = node && typeof node.summary === "string" ? node.summary : ""; - textEl.textContent = value; // text node — the CURRENT stored description + const stored = node && typeof node.summary === "string" ? node.summary : ""; + const pending = node !== null && stored === "" && node.summary_pending; + const value = pending && pendingText ? pendingText : stored; + textEl.className = pending ? "kb-summary-pending" : ""; + if (pending) { + if (pendingTitle) textEl.title = pendingTitle; + } else { + textEl.removeAttribute("title"); + } + textEl.textContent = value; // text node — the CURRENT display state editBtn.hidden = false; status.textContent = message; container.replaceChildren(textEl, editBtn, status); @@ -252,6 +339,7 @@ function wireDescriptionEdit({ editBtn, textEl, container, getTarget, onCleared } const data = await res.json(); node.summary = data.summary; // in-place kbTree update (no re-fetch) + node.summary_pending = false; // phase 98 (D4): a created/updated description is no longer pending — the marker clears in place (no re-fetch) closeEditor(data.summary === null ? "Description cleared." : "Description updated."); } catch { // Network failure: the reachable? copy; the editor stays open. @@ -292,11 +380,27 @@ function wireDescriptionEdit({ editBtn, textEl, container, getTarget, onCleared * miss), so the button is added unconditionally (the view is * admin-only already — the endpoint's require_admin is the API gate). * The shared editor is wired with a CONSTANT target (the row's node). - * `label` is the human name for the button's aria-label. */ + * `label` is the human name for the button's aria-label. + * Phase 98 (task 04, D4): the cell has THREE states — a stored summary + * (the stored text, textContent only), PENDING (no stored summary but + * node.summary_pending — the marker: the kb-summary-pending class + + * the "Summary pending" text + the D4 title; text + color, never color + * alone), and the empty cell (neither — the ls rule, unchanged). The + * Edit button is UNCHANGED in all three (a manual save creates the row + * and clears the marker in place — the editor's success path). */ function makeDescCell(node, source, folder, label) { const td = document.createElement("td"); const text = document.createElement("span"); - text.textContent = (node && node.summary) || ""; // text node — never innerHTML + if (node && node.summary) { + text.textContent = node.summary; // stored description — text node, never innerHTML + } else if (node && node.summary_pending) { + // Phase 98 (D4): the summary is due but not stored yet — the marker. + text.className = "kb-summary-pending"; + text.textContent = "Summary pending"; + text.title = "No stored description yet — the next sync will generate one."; + } else { + text.textContent = ""; // neither stored nor pending — the empty cell (the ls rule) + } const btn = document.createElement("button"); btn.type = "button"; btn.className = "kb-summary-edit"; @@ -308,6 +412,10 @@ function makeDescCell(node, source, folder, label) { textEl: text, container: td, getTarget: () => ({ node, source, folder }), + // Phase 98 (D4): the row's pending display (closeEditor restores + // it on a cancel; a save clears the marker in place). + pendingText: "Summary pending", + pendingTitle: "No stored description yet — the next sync will generate one.", }); return td; } @@ -328,8 +436,16 @@ export async function mount(root) { * status endpoints — GET /api/sync/status + * GET /api/git-sources/upload/status — and applies this decision * tree, in order (startSyncPolling): - * 1. sync running → "Syncing… (n/m)" — bare "Syncing…" until - * the import's first file (clone/pull, A4); + * 1. sync running → the phase-aware label (Phase 98 task 02, D2) + * — the null-phase prelude + "import" → + * "Syncing… (n/m)" — bare "Syncing…" + * until the import's first file (clone/pull, + * A4, byte-identical); "overview" → "Writing KB + * overview…"; "summaries" → "Summarizing + * folders… (n/m)" (the folder + * part omitted while current_summary is null — + * the phase's first poll) — the post-import span + * where the file count sits still; * 2. upload running → BARE "Importing…" — the background upload * RUN (phase 90: unpack + register only, no * scan — its status never carries a file or @@ -346,12 +462,16 @@ export async function mount(root) { * Sources page's error banner, never this page's (A3); * 7. both idle → retry-ready idle. * The live label is the status endpoint's full source/relative/path - * (A4): CSS ellipsizes #sync-label; the full untruncated path also - * rides the button title (hover) and #sync-result (the aria-live + * (A4) — or, in the post-import phases, the phase's own copy (Phase + * 98 task 02): CSS ellipsizes #sync-label; the full untruncated + * label also rides the button title (hover — set for EVERY running + * label, not just file labels) and #sync-result (the aria-live * announcer — screen readers hear it). The load-time re-attach - * (initSyncButton) re-enters a RUNNING upload the same way; a terminal - * upload is a no-op there (the boot-time loadTree() already shows the - * current catalog). + * (initSyncButton) re-enters a RUNNING sync the same way — with + * whatever phase the status reports (a mid-summaries reload shows + * the summaries label — the never-stale contract) — and a RUNNING + * upload the same way; a terminal upload is a no-op there (the + * boot-time loadTree() already shows the current catalog). * * Elements: #sync-btn (the button), #sync-label (the text), * #sync-icon (the spinner icon), #sync-result (aria-live result @@ -397,7 +517,11 @@ export async function mount(root) { * "Importing" label is always the bare one. The counts appear only * once the import has started (total > 0). CSS ellipsizes the button * label; the same untruncated text goes to the button title + - * #sync-result (the aria-live announcer). */ + * #sync-result (the aria-live announcer). Phase 98 (task 02): the + * SYNC job's post-import phases (overview / summaries) take their + * own label via fmtSyncPhaseLabel (below) — this builder keeps that + * prelude/import fall-through byte-identical and serves the UPLOAD + * job unchanged (its status has no phase — D2). */ function fmtSyncLabel(kind, currentFile, done, total) { const prefix = kind === "upload" ? "Importing" : "Syncing…"; let label = currentFile ? `${prefix} ${currentFile}` : prefix; @@ -405,6 +529,29 @@ export async function mount(root) { return label; } + /* Phase 98 (task 02, D2): the SYNC job's phase-aware running label + * — the post-import span where the file count sits still (the + * owner's "the number pauses for a really long time"): "overview" + * → "Writing KB overview…"; "summaries" → "Summarizing folders… + * (n/m)" — the folder part is omitted while + * current_summary is null (the phase's first poll); anything else + * (the null prelude, "import") → the phase-64 fmtSyncLabel + * fall-through, byte-identical (same builder, same fields, same + * order). The UPLOAD job never passes a status (its endpoint has no + * phase — D2) and keeps the bare fmtSyncLabel label. The untruncated + * result rides the button title + #sync-result like every running + * label (A4). */ + function fmtSyncPhaseLabel(status) { + if (status.phase === "overview") return "Writing KB overview…"; + if (status.phase === "summaries") { + const folder = status.current_summary ? ` ${status.current_summary}` : ""; + return `Summarizing folders…${folder} (${status.summaries_done}/${status.summaries_total})`; + } + // null phase (the prelude) or "import" — today's byte-identical + // sync label (the phase-64 contract, unchanged). + return fmtSyncLabel("sync", status.current_file, status.files_done, status.files_total); + } + function fmtSyncResult(detail) { const d = detail || {}; const added = d.added || 0; @@ -425,21 +572,29 @@ export async function mount(root) { /* The single running-state entry point (phase 64 task 04: the label * carries the live file — `kind` "sync" | "upload", the status - * endpoint's current_file + done/total). Same mechanics as before - * (disabled, aria-busy, spinning icon, no is-error) plus: the full - * untruncated path on the button title (removed when null — no file - * yet) and in #sync-result (the aria-live announcer reads the full - * live path; CSS ellipsizes the button's label span only). */ - function enterSyncRunningState(kind, currentFile, done, total) { + * endpoint's current_file + done/total). Phase 98 (task 02, D2): the + * SYNC job passes the status object as `status` — its running label + * is phase-aware (fmtSyncPhaseLabel: "Writing KB overview…" / + * "Summarizing folders… (n/m)"); the UPLOAD job and the + * 202/409 click (no status yet) keep the bare fmtSyncLabel label. + * Same mechanics as before (disabled, aria-busy, spinning icon, no + * is-error) plus: the FULL untruncated label on the button title + * (set for EVERY running label — the phase-64 file-only rule is + * adjusted for the phase labels) and in #sync-result (the aria-live + * announcer reads it; CSS ellipsizes the button's label span only). + */ + function enterSyncRunningState(kind, currentFile, done, total, status) { if (!syncBtn) return; syncBtn.disabled = true; syncBtn.setAttribute("aria-busy", "true"); - if (currentFile) syncBtn.title = currentFile; - else syncBtn.removeAttribute("title"); + const label = + kind === "sync" && status + ? fmtSyncPhaseLabel(status) + : fmtSyncLabel(kind, currentFile, done, total); + syncBtn.title = label; // A4: the untruncated label rides the title, always syncBtn.setAttribute("aria-label", "Sync sources"); syncBtn.classList.remove("is-error"); if (syncIcon) syncIcon.classList.add("is-spinning"); - const label = fmtSyncLabel(kind, currentFile, done, total); if (syncLabel) syncLabel.textContent = label; if (syncResult) syncResult.textContent = label; } @@ -572,11 +727,14 @@ export async function mount(root) { syncPollTimer = setTimeout(tick, SYNC_POLL_MS); return; } - // 1. sync running: the live sync file (bare "Syncing…" until the - // import's first file — A4). + // 1. sync running: the phase-aware label (phase 98 task 02 — the + // null prelude + "import" keep the byte-identical file label, + // "overview" / "summaries" name themselves; the WHOLE status + // goes in for the phase fields). if (syncStatus.state === "running") { enterSyncRunningState( - "sync", syncStatus.current_file, syncStatus.files_done, syncStatus.files_total + "sync", syncStatus.current_file, syncStatus.files_done, syncStatus.files_total, + syncStatus ); syncPollTimer = setTimeout(tick, SYNC_POLL_MS); return; @@ -668,7 +826,11 @@ export async function mount(root) { * + register — the bare "Importing…" label) adopts the button the * same way — the "user clicked upload, then opened sources" case; a * terminal upload is a no-op (the boot-time loadTree() already shows - * the current catalog). */ + * the current catalog). Phase 98 (task 02): a RUNNING sync re-enters + * with the WHOLE status — the label is phase-aware, so a mid- + * summaries reload re-enters with the summaries label (the + * never-stale contract); a RUNNING upload re-attach stays on the + * bare label (its status has no phase — D2). */ async function initSyncButton() { if (!syncBtn) return; if (!(await fetchIsAdmin())) return; @@ -681,7 +843,8 @@ export async function mount(root) { } catch { return; } if (status.state === "running") { enterSyncRunningState( - "sync", status.current_file, status.files_done, status.files_total + "sync", status.current_file, status.files_done, status.files_total, + status ); emitSyncStatus(status); startSyncPolling(); @@ -1040,14 +1203,24 @@ export async function mount(root) { } // The level block: the current level's STORED description (source: - // the (source, "") row; folder: its row) — hidden when none is - // stored (the ls rule: count only, no placeholder). Title = the - // full source-relative path (e.g. `alpha/two`). - if (node.summary) { + // the (source, "") row; folder: its row) — OR, since Phase 98 + // (task 04, D4), the PENDING note when the level is summary_pending + // (the summary is due but not stored yet — the next sync's gap-fill + // will generate it, or the owner can write one now via the block's + // Edit button). Hidden only when NEITHER is stored nor pending + // (the ls rule: count only, no placeholder). Title = the full + // source-relative path (e.g. `alpha/two`). + if (node.summary || node.summary_pending) { levelTitleEl.textContent = current.folder ? current.source + "/" + current.folder : current.source; - levelSummaryEl.textContent = node.summary; + levelSummaryEl.textContent = node.summary || + "No description stored yet — the next sync will generate one. (You can write one yourself.)"; + // Phase 98 (D4): the pending text is the muted marker style — + // and the block is REUSED across levels, so a stored level must + // clear a previous pending level's class (the same state + // closeEditor manages on the editor's close). + levelSummaryEl.className = node.summary ? "" : "kb-summary-pending"; levelEl.hidden = false; } else { levelEl.hidden = true; @@ -1144,6 +1317,10 @@ export async function mount(root) { const node = currentLevelNode(); return node ? { node, source: current.source, folder: current.folder } : null; }, + // Phase 98 (D4): the level's pending display (the D4 note — no + // tooltip on the level's

: D4's title is the row cell's). + pendingText: + "No description stored yet — the next sync will generate one. (You can write one yourself.)", onCleared: () => { // The phase-57 announcement beat: keep the block visible a short // beat so the role=status "Description cleared." is still diff --git a/frontend/assets/styles.css b/frontend/assets/styles.css index de1e0c6..bf42db5 100644 --- a/frontend/assets/styles.css +++ b/frontend/assets/styles.css @@ -2084,6 +2084,13 @@ details.thinking .thinking-text ul { margin: 0 0 0.5rem; } color: var(--ink-soft); /* 5.1:1 on --surface (AA) */ } .kb-summary-status:empty { margin-top: 0; } +/* Phase 98 (task 04, D4): the row-cell "Summary pending" marker — the + muted ink-soft pair (5.1:1 on --surface, AA); text + color, never + color alone. The class sits on the EXISTING description text span, + so the row cell's font-size/line-height apply — the marker must not + change row height (no font/white-space overrides). No italic, no + new hue (the phase-92 monochrome invariant). */ +.kb-summary-pending { color: var(--ink-soft); } /* ---------- Git sources page (phase 35) ---------- /git-sources.html: the admin-only manager for the stored git source diff --git a/tests/e2e/test_sync_summary_visibility.py b/tests/e2e/test_sync_summary_visibility.py new file mode 100644 index 0000000..f8e0ccb --- /dev/null +++ b/tests/e2e/test_sync_summary_visibility.py @@ -0,0 +1,958 @@ +"""Phase 98 task 05 E2E (Playwright, mock-only): the sync's summary +phases are visible live — endpoint + button label — and missing folder +summaries read "waiting to generate" (the "Summary pending" markers) +until a manual save or the next sync's gap-fill makes them go away. + +The dedicated story suite for ``98_sync_summary_visibility`` (A16 — one +Playwright file per phase, run in isolation): the owner's report was +that while a sync runs "the number pauses for a really long time" +(after the import, the KB-overview + the per-folder summary span burn +minutes of ``lite`` calls with nothing on the wire) and that a folder +whose summary is due but missing looks "missed" in the catalog. + +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_sync_summary_visibility.py -v --no-cov + +MOCK-ONLY suite: ``E2E_REAL_LLM=1`` is not supported — the gate is the +deterministic mock (the phase-94 ``FOLDER_SUMMARY_MODE`` canned +``Fixture folder summary for .`` lines) plus the phase-64 +``tests/e2e/slow_llm.py`` delay proxy that stretches the run past both +the ~100 ms recorder cadence and the UI's 2 s poll. + +Timing fixture (the phase-64 sizing rationale, re-sized for the phase +labels): ``SLOW_DELAY_S = 1.5`` — ONE LLM call outlives the recorder's +~100 ms cadence by ~15×, so every phase span (the overview's one call +≈ 1.5 s, the summaries' three ≈ 4.5 s) is densely sampled by the +recorder AND outlives the UI's 2 s status poll (any window ≥ the poll +period catches a tick — the rendered-label assertions are +deterministic, not races). The whole changed sync is ≈ 10 LLM calls +(2 model probes + 4 file embeds + 1 overview + 3 folder summaries) +≈ 15–18 s — well inside the generous settle budgets. + +The suite runs TWO module apps (the phase-79 full-inventory pattern — +each binds its own port) over the shared E2E Postgres: + +* the SLOW app — ``BOR_LLM_BASE_URL`` = the slow proxy in front of the + mock: tests 1–2 (the phase machine at the endpoint + the phase-aware + button label); +* the FAST app — ``BOR_LLM_BASE_URL`` = the mock directly (it stays on + its port for the fast tests): test 3 (the pending markers + the + gap-fill recovery — the canned summaries are byte-stable). + +KB fixture — a host temp dir (``tmp_path_factory``; the app runs on +the same host) registered as a ``kind=local`` source (the +``test_local_directory_sources.py`` / phase-94 API-registration +pattern; no git anywhere): ``syncsum/`` with TWO ≥ 2-doc folders — +``alpha/`` (2 docs) and ``bravo/`` (2 docs), no root-level files. The +generator's candidate set is therefore exactly 3: the source root +(``folder_path ""``) + the two folders (the recursive-subtree ≥ 2 rule). + +Autouse cleanup (the phase-96 pattern): before each test wait for no +running sync on EITHER app, then truncate the shared registry + KB +tables (documents/chunks/query_log/steering_notes/kb_overview/ +git_sources/folder_summaries) — each test seeds its own source and +runs its own sync; after each test the same truncate. + +Test → observable mapping (Playwright Mapping Rule): +1. ``test_sync_status_reports_the_summary_phases`` (endpoint — the + slow leg, the phase-64 tight-poll recorder): some running tick has + ``phase "import"`` (with the live ``current_file`` + counts the + import always carried), some have ``phase "overview"``, some have + ``phase "summaries"`` with ``summaries_total == 3``, non-null + ``current_summary`` (starting with the source name — one tick the + BARE source name, the source-root call) and ``summaries_done`` + climbing 1 → 2 → 3; every summaries tick keeps + ``files_done == files_total`` (the import is finished — the + reported pause is now labeled); the terminal has ``phase``/ + ``current_summary`` null but keeps ``summaries_done == + summaries_total == 3`` (the keep-final-counts convention). +2. ``test_sync_button_names_the_summary_phase`` (UI — the slow leg): + a fresh admin page on ``/sources.html`` clicks **Sync sources**; + the page's OWN 2 s poll renders ``#sync-label`` matching + ``Summarizing folders… [folder] (n/3)`` (the asserted text is the + RENDERED DOM label, with the button ``title`` riding the same + untruncated value — A4); on settle the button reads the terminal + ``Synced HH:MM`` (the phase-32 contract) and ``#sync-result`` + carries the counts line. +3. ``test_missing_folder_summaries_read_as_pending_and_self_heal`` + (tree — the fast mock leg): after a successful sync (every + candidate stores its canned line) NO marker is anywhere; DELETE + one folder's row AND the source-root row directly (the phase-96 + row-deletion pattern) + re-fetch (the phase-77 re-show refresh) → + the two affected rows show ``Summary pending`` (+ the D4 title, + the D4 class, the always-present Edit) while the intact folder + shows its stored line and NO marker; the source level AND the + affected folder's level block show the D4 pending note; saving a + manual description from the row's Edit clears the marker IN PLACE + (no reload); a second UNCHANGED sync's gap-fill regenerates the + OTHER deleted row (the source root) — its marker is gone, its + cell/level carry the deterministic mock line, and the manual row + is untouched (``only_missing`` + ``manually_edited``). +""" +from __future__ import annotations + +import json +import os +import re +import subprocess +import sys +import threading +import time +from collections.abc import Iterator +from datetime import datetime +from pathlib import Path +from typing import Any + +import httpx +import pytest +from playwright.sync_api import Page, expect +from sqlalchemy import select, text + +from app.config import Settings as _Settings +from app.db import SessionLocal +from app.models import FolderSummary +from e2e.auth_helpers import login +from e2e.conftest import ( + ADMIN_PASSWORD, + MOCK_PORT, + SESSION_SECRET, + USE_REAL_LLM, + _wait_http, +) + +REPO = Path(__file__).resolve().parents[2] + +# Phase 79 (task 04, full inventory): the conftest session app owns its +# port in a combined run — this module's apps bind their own ports +# instead (a same-port second uvicorn dies on bind and would drive the +# wrong server). Env-overridable. +SLOW_APP_PORT = int(os.environ.get("E2E_APP_PORT_SYNCSUM_SLOW", "8142")) +FAST_APP_PORT = int(os.environ.get("E2E_APP_PORT_SYNCSUM_FAST", "8143")) +SLOW_APP_URL = f"http://127.0.0.1:{SLOW_APP_PORT}" +FAST_APP_URL = f"http://127.0.0.1:{FAST_APP_PORT}" + +#: The slow-LLM proxy's port (the conftest's mock LLM stays on +#: MOCK_PORT — the FAST app points at it directly for test 3). +SLOW_PORT = int(os.environ.get("E2E_SLOW_LLM_PORT_SYNCSUM", "8904")) +SLOW_URL = f"http://127.0.0.1:{SLOW_PORT}" + +#: Per-LLM-request delay on the proxy — sized so ONE LLM call outlives +#: the recorder's ~100 ms cadence by ~15× (the phase-64 rationale): +#: the overview span (one call ≈ 1.5 s) and the summaries span (three +#: calls ≈ 4.5 s) are densely sampled by the recorder AND outlive the +#: UI's 2 s poll period (any window ≥ the period catches a tick — the +#: rendered-label assertion is deterministic). A changed sync is +#: ≈ 10 calls ≈ 15–18 s (see the module docstring). +SLOW_DELAY_S = "1.5" + +# -------------------------------------------------------------------------- +# Fixture constants (deterministic, token-controlled) +# -------------------------------------------------------------------------- + +#: The local source — the temp directory's basename (``kind=local`` → +#: the directory's basename is the source name, phase 38). +SOURCE = "syncsum" + +#: The two ≥ 2-doc folders (path order — the tree rows' order). +FOLDER_A = "alpha" +FOLDER_B = "bravo" +FOLDERS = (FOLDER_A, FOLDER_B) + +N_FILES = 4 # two folders × 2 docs +N_CANDIDATES = 3 # the source root + the two folders (the ≥ 2-doc rule) + +#: The mock's byte-stable ``FOLDER_SUMMARY_MODE`` lines for this fixture +#: (the phase-94 template — the label is the ``FOLDER_HEADER_PREFIX`` +#: tail: ```` for the root, ``/`` for a folder). +SUM_ROOT = f"Fixture folder summary for {SOURCE}." +SUM_ALPHA = f"Fixture folder summary for {SOURCE}/{FOLDER_A}." +SUM_BRavo = f"Fixture folder summary for {SOURCE}/{FOLDER_B}." + +#: The D4 "Summary pending" marker copy (phase 98 task 04 — verbatim +#: from the D4 decision): the row-cell text, the row-cell ``title``, +#: and the level-block pending note. +PENDING_COPY = "Summary pending" +PENDING_TITLE = "No stored description yet — the next sync will generate one." +PENDING_NOTE = ( + "No description stored yet — the next sync will generate one. " + "(You can write one yourself.)" +) + +#: The hand-written description test 3 saves on the affected folder — +#: a distinctive sentence no part of the fixture or the canned +#: template contains (the round-trip assertions can never pass against +#: the old text, the phase-97 pattern). +MANUAL_BRavo = ( + "Owner note: bravo holds the bravo-a and bravo-b fixture files — " + "written by hand after the row was deleted. (RESE-SYNCSUM-01)" +) + +#: "Synced HH:MM" — the local-time last-result label (sources.js's +#: fmtSyncTime), any hour/minute (the test_sync_button.py pattern). +SYNCED_LABEL = re.compile(r"Synced \d{1,2}:\d{2}$") + +#: Generous settle budgets: a changed sync against the slowed LLM is +#: ≈ 15–18 s; the UI's 2 s poll settles at most one tick after the +#: terminal state lands. +SETTLE_TIMEOUT_MS = 90_000 + + +def _md(title: str, body: str) -> str: + return f"# {title}\n\n{body}\n" + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="module") +def syncsum_dir(tmp_path_factory: pytest.TempPathFactory) -> Path: + """The one-source temp tree (see the module docstring): the app + servers run on the same host, so the paths are visible to them. + The directory NAME is the source name (``kind=local``, phase 38).""" + root = tmp_path_factory.mktemp("bor_synccom") + src = root / SOURCE + for folder in FOLDERS: + (src / folder).mkdir(parents=True) + for letter in ("a", "b"): + (src / folder / f"{folder}-{letter}.md").write_text( + _md( + f"Synccom {folder.title()} {letter.upper()}", + f"Synccom {folder} fixture note {letter.upper()}: covers " + f"topic {letter.upper()} of the {SOURCE} source tree.", + ), + encoding="utf-8", + ) + assert len(list(src.rglob("*.md"))) == N_FILES + return src + + +def _app_env(base_url: str, tmp_path_factory: pytest.TempPathFactory) -> dict[str, str]: + """The per-module app env (the conftest pattern, cf. + ``test_oneshot_llm_retry.py`` / ``test_kb_tree.py``): the + ``base_url`` picks the LLM (slow proxy vs. the mock directly), + ``BOR_GIT_SOURCES`` forced empty (the sync source is this suite's + own DB-registered local row), and the leak-guarded code defaults.""" + env = dict(os.environ) + env.pop("DEBUGPY", None) + env["BOR_ENVIRONMENT"] = "e2e" + env["BOR_STATIC_DIR"] = str(REPO / "frontend") + env["BOR_LLM_BASE_URL"] = ( + "https://aipi.reeseapps.com/v1" if USE_REAL_LLM else base_url + ) + # Mock-calibrated threshold (conftest pattern): this suite never + # asks the chat model anything — the gate is never on a path. + env["BOR_RELEVANCE_THRESHOLD"] = "0.30" + # Phase 67: instant retry waits + the code-default budget (the + # conftest leak-guard pattern). + env["BOR_LLM_RETRY_DELAY"] = "0" + env["BOR_LLM_RETRIES"] = str(_Settings.model_fields["llm_retries"].default) + env.setdefault( + "BOR_DATABASE_URL", + "postgresql+psycopg://reese:reese@localhost:5432/brain_of_reese", + ) + # Phase 16: admin auth must be set or create_app() refuses to boot. + env["BOR_ADMIN_PASSWORD"] = ADMIN_PASSWORD + env["BOR_SESSION_SECRET"] = SESSION_SECRET + # The repo's .env file carries the owner's BOR_GIT_SOURCES (the app + # reads it from cwd) — override it with an EMPTY value (the env var + # beats the .env file): the registry must hold EXACTLY the local + # directory this suite registers. + env["BOR_GIT_SOURCES"] = "" + # Leak guards (conftest pattern): an operator's local (gitignored) + # .env cannot leak corpus-specific settings into the app under test. + env["BOR_DOCS_REPO"] = "" + env["BOR_SUGGESTIONS"] = json.dumps( + _Settings.model_fields["suggestions"].default + ) + env["BOR_INPUT_PLACEHOLDER"] = _Settings.model_fields["input_placeholder"].default + env["BOR_FOOTER_TEXT"] = _Settings.model_fields["footer_text"].default + env["BOR_SOURCES_DIR"] = str(tmp_path_factory.mktemp("bor_checkouts")) + return env + + +@pytest.fixture(scope="module") +def slow_llm(mock_llm: int) -> Iterator[int]: + """The delay-injecting reverse proxy in front of the mock LLM + (``tests/e2e/slow_llm.py``) — the timing fixture for tests 1–2: + the phase-machine ticks and the rendered phase label need the run + to outlive both the ~100 ms recorder cadence and the UI's 2 s + poll (see ``SLOW_DELAY_S``).""" + env = dict(os.environ) + env.pop("DEBUGPY", None) + env["SLOW_LLM_DELAY_S"] = SLOW_DELAY_S + env["E2E_MOCK_PORT"] = str(MOCK_PORT) + proc = subprocess.Popen( + [sys.executable, "-m", "uvicorn", "tests.e2e.slow_llm:app", + "--host", "127.0.0.1", "--port", str(SLOW_PORT), "--log-level", "warning"], + cwd=REPO, + env=env, + ) + try: + _wait_http(f"{SLOW_URL}/v1/models") + yield SLOW_PORT + finally: + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + + +@pytest.fixture(scope="module") +def app_server( + mock_llm: int, + slow_llm: int, + syncsum_dir: Path, + tmp_path_factory: pytest.TempPathFactory, +) -> Iterator[str]: + """The SLOW app under test (tests 1–2): ``BOR_LLM_BASE_URL`` is the + slow proxy in front of the mock (the timing fixture — see the + module docstring). ``syncsum_dir`` is a dependency only for the + fixture ordering (the temp tree exists before the first test).""" + env = _app_env(f"{SLOW_URL}/v1", tmp_path_factory) + proc = subprocess.Popen( + [sys.executable, "-m", "uvicorn", "app.main:app", + "--host", "127.0.0.1", "--port", str(SLOW_APP_PORT), "--log-level", "warning"], + cwd=REPO, + env=env, + ) + try: + _wait_http(f"{SLOW_APP_URL}/api/health") + yield SLOW_APP_URL + finally: + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + + +@pytest.fixture(scope="module") +def app_url(app_server: str) -> str: + return app_server + + +@pytest.fixture(scope="module") +def fast_app_server( + mock_llm: int, + syncsum_dir: Path, + tmp_path_factory: pytest.TempPathFactory, +) -> Iterator[str]: + """The FAST app under test (test 3): ``BOR_LLM_BASE_URL`` is the + mock DIRECTLY (it stays on its port for the fast tests — the + canned ``FOLDER_SUMMARY_MODE`` lines are byte-stable there, and + the pending-marker / gap-fill assertions key on that exact text).""" + env = _app_env(f"http://127.0.0.1:{mock_llm}/v1", tmp_path_factory) + proc = subprocess.Popen( + [sys.executable, "-m", "uvicorn", "app.main:app", + "--host", "127.0.0.1", "--port", str(FAST_APP_PORT), "--log-level", "warning"], + cwd=REPO, + env=env, + ) + try: + _wait_http(f"{FAST_APP_URL}/api/health") + yield FAST_APP_URL + finally: + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + + +@pytest.fixture(scope="module") +def fast_app_url(fast_app_server: str) -> str: + return fast_app_server + + +def _truncate_all() -> None: + """Fresh registry + KB (the E2E isolation pattern): the E2E suites + share one Postgres, so a leftover git_sources row or document would + pollute the top-level rows, the candidate count, and the counts + the syncs and the tree assertions pin exactly.""" + with SessionLocal() as db: + db.execute( + text( + "TRUNCATE chunks, documents, query_log, steering_notes, " + "kb_overview, git_sources, folder_summaries" + ) + ) + db.commit() # without the commit the TRUNCATE rolls back (the house pattern) + + +def _wait_sync_settled(base_url: str, timeout_s: float = 180.0) -> None: + """No background sync may leak across tests (the run state lives in + the app's memory, and a still-running run would keep importing into + the NEXT test's truncated KB): wait for the app's sync status to + be non-running BEFORE the truncate. Own admin session (the + endpoint is admin-only) — usually a no-op: the tests settle only + after their run's terminal state.""" + with httpx.Client(base_url=base_url, timeout=5.0) as client: + r = client.post("/api/login", json={"password": ADMIN_PASSWORD}) + assert r.status_code == 204, r.text + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + r = client.get("/api/sync/status") + assert r.status_code == 200, r.text + if r.json()["state"] != "running": + return + time.sleep(0.2) + raise AssertionError(f"a sync was still running at a test boundary: {base_url}") + + +@pytest.fixture(autouse=True) +def _clean(app_url: str, fast_app_url: str, db_ready: None) -> Iterator[None]: + """Per-test KB isolation (the phase-96 cleanup pattern): the + shared E2E Postgres is truncated before AND after every test — + each test seeds its own source and runs its own sync(s), so the + KB rows + folder_summaries rows of the temp source are exactly + this test's own. The pre-wait keeps a leaked run from the + previous test (module apps are shared) from racing the truncate.""" + for base in (app_url, fast_app_url): + _wait_sync_settled(base) + _truncate_all() + yield + _truncate_all() + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _register_source(base_url: str, path: str) -> None: + """Register the temp directory through the authenticated API (the + ``test_local_directory_sources.py`` / phase-94 pattern — a plain + directory is an API/DB-only operation).""" + with httpx.Client(base_url=base_url, timeout=30.0) as client: + r = client.post("/api/login", json={"password": ADMIN_PASSWORD}) + assert r.status_code == 204, r.text + r = client.post( + "/api/git-sources", json={"kind": "local", "path": path} + ) + assert r.status_code == 201, r.text + + +def _run_sync(base_url: str, timeout_s: float = 120.0) -> dict[str, Any]: + """Login + ``POST /api/sync`` + poll the status endpoint until the + run reaches a terminal state (the ``test_local_directory_sources`` + pattern, over plain httpx).""" + with httpx.Client(base_url=base_url, timeout=30.0) as client: + r = client.post("/api/login", json={"password": ADMIN_PASSWORD}) + assert r.status_code == 204, r.text + r = client.post("/api/sync") + assert r.status_code == 202, r.text + deadline = time.monotonic() + timeout_s + body: dict[str, Any] = {} + while time.monotonic() < deadline: + r = client.get("/api/sync/status") + assert r.status_code == 200, r.text + body = r.json() + if body["state"] in ("success", "failed"): + return body + time.sleep(0.5) + raise AssertionError(f"sync did not reach a terminal state: {body}") + + +def _folder_rows() -> dict[str, tuple[str, bool]]: + """The source's stored folder summaries + ``{folder_path: (summary, manually_edited)}`` (``""`` = the source + root) — the test process's direct DB access (the E2E's other + established lens).""" + with SessionLocal() as db: + rows = db.execute( + select( + FolderSummary.folder_path, + FolderSummary.summary, + FolderSummary.manually_edited, + ).where(FolderSummary.source == SOURCE) + ).all() + return { + folder: (summary, manually) + for folder, summary, manually in rows + } + + +def _delete_folder_rows(pairs: list[tuple[str, str]]) -> None: + """DELETE stored ``folder_summaries`` rows directly (the phase-96 + row-deletion pattern — simulating a historical fail-soft miss): + the rows are gone from the app's back without any KB change.""" + with SessionLocal() as db: + for source, folder in pairs: + db.execute( + text( + "DELETE FROM folder_summaries " + "WHERE source = :s AND folder_path = :f" + ), + {"s": source, "f": folder}, + ) + db.commit() + + +class _TickRecorder: + """The deterministic layer, concurrent with the test's own client. + + A daemon thread that tight-polls (~100 ms cadence) + ``GET /api/sync/status`` with its OWN admin session (``httpx`` — + the test's client drives itself in the meantime; the Playwright + sync API is not thread-safe, so the thread never touches it), + recording every tick from the first poll: the idle prelude, the + running ticks, and the terminal body. The thread only READS the + same endpoint the UI's 2 s poll reads — it starts no jobs and + cannot skew the run. (The phase-64 recorder pattern.) + + The stale-terminal guard (the module apps are shared — a previous + test's terminal lives in the app's memory): a terminal is + accepted only after a RUNNING tick was observed (this suite's runs + outlive the 100 ms cadence by ~15× — ``require_running`` is + always on here).""" + + def __init__(self, app_url: str, path: str) -> None: + self._url = f"{app_url}{path}" + self._login_url = f"{app_url}/api/login" + self._t0 = time.time() + self._ticks: list[dict[str, Any]] = [] + self._terminal: dict[str, Any] | None = None + self._stop = threading.Event() + self._thread: threading.Thread | None = None + + def _terminal_is_this_run(self, body: dict[str, Any], saw_running: bool) -> bool: + if saw_running: + return True + started = body.get("started_at") + if not started: + return False + try: + started_at = datetime.fromisoformat(str(started)).timestamp() + except ValueError: + return False + return started_at >= self._t0 - 2.0 # tolerance for the pre-submit gap + + def start(self) -> None: + def run() -> None: + with httpx.Client(timeout=5.0) as client: + # Its own admin session — the recorder's reads must not + # depend on (or disturb) the test's client's cookie. + client.post(self._login_url, json={"password": ADMIN_PASSWORD}) + saw_running = False + while not self._stop.is_set(): + try: + r = client.get(self._url) + if r.status_code == 200: + body = r.json() + self._ticks.append(body) + if body["state"] == "running": + saw_running = True + elif ( + body["state"] in ("success", "failed") + and self._terminal_is_this_run(body, saw_running) + ): + self._terminal = body + return + except Exception: # noqa: BLE001 — blip: retry next tick + pass + self._stop.wait(0.1) + + self._thread = threading.Thread(target=run, daemon=True) + self._thread.start() + + def stop(self, timeout_s: float = 120.0) -> dict[str, Any]: + """Join until a terminal tick is recorded (or fail the test).""" + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + if self._terminal is not None: + break + time.sleep(0.05) + self._stop.set() + if self._thread is not None: + self._thread.join(timeout=5) + assert self._terminal is not None, ( + "the recorder saw no terminal state " + f"(last ticks: {self._ticks[-3:] if self._ticks else 'none'})" + ) + return self._terminal + + @property + def running_ticks(self) -> list[dict[str, Any]]: + return [t for t in self._ticks if t["state"] == "running"] + + +def _wait_top_level(page: Page) -> None: + """The admin boot / re-fetch has rendered the top level: the single + source row is in the folders table (the tree's fetch settled) and + the breadcrumb is hidden (the top level is home).""" + expect(page.locator("#folders-tbody tr")).to_have_count(1, timeout=30_000) + expect(page.locator("#kb-crumb")).to_be_hidden() + + +def _drill(page: Page, *names: str) -> None: + """Drill one level at a time (client-side — no fetch, no URL + change): each name is the EXACT text of the source/folder link at + the current level (the row builders' link text: the source name, + or the folder's last path segment — the phase-97 pattern).""" + for name in names: + page.click(f'#folders-tbody a.folder-link:text-is("{name}")') + + +# --------------------------------------------------------------------------- +# 1. Endpoint — the phase machine: import keeps the file label, the +# overview is named, the folder span reports the folder + a +# done/total that climbs to the candidate count +# --------------------------------------------------------------------------- + + +def test_sync_status_reports_the_summary_phases( + app_url: str, syncsum_dir: Path, db_ready: None +) -> None: + """A KB-changing sync of the temp source (candidates: the source + root + the two folders = 3) is tight-polled by the ~100 ms + recorder from the 202 to the terminal (the phase-64 pattern): the + recorded ticks show the D1 phase machine end to end — the + model-check prelude keeps ``phase`` null, the import keeps its + byte-identical live-file fields with ``phase "import"``, the + overview span is named, and the folder span carries + ``current_summary`` + ``summaries_done``/``summaries_total`` + (climbing 1 → 2 → 3) with ``files_done == files_total`` (the + import is finished — the pause the user reported) — while the + terminal clears ``phase``/``current_summary`` but KEEPS the final + summary counts (the phase-64 keep-final-counts convention).""" + _register_source(app_url, str(syncsum_dir)) + + recorder = _TickRecorder(app_url, "/api/sync/status") + recorder.start() + with httpx.Client(base_url=app_url, timeout=30.0) as client: + r = client.post("/api/login", json={"password": ADMIN_PASSWORD}) + assert r.status_code == 204, r.text + r = client.post("/api/sync") + assert r.status_code == 202, r.text + terminal = recorder.stop(timeout_s=120) + + # --- the terminal (D1: clear the phase + folder, keep the final + # summary counts; the phase-64 keep-final-counts convention extends + # to the file counts) --- + assert terminal["state"] == "success", terminal + detail = terminal["detail"] + assert detail["added"] == N_FILES, detail + assert terminal["current_file"] is None + assert terminal["files_done"] == N_FILES + assert terminal["files_total"] == N_FILES + assert terminal["phase"] is None, terminal + assert terminal["current_summary"] is None, terminal + assert terminal["summaries_done"] == N_CANDIDATES, terminal + assert terminal["summaries_total"] == N_CANDIDATES, terminal + + running = recorder.running_ticks + assert running, "the recorder caught no running tick" + + # --- the prelude (model check): ``phase`` null — the bare + # "Syncing…" label stays (the phase-64 pins hold). Two probes × + # 1.5 s under the slow proxy: the recorder cannot miss it. --- + prelude = [t for t in running if t["phase"] is None] + assert prelude, f"no phase-less prelude tick: {running[:6]}" + assert all(t["current_file"] is None for t in prelude), prelude + + # --- the import phase: the byte-identical file label + counts + # (phase 64), now carrying ``phase "import"`` (D1) --- + imp = [t for t in running if t["phase"] == "import"] + assert imp, f"no import-phase tick: {[t['phase'] for t in running]}" + with_file = [t for t in imp if t["current_file"]] + assert with_file, f"no import tick carried a current_file: {imp[:6]}" + assert all( + t["current_file"].startswith(f"{SOURCE}/") for t in with_file + ), with_file + assert all(t["files_total"] == N_FILES for t in with_file), with_file + dones = [t["files_done"] for t in with_file] + assert dones == sorted(dones), f"files_done not monotonic: {dones}" + assert dones[0] >= 1 and max(dones) == N_FILES, dones + + # --- the overview phase: named (D1) --- + ov = [t for t in running if t["phase"] == "overview"] + assert ov, f"no overview-phase tick: {[t['phase'] for t in running]}" + + # --- the summaries phase (D1/D5): the folder being summarized + + # the done/total through the long span --- + sm = [t for t in running if t["phase"] == "summaries"] + assert sm, f"no summaries-phase tick: {[t['phase'] for t in running]}" + # EVERY summaries tick: the import is finished — the file count + # sits still (the reported pause is now labeled). + for t in sm: + assert t["files_done"] == t["files_total"] == N_FILES, t + # The steady ticks (after the generator's loop starts): total is + # the candidate count and the folder part is non-null, starting + # with the source name (D1's shape). + steady = [t for t in sm if t["summaries_total"] == N_CANDIDATES] + assert steady, f"no steady summaries tick (total {N_CANDIDATES}): {sm[:4]}" + for t in steady: + assert t["current_summary"], t + assert t["current_summary"].startswith(SOURCE), t + # The per-done positions: the sorted candidate order is + # (root, alpha, bravo) — the bare source name is the ROOT call, + # the folders ride ``source/folder``. Under the 15× sizing each + # position is held ≈ 1.5 s, so the recorded progression is exactly + # 1 → 2 → 3 (monotonic, every value present, never revisited). + by_done: dict[int, list[str]] = {} + for t in steady: + by_done.setdefault(t["summaries_done"], []).append(t["current_summary"]) + assert sorted(by_done) == [1, 2, 3], f"summaries_done progression: {sorted(by_done)}" + assert set(by_done[1]) == {SOURCE}, by_done[1] # the bare source name — the root + assert set(by_done[2]) == {f"{SOURCE}/{FOLDER_A}"}, by_done[2] + assert set(by_done[3]) == {f"{SOURCE}/{FOLDER_B}"}, by_done[3] + + # --- the phase machine's ORDER: null → import → overview → + # summaries (summaries is the last phase — the run goes straight + # to the bump/terminal after it) --- + phased = [t for t in running if t["phase"] is not None] + assert phased[0]["phase"] == "import", phased[0] + first_sm = next( + i for i, t in enumerate(phased) if t["phase"] == "summaries" + ) + assert any(t["phase"] == "overview" for t in phased[:first_sm]) + assert all(t["phase"] == "summaries" for t in phased[first_sm:]) + + +# --------------------------------------------------------------------------- +# 2. UI — the sync button names the phase: the page's own 2 s poll +# renders the summaries label (folder + counts), then the phase-32 +# terminal settle +# --------------------------------------------------------------------------- + + +def test_sync_button_names_the_summary_phase( + page: Page, app_url: str, syncsum_dir: Path, db_ready: None +) -> None: + """A fresh admin page on ``/sources.html`` clicks **Sync sources**; + the page's OWN 2 s poll renders ``#sync-label`` as + ``Summarizing folders… [source/folder] (n/3)`` (asserted on the + RENDERED text — the label is built by the page JS — with the + button ``title`` carrying the same untruncated value, A4); on + settle the button shows the terminal ``Synced HH:MM`` (the + phase-32 contract) and ``#sync-result`` carries the counts line. + The prelude/import labels stay the phase-64 shape (the endpoint + test pins those phases; the UI pins the new summaries label).""" + page.set_default_timeout(30_000) + _register_source(app_url, str(syncsum_dir)) + login(page, app_url) # lands on /sources.html (the button's home) + + btn = page.locator("#sync-btn") + expect(btn).to_be_visible(timeout=30_000) + # The button is settled and retry-ready at boot: idle ("Sync + # sources") — or the RE-ATTACHED terminal from the previous + # test's sync (the module app is shared, its in-memory sync state + # survives the test boundary, and the phase-32 boot re-attach + # renders the last result). Waiting on the settled label also + # proves the boot re-attach COMPLETED (no later re-render can + # clobber the running state) — the phase-64 pattern. + expect(page.locator("#sync-label")).to_have_text( + re.compile(r"^(Sync sources|Synced \d{1,2}:\d{2})$") + ) + expect(btn).to_be_enabled() + + btn.click() + + # The click's immediate state (the 202 moment — the bare prefix, + # the phase-64 contract): disabled, aria-busy, spinning icon, no + # error… + expect(btn).to_be_disabled() + expect(btn).to_have_attribute("aria-busy", "true") + expect(btn.locator(".sync-icon")).to_have_class(re.compile(r"\bis-spinning\b")) + expect(page.locator("#sync-label")).to_have_text("Syncing…") + expect(page.locator("#sync-error-banner")).to_be_hidden() + + # UI layer (D2): the page's own 2 s poll names the summaries phase + # — the RENDERED label matches the D2 shape (the folder part is + # optional — the phase's first poll may precede the first hook) + # with the (n/3) counts, and the button ``title`` carries the same + # untruncated value (A4). ``wait_for_function`` checks BOTH in ONE + # in-browser evaluation: a poll tick updates label + title together + # in a single JS task, so the atomic check can never straddle one + # (the JS regex is the D2 shape, unit-pinned in + # tests/unit/test_frontend_sync_upload.py). + page.wait_for_function( + """() => { + const label = document.querySelector('#sync-label'); + const btn = document.querySelector('#sync-btn'); + if (!label || !btn) return false; + const text = label.textContent; + const re = /^Summarizing folders…(?: \\S+)? \\(\\d+\\/3\\)$/; + return re.test(text) && btn.getAttribute('title') === text; + }""", + timeout=SETTLE_TIMEOUT_MS, + ) + + # The success settle (the phase-32 contract, preserved): "Synced + # HH:MM" + the counts result line, the button re-enabled, no + # error — the run's whole span (prelude + import + overview + + # summaries) was labeled along the way (test 1 pins the rest). + expect(page.locator("#sync-label")).to_have_text( + SYNCED_LABEL, timeout=60_000 + ) + expect(btn).to_be_enabled() + expect(btn).not_to_have_attribute("aria-busy") + expect(page.locator("#sync-result")).to_have_text(f"{N_FILES} added") + + +# --------------------------------------------------------------------------- +# 3. Tree — the pending markers: missing rows read "waiting", a manual +# save clears the marker in place, and the next unchanged sync's +# gap-fill heals the other deleted row +# --------------------------------------------------------------------------- + + +def test_missing_folder_summaries_read_as_pending_and_self_heal( + page: Page, fast_app_url: str, syncsum_dir: Path, db_ready: None +) -> None: + """After a successful (fast mock) sync — every candidate stores + its deterministic canned line — NO ``Summary pending`` is + anywhere. DELETE one folder's row AND the source-root row directly + (the phase-96 pattern) + re-fetch (the phase-77 re-show refresh) → + the two affected rows show ``Summary pending`` (+ the D4 title, + the D4 class, the always-present Edit) while the intact folder + shows its stored line and NO marker; the source level AND the + affected folder's level block show the D4 pending note. Saving a + manual description from the row's Edit clears the marker IN PLACE + (the cell shows the saved text — no reload). A second UNCHANGED + sync's gap-fill then regenerates the OTHER deleted row (the + source root): its marker is gone, its cell/level carry the + deterministic mock line — and the manual row is untouched + (``only_missing`` + ``manually_edited``).""" + page.set_default_timeout(30_000) + login(page, fast_app_url) # lands on /sources.html (the RAG view) + + # The changed sync under the fast mock: every candidate (root + + # the two folders) stores its canned line; the terminal keeps the + # full-regeneration summary counts. + _register_source(fast_app_url, str(syncsum_dir)) + body = _run_sync(fast_app_url) + assert body["state"] == "success", body + detail = body["detail"] + assert detail["added"] == N_FILES, detail + assert detail["updated"] == 0 and detail["pruned"] == 0, detail + assert detail["overview"] is True, detail + assert body["summaries_done"] == N_CANDIDATES, body + assert body["summaries_total"] == N_CANDIDATES, body + # The full regeneration pinned in the DB: one AI row per candidate. + assert _folder_rows() == { + "": (SUM_ROOT, False), + FOLDER_A: (SUM_ALPHA, False), + FOLDER_B: (SUM_BRavo, False), + } + + # The re-fetch (the phase-77 re-show refresh — the nav re-click): + # the synced catalog renders, and NO marker is anywhere — every + # description-bearing row has a stored line. + page.click("#nav-sources") + _wait_top_level(page) + src_row = page.locator("#folders-tbody tr") + expect(src_row.locator("a.folder-link")).to_have_text(SOURCE) + expect(src_row.locator("td:nth-child(2)")).to_have_text(str(N_FILES)) + expect(src_row.locator("td:nth-child(3) span")).to_have_text(SUM_ROOT) + expect(page.locator(".kb-summary-pending")).to_have_count(0) + + # The gap: delete the bravo row AND the source-root row directly + # (a historical fail-soft miss — the rows are gone from the app's + # back, no KB change anywhere). + _delete_folder_rows([(SOURCE, FOLDER_B), (SOURCE, "")]) + assert set(_folder_rows()) == {FOLDER_A} + + # The re-fetch: the two affected rows carry the D4 marker (the + # pending set = exactly the missing rows — the D3 concept), the + # intact folder shows its stored line and NO marker. + page.click("#nav-sources") + _wait_top_level(page) + src_row = page.locator("#folders-tbody tr") + s_span = src_row.locator("td:nth-child(3) span") + expect(s_span).to_have_text(PENDING_COPY) + expect(s_span).to_have_class("kb-summary-pending") + expect(s_span).to_have_attribute("title", PENDING_TITLE) + # The Edit button is KEPT with the marker (a manual save creates + # the row and clears the marker in place). + expect(src_row.locator(".kb-summary-edit")).to_be_visible() + + _drill(page, SOURCE) + # The SOURCE level itself is pending too (its root row is gone) — + # the level block shows the D4 pending note (title = the source + # name — the source root is folder ""). + expect(page.locator("#kb-level")).to_be_visible() + expect(page.locator("#kb-level-title")).to_have_text(SOURCE) + expect(page.locator("#kb-level-summary")).to_have_text(PENDING_NOTE) + rows = page.locator("#folders-tbody tr") + expect(rows).to_have_count(2) + alpha_row = rows.nth(0) + bravo_row = rows.nth(1) + expect(alpha_row.locator("a.folder-link")).to_have_text(FOLDER_A) + # The intact folder: the stored line, never the marker. + expect(alpha_row.locator("td:nth-child(3) span")).to_have_text(SUM_ALPHA) + expect(alpha_row.locator("td:nth-child(3) span")).not_to_have_class( + "kb-summary-pending" + ) + # The affected folder: the marker (copy + class + title). + b_span = bravo_row.locator("td:nth-child(3) span") + expect(b_span).to_have_text(PENDING_COPY) + expect(b_span).to_have_class("kb-summary-pending") + expect(b_span).to_have_attribute("title", PENDING_TITLE) + + # Drill into the affected folder: ITS level block shows the D4 + # pending note (title = the full source-relative path). + bravo_row.locator("a.folder-link").click() + expect(page.locator("#kb-level-title")).to_have_text(f"{SOURCE}/{FOLDER_B}") + expect(page.locator("#kb-level-summary")).to_have_text(PENDING_NOTE) + + # The row's Edit → Save a manual description → the marker is gone + # IN PLACE (the cell shows the saved text — no reload, the D4 + # in-place clear). + page.click(f'#kb-crumb a.kb-crumb-link:text-is("{SOURCE}")') # up to the source level + bravo_row = page.locator("#folders-tbody tr").nth(1) + expect(bravo_row.locator("a.folder-link")).to_have_text(FOLDER_B) + bravo_row.locator(".kb-summary-edit").click() + page.fill(".kb-summary-editor", MANUAL_BRavo) + page.click(".kb-summary-save") + expect(bravo_row.locator(".kb-summary-status")).to_have_text( + "Description updated." + ) + b_span = bravo_row.locator("td:nth-child(3) span") + expect(b_span).to_have_text(MANUAL_BRavo) + expect(b_span).not_to_have_class("kb-summary-pending") + + # The second (UNCHANGED-KB) sync: the gap probe finds exactly the + # source root missing (alpha stored, bravo manual) → the targeted + # fill regenerates the root row only. + body = _run_sync(fast_app_url) + assert body["state"] == "success", body + detail = body["detail"] + assert detail["added"] == 0, detail + assert detail["updated"] == 0 and detail["pruned"] == 0, detail + assert detail["overview"] is False, detail # unchanged → no overview burn + # The terminal keeps the gap-fill's final counts (ONE missing row). + assert body["summaries_done"] == 1, body + assert body["summaries_total"] == 1, body + # The DB pins the targeted fill: the root is BACK with its canned + # text (AI-written again), bravo keeps the MANUAL row (untouched — + # a regeneration would have rewritten the owner's words), alpha + # untouched. + rows_db = _folder_rows() + assert rows_db[""] == (SUM_ROOT, False), rows_db + assert rows_db[FOLDER_B] == (MANUAL_BRavo, True), rows_db + assert rows_db[FOLDER_A] == (SUM_ALPHA, False), rows_db + + # The re-fetch: the marker is GONE — the current level (the source + # root) shows the regenerated canned line in its level block, the + # folder rows show the stored texts, and no marker anywhere. + page.click("#nav-sources") + expect(page.locator("#kb-level")).to_be_visible() + expect(page.locator("#kb-level-title")).to_have_text(SOURCE) + expect(page.locator("#kb-level-summary")).to_have_text(SUM_ROOT) + rows = page.locator("#folders-tbody tr") + expect(rows.nth(0).locator("td:nth-child(3) span")).to_have_text(SUM_ALPHA) + expect(rows.nth(1).locator("td:nth-child(3) span")).to_have_text(MANUAL_BRavo) + expect(page.locator(".kb-summary-pending")).to_have_count(0) + # And at the top level: the source row's cell carries the + # regenerated root line, no marker. + page.locator("#kb-crumb a.kb-crumb-link").nth(0).click() + src_row = page.locator("#folders-tbody tr") + s_span = src_row.locator("td:nth-child(3) span") + expect(s_span).to_have_text(SUM_ROOT) + expect(s_span).not_to_have_class("kb-summary-pending") + expect(page.locator(".kb-summary-pending")).to_have_count(0) diff --git a/tests/integration/test_docs_api.py b/tests/integration/test_docs_api.py index c79ab96..b0a1f7c 100644 --- a/tests/integration/test_docs_api.py +++ b/tests/integration/test_docs_api.py @@ -14,7 +14,7 @@ from datetime import UTC, datetime, timedelta import pytest from fastapi.testclient import TestClient -from sqlalchemy import func, select, text +from sqlalchemy import delete, func, select, text import app.api.docs as docs_api from app.config import Settings @@ -22,6 +22,7 @@ from app.core import tokens as token_service from app.main import app as fastapi_app from app.models import Chunk, Document, FolderSummary, GitSource from app.rag import git_sources as rag_git_sources +from app.rag.folder_summaries import missing_folder_summaries _TREE_TABLES = "chunks, documents, folder_summaries, git_sources" @@ -65,6 +66,25 @@ def _tree_file_nodes(sources) -> list[dict]: return files +def _tree_pending_keys(sources) -> set[tuple[str, str]]: + """Every ``(source, folder_path)`` flagged ``summary_pending`` in a + tree response — the SOURCE root rides ``folder_path = ""`` (the + ``folder_summaries`` convention); walked recursively over the whole + tree (the D3 set the cross-check compares against + :func:`missing_folder_summaries`).""" + keys: set[tuple[str, str]] = set() + + def _walk(source_name: str, node: dict) -> None: + if node.get("summary_pending"): + keys.add((source_name, node["path"] if node.get("kind") == "folder" else "")) + for child in node.get("children", ()): + _walk(source_name, child) + + for source in sources: + _walk(source["name"], source) + return keys + + def test_docs_empty_shape(admin_client, db) -> None: db.execute(text("TRUNCATE chunks, documents")) db.commit() @@ -177,9 +197,12 @@ def test_docs_tree_populated_shape_order_counts_summaries(admin_client, db) -> N assert [s["name"] for s in sources] == ["Homelab", "Deployments"] homelab, deployments = sources - assert set(homelab) == {"name", "documents", "summary", "children"} + # Wire-additive (phase 98, task 03): the pre-pending keys are all + # still there, joined by ``summary_pending``. + assert set(homelab) == {"name", "documents", "summary", "summary_pending", "children"} assert homelab["documents"] == 4 # the whole recursive count assert homelab["summary"] == "Homelab docs." # the (source, "") row + assert homelab["summary_pending"] is False # the stored root row covers it assert deployments["summary"] is None # no stored root row assert deployments["documents"] == 2 @@ -276,13 +299,147 @@ def test_docs_tree_indexed_only_source_after_registered(admin_client, db) -> Non assert [s["name"] for s in sources] == ["Alpha", "Midx", "Zeta"] alpha, midx, zeta = sources assert (alpha["documents"], alpha["children"], alpha["summary"]) == (0, [], None) + assert alpha["summary_pending"] is False # 0 documents — never pending assert midx["documents"] == 1 + assert midx["summary_pending"] is False # 1 document — below the minimum assert zeta["documents"] == 1 + assert zeta["summary_pending"] is False # 1 document — below the minimum assert [c["path"] for c in midx["children"]] == ["m1.md"] _truncate_tree_tables(db) +def test_docs_tree_summary_pending_on_source_and_folder_nodes(admin_client, db) -> None: + """The endpoint returns ``summary_pending`` on SOURCE + FOLDER + nodes (phase 98, task 03 — D3): true iff the recursive count is + ≥ 2 AND no stored row; false WITH a stored row (any — the endpoint + cannot tell AI from manual); false for a < 2-document folder + (never pending) — including one NESTED. File nodes carry no flag.""" + _truncate_tree_tables(db) + base = datetime.now(UTC) + db.add(GitSource(url="https://github.com/reese/Homelab.git", kind="git", added_at=base)) + # k8s → 3 documents (talos + cluster + charts), NO stored row → pending + _seed_doc(db, "Homelab", "k8s/talos.md", "Talos", 1, base) + _seed_doc(db, "Homelab", "k8s/cluster.md", "Cluster", 1, base) + # k8s/helm → 1 document — below the 2-doc minimum, never pending + _seed_doc(db, "Homelab", "k8s/helm/charts.md", "Charts", 1, base) + # wiki → 2 documents, WITH a stored row → not pending + _seed_doc(db, "Homelab", "wiki/one.md", "One", 1, base) + _seed_doc(db, "Homelab", "wiki/two.md", "Two", 1, base) + db.add_all( + [ + # The 5-doc source root IS covered → the source node is not pending + FolderSummary(source="Homelab", folder_path="", summary="Homelab docs."), + FolderSummary(source="Homelab", folder_path="wiki", summary="Wiki pages."), + ] + ) + db.commit() + + r = admin_client.get("/api/docs/tree") + assert r.status_code == 200 + (homelab,) = r.json()["sources"] + assert set(homelab) == {"name", "documents", "summary", "summary_pending", "children"} + assert homelab["summary"] == "Homelab docs." + assert homelab["summary_pending"] is False + # Direct subfolders in path order: k8s < wiki. + k8s, wiki = [c for c in homelab["children"] if c["kind"] == "folder"] + assert k8s["path"] == "k8s" + assert k8s["summary"] is None + assert k8s["summary_pending"] is True # 3 docs, no stored row + helm = k8s["children"][0] + assert (helm["kind"], helm["path"]) == ("folder", "k8s/helm") + assert helm["summary_pending"] is False # 1 doc — never pending + assert wiki["summary"] == "Wiki pages." + assert wiki["summary_pending"] is False # 2 docs, but a stored row covers it + # File nodes carry no pending flag at all (the file table has no + # description column — D3's file exclusion). + for node in (k8s, wiki, helm): + for child in node["children"]: + if child["kind"] == "file": + assert "summary_pending" not in child + + _truncate_tree_tables(db) + + +def test_docs_tree_pending_set_equals_missing_folder_summaries(admin_client, db) -> None: + """The D3 cross-check (ONE concept end to end): with a PARTIAL + summary table (some rows deleted — the phase-96 gap-fill pattern), + the set of ``(source, folder_path)`` flagged pending in the fetched + tree (source root = ``""``) equals + :func:`missing_folder_summaries` — the marker can never drift from + the gap-fill.""" + _truncate_tree_tables(db) + base = datetime.now(UTC) + db.add(GitSource(url="https://github.com/reese/Alpha.git", kind="git", added_at=base)) + db.add( + GitSource( + url="https://github.com/reese/Beta.git", + kind="git", + added_at=base + timedelta(hours=1), + ) + ) + db.add( + GitSource( + url="https://github.com/reese/Gamma.git", + kind="git", + added_at=base + timedelta(hours=2), + ) + ) + # Alpha: a/b holds 2 docs, c holds 1 (NEVER a candidate), the root + # holds 4 — candidates (Alpha, ""), (Alpha, "a"), (Alpha, "a/b"). + _seed_doc(db, "Alpha", "a/b/c1.md", "C1", 1, base) + _seed_doc(db, "Alpha", "a/b/c2.md", "C2", 1, base) + _seed_doc(db, "Alpha", "c/solo.md", "Solo", 1, base) + _seed_doc(db, "Alpha", "top.md", "Top", 1, base) + # Beta: x holds 2 docs, the root holds 2 — candidates + # (Beta, ""), (Beta, "x"). + _seed_doc(db, "Beta", "x/one.md", "One", 1, base) + _seed_doc(db, "Beta", "x/two.md", "Two", 1, base) + # Gamma: registered, 0 documents — no candidates at all. + # Seed every candidate row, then DELETE two of them (the phase-96 + # direct-row-deletion pattern — a fail-soft miss / cleared row). + db.add_all( + FolderSummary(source=s, folder_path=f, summary=t) + for (s, f), t in { + ("Alpha", ""): "Alpha root.", + ("Alpha", "a"): "Alpha a.", + ("Alpha", "a/b"): "Alpha a b.", + ("Beta", ""): "Beta root.", + ("Beta", "x"): "Beta x.", + }.items() + ) + db.commit() + db.execute( + delete(FolderSummary).where( + FolderSummary.source == "Alpha", FolderSummary.folder_path == "a" + ) + ) + db.execute( + delete(FolderSummary).where( + FolderSummary.source == "Beta", FolderSummary.folder_path == "" + ) + ) + db.commit() + + r = admin_client.get("/api/docs/tree") + assert r.status_code == 200 + pending = _tree_pending_keys(r.json()["sources"]) + # THE cross-check: the marker set IS the gap-fill's candidate set. + assert pending == set(missing_folder_summaries(db)) + # And the explicit expectation (the test is readable without the + # helper): exactly the two deleted keys, root riding "". + assert pending == {("Alpha", "a"), ("Beta", "")} + # Never flagged: the < 2-doc folder (Alpha/c), the 0-document + # registered source (Gamma), and every node that still holds a row. + assert ("Alpha", "c") not in pending + assert not any(name == "Gamma" for name, _ in pending) + assert ("Alpha", "a/b") not in pending + assert ("Beta", "x") not in pending + assert ("Alpha", "") not in pending + + _truncate_tree_tables(db) + + # -------------------------------------------------------------------- # PATCH /api/folders/summary (phase 97, task 03) — the admin # folder-description editor: update / create / source-root / clear / diff --git a/tests/integration/test_sync_api.py b/tests/integration/test_sync_api.py index b794c0c..8024a66 100644 --- a/tests/integration/test_sync_api.py +++ b/tests/integration/test_sync_api.py @@ -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) -------------------- diff --git a/tests/integration/test_sync_folder_summaries.py b/tests/integration/test_sync_folder_summaries.py index 36e419e..62c4620 100644 --- a/tests/integration/test_sync_folder_summaries.py +++ b/tests/integration/test_sync_folder_summaries.py @@ -729,6 +729,13 @@ def test_api_changed_sync_generates_folder_rows( assert body["detail"]["added"] == 2 assert body["detail"]["overview"] is True assert body["detail"]["sources_version"] == 1 + # Phase 98 (task 01): the progress hook fired on the CHANGED-KB + # regeneration branch — the terminal clears phase + current_summary + # and keeps the hook's final counts (2 candidates: root + a/ — the + # only writer of those counters is the hook itself). + assert body["phase"] is None + assert body["current_summary"] is None + assert body["summaries_done"] == 2 and body["summaries_total"] == 2 # One LLM client for probe + import + overview + folder summaries; # exactly two FOLDER_SUMMARY_MODE calls (root + the 2-doc a/ folder). assert len(clients) == 1 @@ -770,6 +777,15 @@ def test_api_unchanged_resync_burns_zero_folder_calls( body = _poll(sync_client, "success") assert body["detail"]["overview"] is False assert body["detail"]["sources_version"] == 1 # unchanged → no bump + # Phase 98 (task 01): the no-gap skip branch never CALLS the + # generator — the hook never fires, so the terminal's summary + # counters stay 0/0 (the run stayed in the import phase through to + # the terminal, which then cleared it). The counter keep at 0/0 is + # the deterministic pin of "never fired": a fired hook's counts + # would survive to the terminal (the keep-final-counts rule). + assert body["phase"] is None + assert body["current_summary"] is None + assert body["summaries_done"] == 0 and body["summaries_total"] == 0 assert len(clients) == 2 # Zero GENERATION calls — no folder summary, no KB overview. The # only chat the re-sync's client makes is the phase-41 probe's ping. @@ -849,6 +865,12 @@ def test_api_unchanged_resync_with_gap_fills_only_the_missing_row( "chunks", "summaries", "summary_errors", "overview", "sources_version", } + # Phase 98 (task 01): the progress hook fired on the UNCHANGED-KB + # GAP-FILL branch — total is the missing count (the one deleted + # row), kept at the terminal next to the cleared phase keys. + assert body["phase"] is None + assert body["current_summary"] is None + assert body["summaries_done"] == 1 and body["summaries_total"] == 1 assert len(clients) == 2 # Targeted fill — exactly ONE folder call, the missing folder only # (plus the phase-41 probe's ping on the same client). @@ -910,6 +932,11 @@ def test_api_folder_lite_failure_keeps_rows_stays_green_and_bumps( assert body["error"] is None assert body["detail"]["updated"] == 1 assert body["detail"]["sources_version"] == 2 # the bump was not blocked + # Phase 98 (task 01): the failed (fail-soft) folder still advanced + # the hook's counter — the hook fires BEFORE the attempt, so the + # terminal keeps 2/2, not 1/2. + assert body["phase"] is None and body["current_summary"] is None + assert body["summaries_done"] == 2 and body["summaries_total"] == 2 assert current_sources_version(db) == 2 rows_after = _rows(db) assert rows_after["LocalDocs", "a"] == rows_before["LocalDocs", "a"] @@ -951,6 +978,11 @@ def test_api_empty_table_first_sync_regenerates( assert body["detail"]["overview"] is False # unchanged → no overview assert body["detail"]["sources_version"] == 1 # unchanged → no bump + # Phase 98 (task 01): the gap-fill branch over the emptied table — + # every candidate missing (2), the hook's final counts kept at the + # terminal next to the cleared phase keys. + assert body["phase"] is None and body["current_summary"] is None + assert body["summaries_done"] == 2 and body["summaries_total"] == 2 assert len(clients) == 2 assert len(_folder_calls(clients[1])) == 2 # the folders regenerated assert set(_rows(db)) == {("LocalDocs", ""), ("LocalDocs", "a")} diff --git a/tests/unit/test_folder_summaries.py b/tests/unit/test_folder_summaries.py index ee7b0b4..3bba5c9 100644 --- a/tests/unit/test_folder_summaries.py +++ b/tests/unit/test_folder_summaries.py @@ -48,7 +48,9 @@ class _FakeLLM: Records each ``(system, user)`` request and the ``model`` kwarg; returns the canned reply, or raises — either a fixed exception or a - per-folder failure keyed on the user message's ``Folder: …`` header + per-folder failure keyed on the user message's FIRST LINE (the + ``Folder: …`` header) — EXACT match, so a nested folder's header + (``Folder: S/a/b``) can never shadow its parent's (``Folder: S/a``) (the per-folder fail-soft tests). """ @@ -74,8 +76,9 @@ class _FakeLLM: system = messages[0]["content"] user = messages[-1]["content"] self.requests.append((system, user)) + header = user.splitlines()[0] if user else "" for folder in self._fail_folders: - if FOLDER_HEADER_PREFIX + folder in user: + if header == FOLDER_HEADER_PREFIX + folder: raise LLMError(f"simulated lite-model failure for {folder}") if self._fail is not None: raise self._fail @@ -916,3 +919,144 @@ def test_gap_probe_subsumes_the_table_empty_gate(db: Session, clean_tables) -> N db.execute(text("DELETE FROM folder_summaries")) db.commit() assert missing_folder_summaries(db) == [("FSU", ""), ("FSU", "a")] # emptied again + + +# ---------- on_progress hook (phase 98, task 01) ---------- + + +def _record_progress() -> tuple[list[tuple[int, int, str, str]], Any]: + """A fresh event list + the ``on_progress`` recorder that appends + every ``(done, total, source, folder_path)`` event it receives.""" + events: list[tuple[int, int, str, str]] = [] + + def record(done: int, total: int, source: str, folder_path: str) -> None: + events.append((done, total, source, folder_path)) + + return events, record + + +def test_on_progress_fires_once_per_candidate_in_sorted_key_order( + db: Session, clean_tables +) -> None: + """Phase 98 (task 01): the hook fires once per candidate, in the + same sorted ``(source, folder_path)`` order the folders are + attempted — done climbs 1..total, total = the candidate count + (the single-doc FSU-solo root is not a candidate — no event).""" + _seed_catalogue(db) + llm = _FakeLLM() + events, record = _record_progress() + stats = asyncio.run(generate_folder_summaries(db, llm, on_progress=record)) + assert events == [ + (1, 3, "FSU", ""), + (2, 3, "FSU", "a"), + (3, 3, "FSU", "a/b"), + ] + assert llm.calls == 3 # one event per attempt, in the same order + assert stats["generated"] == 3 + + +def test_on_progress_manual_skip_still_advances(db: Session, clean_tables) -> None: + """A manual-skip key is an INSTANT skip — no ``lite`` call burns, + but the counter still advances for it (D5: the UI's position moves + on either outcome).""" + _seed_catalogue(db) + db.add( + FolderSummary( + source="FSU", folder_path="a", summary="owner text", + manually_edited=True, + ) + ) + db.commit() + llm = _FakeLLM() + events, record = _record_progress() + stats = asyncio.run(generate_folder_summaries(db, llm, on_progress=record)) + assert events == [ + (1, 3, "FSU", ""), + (2, 3, "FSU", "a"), # the instant manual skip still advances + (3, 3, "FSU", "a/b"), + ] + assert llm.calls == 2, "the skip itself burns no call" + assert stats["kept_manual"] == 1 and stats["generated"] == 2 + + +def test_on_progress_failed_key_still_advances(db: Session, clean_tables) -> None: + """A failed (``LLMError``) key still advances — the hook fires + BEFORE the attempt, so a fail-soft miss is visible to the UI as a + completed step (the run never flips to failed, neither does the + counter stall).""" + _seed_catalogue(db) + llm = _FakeLLM(fail_folders=("FSU/a",)) + events, record = _record_progress() + stats = asyncio.run(generate_folder_summaries(db, llm, on_progress=record)) + assert events == [ + (1, 3, "FSU", ""), + (2, 3, "FSU", "a"), # the failed attempt still advances + (3, 3, "FSU", "a/b"), + ] + assert stats["failed"] == 1 and stats["generated"] == 2 + + +def test_on_progress_only_missing_reports_the_missing_count( + db: Session, clean_tables +) -> None: + """Under ``only_missing=True`` ``total`` is the MISSING count (the + loop-start count after the missing filter, not the full candidate + count) and the events name exactly the missing keys, sorted.""" + _seed_catalogue(db) + asyncio.run(generate_folder_summaries(db, _FakeLLM())) + db.commit() + db.execute( + text( + "DELETE FROM folder_summaries" + " WHERE (source, folder_path) IN (('FSU', ''), ('FSU', 'a/b'))" + ) + ) + db.commit() + llm = _FakeLLM() + events, record = _record_progress() + stats = asyncio.run( + generate_folder_summaries(db, llm, only_missing=True, on_progress=record) + ) + assert events == [ + (1, 2, "FSU", ""), + (2, 2, "FSU", "a/b"), + ] + assert llm.calls == 2 + assert stats["generated"] == 2 + + +def test_on_progress_skip_true_fires_zero_calls(db: Session, clean_tables) -> None: + """``skip=True`` (the ``--limit`` debug run) returns before the + loop — the hook never fires.""" + _seed_catalogue(db) + llm = _FakeLLM() + events, record = _record_progress() + stats = asyncio.run(generate_folder_summaries(db, llm, skip=True, on_progress=record)) + assert events == [] + assert llm.calls == 0 + assert stats == {"generated": 0, "failed": 0, "pruned": 0, "kept_manual": 0} + + +def test_on_progress_none_is_a_zero_cost_noop(db: Session, clean_tables) -> None: + """``on_progress=None`` (the ``scripts/import_docs.py`` CLI path) + behaves byte-identically to today: the fake LLM's call log, the + stats dict, and the stored rows are identical across a hooked and + a hook-less run.""" + _seed_catalogue(db) + hooked = _FakeLLM() + events, record = _record_progress() + stats_hooked = asyncio.run( + generate_folder_summaries(db, hooked, on_progress=record) + ) + requests_hooked = hooked.requests.copy() + assert len(events) == 3, "the hooked run fired (the contrast is real)" + db.rollback() # the generator only flushes — drop the uncommitted rows + + plain = _FakeLLM() + stats_plain = asyncio.run(generate_folder_summaries(db, plain, on_progress=None)) + + assert plain.requests == requests_hooked, "the fake LLM's call log is unchanged" + assert stats_plain == stats_hooked + assert _rows(db) == { + ("FSU", ""): REPLY, ("FSU", "a"): REPLY, ("FSU", "a/b"): REPLY + } diff --git a/tests/unit/test_frontend_sync_upload.py b/tests/unit/test_frontend_sync_upload.py index 6777f71..685758d 100644 --- a/tests/unit/test_frontend_sync_upload.py +++ b/tests/unit/test_frontend_sync_upload.py @@ -34,6 +34,18 @@ settled result line is off the status's {"message": "uploaded"} detail; the nameless variant after a reload / on the 409 re-attach), and the success announce names the next step. + +Phase 98 (task 02) pins the SYNC job's phase-aware label (D2): +fmtSyncPhaseLabel — "overview" → exactly "Writing KB overview…", +"summaries" → "Summarizing folders… (n/m)" (the +folder part omitted while current_summary is null → the bare +"Summarizing folders… (n/m)"), anything else → the byte-identical +phase-64 fmtSyncLabel("sync", …) fall-through — plus the status-object +threading (the tick's branch 1 + the initSyncButton running re-attach +pass the WHOLE status; the upload job and the 202/409 click stay on +the bare label — the upload status has no phase) and the A4 title-rule +adjustment (the full untruncated label rides the button title for +EVERY running label — the file-only rule is gone). """ from __future__ import annotations @@ -182,30 +194,86 @@ def test_fmt_sync_label_signature_and_prefixes() -> None: # ---------- enterSyncRunningState: full path to title + announcer ---------- -def test_enter_running_state_writes_full_path_to_title_and_announcer() -> None: +def test_fmt_sync_phase_label_is_the_exact_d2_contract() -> None: + """fmtSyncPhaseLabel(status) — the SYNC job's phase-aware label + (phase 98 task 02, D2): "overview" → exactly "Writing KB + overview…" (no folder, no counts); "summaries" → + "Summarizing folders… (n/m)" — the folder part + (one leading space + current_summary) is omitted while + current_summary is null (the phase's first poll → the bare + "Summarizing folders… (n/m)"); anything else (the null prelude, + "import") → the phase-64 fmtSyncLabel fall-through, byte-identical + (same builder, same fields, same order). The UPLOAD job never + enters here — its status has no phase (D2).""" + body = _fn(_js(), "fmtSyncPhaseLabel") + assert "function fmtSyncPhaseLabel(status)" in body + # "overview": the exact copy, no folder, no counts. + i_ov = body.find('status.phase === "overview"') + i_sum = body.find('status.phase === "summaries"') + assert -1 < i_ov < i_sum + ov = body[i_ov:i_sum] + assert 'return "Writing KB overview…";' in ov + assert "current_summary" not in ov and "summaries_done" not in ov, ( + "the overview label carries no folder and no counts" + ) + # "summaries": the exact template — optional folder, counts always. + i_fall = body.find("return fmtSyncLabel") + assert i_sum < i_fall, "the summaries branch precedes the fall-through" + sum_ = body[i_sum:i_fall] + assert 'status.current_summary ? ` ${status.current_summary}` : ""' in sum_, ( + "the folder part — one leading space + the folder, or nothing" + ) + assert ( + "return `Summarizing folders…${folder} " + "(${status.summaries_done}/${status.summaries_total})`;" + ) in sum_, "the exact D2 copy (with and without the folder)" + # anything else (null prelude, "import"): byte-identical to today's + # sync label — same builder, same fields, same order. + assert ( + 'return fmtSyncLabel("sync", status.current_file, ' + "status.files_done, status.files_total);" + ) in body[i_fall:] + + +def test_enter_running_state_is_phase_aware_and_announces_full_label() -> None: """The running-state entry keeps the §7.4 never-stale mechanics (disabled, aria-busy, spinning icon, the stale .is-error removed) - and — phase 64 — writes the FULL untruncated current file to the - button title (removed when null: no file yet) and the full - untruncated fmtSyncLabel text to BOTH the label span and - #sync-result (the aria-live announcer reads the full live path; - CSS only ellipsizes the button's span).""" + and — phase 98 (task 02, D2) — is phase-aware for the SYNC job: + a sync-kind call carrying the status object takes the + fmtSyncPhaseLabel form, while the upload job (no status — its + endpoint has no phase) and the 202/409 click keep the bare + fmtSyncLabel label, byte-identical. The FULL untruncated label — + whatever its form — rides the button title (set for EVERY running + label: the phase-64 "title only when there is a current file" + rule is adjusted away) and #sync-result (the aria-live announcer); + CSS ellipsizes only the label span.""" body = _fn(_js(), "enterSyncRunningState") + assert ( + "function enterSyncRunningState(kind, currentFile, done, total, status)" + in body + ) assert "syncBtn.disabled = true" in body assert 'syncBtn.setAttribute("aria-busy", "true")' in body assert "syncIcon.classList.add(\"is-spinning\")" in body assert "syncBtn.classList.remove(\"is-error\")" in body - assert "syncBtn.title = currentFile" in body, "the full path on hover" - assert 'syncBtn.removeAttribute("title")' in body, "removed when no file yet" - i_set = body.find("if (currentFile) syncBtn.title = currentFile") - i_remove = body.find('syncBtn.removeAttribute("title")') - assert -1 < i_set < i_remove, "title is set (not removed) only when a file exists" - assert "fmtSyncLabel(kind, currentFile, done, total)" in body - i_label = body.find("fmtSyncLabel(kind, currentFile, done, total)") + # the phase-aware split: sync + status → the D2 builder; the + # fall-through (upload / the click, no status) is the byte- + # identical phase-64 builder. + i_phase = body.find("fmtSyncPhaseLabel(status)") + i_fall = body.find("fmtSyncLabel(kind, currentFile, done, total)") + assert 'kind === "sync" && status' in body + assert -1 < i_phase < i_fall, "the phase label precedes the fall-through" + # A4: the untruncated label rides the title for EVERY running label + # (the remove-when-no-file rule is gone) and #sync-result. + assert "syncBtn.title = label" in body, "the full label on hover, always" + assert 'syncBtn.removeAttribute("title")' not in body, ( + "the title is set for every running label (D2: not just file labels)" + ) + i_title = body.find("syncBtn.title = label") i_span = body.find("syncLabel.textContent = label") i_result = body.find("syncResult.textContent = label") - assert -1 < i_label < i_span < i_result, ( - "one label: built once, written to the span AND the announcer" + assert -1 < i_title < i_span < i_result, ( + "one label: built once, written to title, span AND the announcer" ) @@ -327,14 +395,16 @@ def test_click_branch_enters_sync_running_without_a_file() -> None: def test_reattach_adopts_a_running_upload_only() -> None: - """initSyncButton: the sync branches are unchanged (running - re-enters with the live file; the terminals render the last - result). With the sync IDLE it fetches the upload status: a RUNNING - upload run re-attaches (running state, upload kind — phase 90: no - live file, the label stays bare "Importing…" — the synthetic - running frame, the poll starts); a terminal upload is a no-op — - the fall-through is the plain idle settle (the boot-time loadTree() - already shows the current catalog).""" + """initSyncButton: the sync terminals render the last result; the + RUNNING sync re-enters with the live file AND the whole status + (phase 98 task 02: the label is phase-aware — a mid-summaries + reload re-enters with the summaries label). With the sync IDLE it + fetches the upload status: a RUNNING upload run re-attaches + (running state, upload kind — phase 90: no live file, the label + stays bare "Importing…" — the synthetic running frame, the poll + starts); a terminal upload is a no-op — the fall-through is the + plain idle settle (the boot-time loadTree() already shows the + current catalog).""" body = _fn(_js(), "initSyncButton") assert "await fetchIsAdmin()" in body, "admin-only (no extra fetch)" assert 'fetch("/api/git-sources/upload/status")' in body @@ -356,6 +426,61 @@ def test_reattach_adopts_a_running_upload_only() -> None: assert body.rstrip().removesuffix("}").rstrip().endswith("applySyncIdle(status);") +def test_tick_sync_running_passes_the_status_for_the_phase_label() -> None: + """Branch 1 of the two-job tick (phase 98 task 02): the sync + running state passes the WHOLE syncStatus object as the fifth + argument — the phase-aware label (overview / summaries) reads its + phase fields. The upload running branch stays on the BARE four-arg + label (its status has no phase — D2): no fifth argument, no status + object anywhere in the branch.""" + tick = _tick(_js()) + b_sync_run = tick.find('syncStatus.state === "running"') + b_up_run = tick.find('uploadStatus && uploadStatus.state === "running"') + b_sync_ok = tick.find('syncStatus.state === "success"') + assert -1 < b_sync_run < b_up_run < b_sync_ok + branch1 = tick[b_sync_run:b_up_run] + i_call = branch1.find("enterSyncRunningState(") + call = branch1[i_call:branch1.find(");", i_call)] + assert re.search( + r'"sync",\s*syncStatus\.current_file,\s*syncStatus\.files_done,\s*' + r"syncStatus\.files_total,\s*syncStatus\s*$", + call, + ), "branch 1 passes the whole status (fifth arg) for the phase-aware label" + branch2 = tick[b_up_run:b_sync_ok] + i_call2 = branch2.find("enterSyncRunningState(") + call2 = branch2[i_call2:branch2.find(");", i_call2)] + assert re.fullmatch( + r'enterSyncRunningState\(\s*"upload",\s*uploadStatus\.current_file,\s*' + r"uploadStatus\.files_done,\s*uploadStatus\.files_total\s*$", + call2, + ), "the upload job keeps the bare four-arg label — no status, no phase (D2)" + assert "phase" not in call2 and "summaries" not in call2 + + +def test_reattach_sync_running_passes_the_status_for_the_phase_label() -> None: + """initSyncButton (phase 98 task 02): the RUNNING sync re-attach + passes the WHOLE status object — a reload mid-summaries re-enters + the running state with the summaries label (the never-stale + contract); the RUNNING upload re-attach stays on the bare four-arg + label (its status has no phase — D2).""" + body = _fn(_js(), "initSyncButton") + i_sync = body.find('"sync", status.current_file') + assert i_sync != -1 + call = body[i_sync:body.find(");", i_sync)] + assert re.search( + r'"sync",\s*status\.current_file,\s*status\.files_done,\s*' + r"status\.files_total,\s*status\s*$", + call, + ), "the RUNNING sync re-attach passes the status object (fifth arg)" + i_up = body.find('"upload", upload.current_file') + assert i_up != -1 + call2 = body[i_up:body.find(");", i_up)] + assert re.fullmatch( + r'"upload",\s*upload\.current_file,\s*upload\.files_done,\s*upload\.files_total\s*$', + call2, + ), "the RUNNING upload re-attach keeps the bare four-arg label (D2)" + + # ---------- the section header + the page comment ---------- diff --git a/tests/unit/test_kb_tree_builder.py b/tests/unit/test_kb_tree_builder.py index 8af061d..d5641f3 100644 --- a/tests/unit/test_kb_tree_builder.py +++ b/tests/unit/test_kb_tree_builder.py @@ -283,3 +283,135 @@ def test_cross_check_nested_level_matches_group_folder_listing() -> None: ("one/a.md", "A"), ("one/b.md", "B"), ] + + +# -------------------------------------------------------------------- +# summary_pending (phase 98, task 03) — the D3 rule (ONE concept): +# a SOURCE or FOLDER node is pending iff its recursive document count +# ≥ MIN_DOCS_PER_FOLDER (2) AND it has NO stored folder_summaries row +# (AI or manual — any row). That is exactly +# ``app.rag.folder_summaries.missing_folder_summaries``'s candidate +# set — the marker never drifts from the gap-fill (the integration +# cross-check in ``tests/integration/test_docs_api.py`` pins it +# end to end). FILE nodes carry no flag. +# -------------------------------------------------------------------- + + +def test_folder_two_docs_no_stored_row_is_pending() -> None: + """A ≥ 2-doc folder with NO stored row → ``summary_pending`` true + (the marker's "waiting to generate" semantics); file nodes carry + no flag at all (the file table has no description column).""" + rows = [ + ("S", "one/a.md", "A", 1, T0), + ("S", "one/b.md", "B", 1, T0), + ] + (source,) = build_kb_tree(["S"], rows, {}) + (one,) = _folder_nodes(source) + assert one.documents == 2 + assert one.summary is None + assert one.summary_pending is True + file = _file_nodes(one)[0] + assert "summary_pending" not in file.__class__.model_fields + assert "summary_pending" not in file.model_dump() + + +def test_folder_with_stored_row_is_not_pending() -> None: + """The same ≥ 2-doc folder WITH a stored row — ANY row, the builder + cannot tell AI from manual — is NOT pending (a description exists). + A row on the folder does not cover the source root: with no + ``(source, "")`` row the SOURCE node stays pending.""" + rows = [ + ("S", "one/a.md", "A", 1, T0), + ("S", "one/b.md", "B", 1, T0), + ] + (source,) = build_kb_tree(["S"], rows, {("S", "one"): "Manual."}) + (one,) = _folder_nodes(source) + assert one.summary == "Manual." + assert one.summary_pending is False + # The source root (2 docs) has no (source, "") row of its own. + assert source.summary is None + assert source.summary_pending is True + + +def test_single_document_folder_never_pending() -> None: + """A < 2-document folder is NEVER pending (it never gets a summary + — its one file line IS its description), even with no stored row + — while its ≥ 2-doc source root (no root row) still is.""" + rows = [ + ("S", "solo/only.md", "Only", 1, T0), # 1-doc folder + ("S", "top.md", "Top", 1, T0), # source total = 2 + ] + (source,) = build_kb_tree(["S"], rows, {}) + (solo,) = _folder_nodes(source) + assert solo.documents == 1 + assert solo.summary_pending is False + assert source.summary_pending is True + + +def test_name_collision_pending_follows_recursive_count() -> None: + """The phase-94 count-rule edge: documents ``one/a`` AND ``one/a/b`` + → folder ``one/a`` exists and its recursive count is 2 (the document + whose path EQUALS the folder name counts — the ``path == folder`` + arm) → pending true with no stored row, even though the folder has + only ONE direct file — pending follows the RECURSIVE count, not + the number of direct children. A stored row on the NESTED folder + alone clears only that marker (the rule is per node).""" + rows = [ + ("S", "one/a", "File A", 1, T0), # a file wearing the folder's name + ("S", "one/a/b.md", "B", 1, T0), # makes ``one/a`` a folder + ] + (source,) = build_kb_tree(["S"], rows, {}) + one = _folder_nodes(source)[0] + assert one.path == "one" + assert one.documents == 2 + assert one.summary_pending is True + a = _folder_nodes(one)[0] + assert a.path == "one/a" + assert a.documents == 2 # "one/a" itself + "one/a/b.md" + assert len(_file_nodes(a)) == 1 # ONE direct file — the count is not that + assert a.summary_pending is True + # Stored row on the nested folder only: it clears that node, and + # only that node. + (nested,) = build_kb_tree(["S"], rows, {("S", "one/a"): "Nested."}) + one2 = _folder_nodes(nested)[0] + a2 = _folder_nodes(one2)[0] + assert a2.summary == "Nested." + assert a2.summary_pending is False + assert one2.summary_pending is True + + +def test_source_root_pending_and_zero_document_source_never() -> None: + """The source root: a source with ≥ 2 docs and NO ``(source, "")`` + row → the SOURCE node is pending; the stored root row clears it. + A registered 0-document source is NEVER pending (0 < the minimum — + there is nothing to summarize), with or without a manual row.""" + rows = [ + ("Full", "x/1.md", "1", 1, T0), + ("Full", "y.md", "Y", 1, T0), + ] + full, empty = build_kb_tree(["Full", "Empty"], rows, {}) + assert full.documents == 2 + assert full.summary_pending is True + assert empty.documents == 0 + assert empty.summary_pending is False + full2, empty2 = build_kb_tree( + ["Full", "Empty"], rows, {("Full", ""): "Root desc."} + ) + assert full2.summary == "Root desc." + assert full2.summary_pending is False + assert empty2.summary_pending is False + + +def test_two_sources_pending_independently() -> None: + """Pending is computed per source: with one source holding a root + row and the other not, only the rowless source's node is pending — + the markers never leak across sources.""" + rows = [ + ("A", "a1.md", "A1", 1, T0), + ("A", "a2.md", "A2", 1, T0), + ("B", "b1.md", "B1", 1, T0), + ("B", "b2.md", "B2", 1, T0), + ] + a, b = build_kb_tree(["A", "B"], rows, {("A", ""): "A root."}) + assert (a.summary, a.summary_pending) == ("A root.", False) + assert (b.summary, b.summary_pending) == (None, True) diff --git a/tests/unit/test_kb_tree_ui.py b/tests/unit/test_kb_tree_ui.py index 20ab950..b892030 100644 --- a/tests/unit/test_kb_tree_ui.py +++ b/tests/unit/test_kb_tree_ui.py @@ -68,6 +68,39 @@ affordance, mirrored) — the source pins this module gains for it: ``reset()`` handle called on every re-render (PLAN §7.4); * the five ``.kb-summary-*`` classes in styles.css (house palette, no new hue in the phase-97 block). + +Phase 98 (task 04) adds the "SUMMARY PENDING" markers — the pins this +module gains for them: + +* the row Description cell's THREE text states (``makeDescCell``): + stored → the stored text (NEVER the marker); no stored + + ``summary_pending`` → the ``kb-summary-pending`` class on the + existing text span + the exact ``Summary pending`` copy + the exact + D4 title (textContent/title only — the house rule); neither → the + empty cell (the ls rule, unchanged) — with the Edit button + UNCONDITIONAL in all three (a manual save creates the row); +* the in-place clear: the editor's success path sets + ``node.summary_pending = false`` immediately after + ``node.summary = data.summary`` (no re-fetch — the marker clears in + the surface where the edit happened); +* the level block's OR condition (a stored description OR + ``summary_pending``) + the EXACT pending note — neither stored nor + pending stays hidden (the ls rule, unchanged) — and the level's + pending text takes the muted ``kb-summary-pending`` class on render + (the block is REUSED across levels — a stored level clears it); +* the editor's close re-derives the display state from the node (task + 05 defect fix): the pending marker's muted class + D4 tooltip + CANNOT survive a save (the flag is cleared first — the in-place + clear, with the stale "next sync" tooltip gone), and a cancel + restores the surface's pending display (each surface passes its + pending copy via ``pendingText`` — the row's ``Summary pending`` + marker, the level's D4 note — and its tooltip via ``pendingTitle``, + the row only); + +* ``.kb-summary-pending`` in styles.css (the ink-soft muted AA pair — + text + color, never color alone; no font/white-space/italic + overrides, so the row height is unchanged; no new hue in the + phase-97 block). """ from __future__ import annotations @@ -358,27 +391,45 @@ def test_top_level_lists_sources_and_hides_the_file_table() -> None: ) -def test_level_block_shows_the_stored_description_only() -> None: +def test_level_block_shows_the_stored_description_or_the_pending_note() -> None: """Inside a source/folder: the level block shows the CURRENT level's stored description — the title is the full source-relative path (e.g. `alpha/two`; the source root shows the source name) — - and is HIDDEN when none is stored (the ls rule: count only, no - placeholder).""" + or, since Phase 98 (task 04, D4), the PENDING note when the level + is summary_pending (no stored description yet — the next sync's + gap-fill will generate it, or the owner writes one via the block's + Edit button). HIDDEN only when NEITHER is stored nor pending (the + ls rule: count only, no placeholder — unchanged).""" js = _js() start = js.find("function renderLevel(") body = js[start : js.find("function renderEmpty(")] - assert "if (node.summary) {" in body, "the ls rule: no description → no block" - summary_if = body.find("if (node.summary) {") - shown = body[summary_if : body.find("} else {", summary_if)] + cond = "if (node.summary || node.summary_pending) {" + assert cond in body, "the block shows for a stored OR a pending level" + shown = body[body.find(cond) : body.find("} else {", body.find(cond))] assert "levelTitleEl.textContent = current.folder" in shown assert "current.source + \"/\" + current.folder" in shown, ( "the folder title is the full source-relative path" ) assert ": current.source;" in shown, "the source root title is the source name" - assert "levelSummaryEl.textContent = node.summary;" in shown + assert "levelSummaryEl.textContent = node.summary ||" in shown, ( + "the stored text first, the pending note as the fall-through" + ) + assert ( + "No description stored yet — the next sync will generate one. (You can write one yourself.)" + in shown + ), "the EXACT D4 pending note" + assert ( + 'levelSummaryEl.className = node.summary ? "" : "kb-summary-pending";' + in shown + ), ( + "the pending text is the muted marker style — and a stored level CLEARS a " + "previous pending level's class (the block is reused, task-05 defect fix)" + ) assert "levelEl.hidden = false;" in shown - hidden = body[body.find("} else {", body.find("if (node.summary) {")) :] - assert "levelEl.hidden = true;" in hidden[:200], "no stored row → the block is hidden" + hidden = body[body.find("} else {", body.find(cond)) :] + assert "levelEl.hidden = true;" in hidden[:200], ( + "neither stored nor pending → the block is hidden (the ls rule)" + ) def test_level_lists_subfolders_and_direct_files_only() -> None: @@ -595,8 +646,9 @@ def test_tree_cells_never_use_innerhtml_with_derived_data() -> None: for sink in ( "link.textContent = s.name", "link.textContent = f.path.split(\"/\").pop()", - 'text.textContent = (node && node.summary) || ""', - "levelSummaryEl.textContent = node.summary;", + "text.textContent = node.summary", + 'text.textContent = "Summary pending"', + "levelSummaryEl.textContent = node.summary ||", "a.textContent = label", "span.textContent = label", "countTd.textContent = String(s.documents)", @@ -686,30 +738,135 @@ def test_level_block_ships_the_static_edit_button() -> None: def test_row_description_cell_builds_text_and_always_present_edit() -> None: """makeDescCell (the shared Description-cell builder, called from - BOTH row builders): the stored description as a text node - (textContent — never innerHTML) + the Edit button added + BOTH row builders): the description text as a text node + (textContent — never innerHTML) + the Edit button built UNCONDITIONALLY — a description can be CREATED where none is stored (a < 2-document folder, the generator's fail-soft miss), so - there is NO gate on the stored value. The button is a real - type=button with a human aria-label, and the shared editor is - wired with a CONSTANT target { node, source, folder }.""" + the button sits OUTSIDE the three-state text branch (Phase 98: + the text gates on the node state, the button never does). The + button is a real type=button with a human aria-label, and the + shared editor is wired with a CONSTANT target { node, source, + folder }.""" body = _fn(_js(), "makeDescCell") - assert 'text.textContent = (node && node.summary) || ""' in body, ( - "the stored description is a text node (or an empty cell)" + # The Edit button is built unconditionally — the whole button + # block (from its creation to the append) carries no `if` gate on + # the stored description or the pending flag. + btn_block = body[ + body.find("const btn = document.createElement") : body.find("td.append(text, btn)") + ] + assert "if (" not in btn_block, ( + "always present: no gate on the stored description" ) assert 'btn.type = "button"' in body assert 'btn.className = "kb-summary-edit"' in body assert 'btn.textContent = "Edit"' in body assert 'btn.setAttribute("aria-label", `Edit description: ${label}`)' in body assert "td.append(text, btn)" in body - assert "if (node" not in body, ( - "always present: no gate on the stored description" - ) assert "getTarget: () => ({ node, source, folder })" in body, ( "a row's target is a constant (its own node)" ) +def test_desc_cell_pending_marker_branch() -> None: + """Phase 98 (task 04, D4): makeDescCell's THREE text states, in + order — (1) a stored summary → the stored text, NEVER the + marker; (2) no stored summary AND node.summary_pending → the + marker: the kb-summary-pending class on the EXISTING text span + + the exact "Summary pending" copy + the exact D4 title + (textContent/title only — the house rule, no innerHTML); (3) + neither stored nor pending → the empty cell (the ls rule, + unchanged).""" + body = _fn(_js(), "makeDescCell") + stored_i = body.find("if (node && node.summary) {") + pending_i = body.find("else if (node && node.summary_pending) {") + empty_i = body.find("} else {") + assert -1 < stored_i < pending_i < empty_i, ("stored → pending → empty, in order") + stored = body[stored_i:pending_i] + assert "text.textContent = node.summary;" in stored, "stored → the stored text" + assert "kb-summary-pending" not in stored, "a stored summary is NEVER the marker" + assert "Summary pending" not in stored + pending = body[pending_i:empty_i] + assert 'text.className = "kb-summary-pending"' in pending + assert 'text.textContent = "Summary pending"' in pending, "the D4 marker copy" + assert ( + 'text.title = "No stored description yet — the next sync will generate one."' + in pending + ), "the D4 title" + empty = body[empty_i : empty_i + 160] + assert 'text.textContent = ""' in empty, "neither stored nor pending → the empty cell" + # The marker is textContent/title only — the house rule (no + # innerHTML anywhere in the cell builder). + code = re.sub(r"//.*?$|/\*.*?\*/", "", body, flags=re.S | re.M) + assert "innerHTML" not in code + + +def test_save_success_clears_the_pending_flag_in_place() -> None: + """Phase 98 (task 04, D4): a successful save syncs the in-memory + node IN PLACE — node.summary = data.summary AND, immediately + after, node.summary_pending = false (a created/updated description + is no longer pending: the marker clears in the surface where the + edit happened, NO re-fetch). The flag clear sits between the + summary sync and the announcement, and nothing after it re-fetches + the tree (the re-fetch stays the safety net, not the in-place + clear).""" + body = _fn(_js(), "wireDescriptionEdit") + save = body[body.find("async function saveDescription()") :] + sync_i = save.find("node.summary = data.summary") + clear_i = save.find("node.summary_pending = false") + announce_i = save.find( + 'closeEditor(data.summary === null ? "Description cleared." : "Description updated.")' + ) + assert -1 < sync_i < clear_i < announce_i, ( + "summary sync → pending-flag clear → announce (in place)" + ) + tail = save[clear_i:] + assert "loadTree()" not in tail and 'fetch("' not in tail, ( + "the in-place clear is the whole of it — no re-fetch" + ) + + +def test_close_editor_rederives_the_pending_display() -> None: + """Phase 98 (task 05 defect fix, D4): closeEditor re-renders the + display state from the node with the SAME three-state rule the + surfaces use — the pending marker's muted class + D4 tooltip + CANNOT survive a save (the success path cleared + ``node.summary_pending`` first, so a stale "next sync" tooltip + under a just-created description is impossible), and a cancel + RESTORES the surface's pending display. Each surface passes its + pending copy via ``pendingText`` (the row's ``Summary pending`` + marker, the level's D4 note) and its tooltip via ``pendingTitle`` + (the row only — D4's title is the row cell's, the level's

+ carries none). textContent/class/title only — the house rule.""" + body = _fn(_js(), "wireDescriptionEdit") + close = body[body.find("function closeEditor(") : body.find("function openEditor()")] + flag_i = close.find('const pending = node !== null && stored === "" && node.summary_pending;') + value_i = close.find("const value = pending && pendingText ? pendingText : stored;") + class_i = close.find('textEl.className = pending ? "kb-summary-pending" : "";') + title_i = close.find("if (pendingTitle) textEl.title = pendingTitle;") + clear_title_i = close.find('textEl.removeAttribute("title")') + render_i = close.find("textEl.textContent = value") + assert -1 < flag_i < value_i < class_i < title_i < clear_title_i < render_i, ( + "flag → value → class → title (set/clear) → text, in order" + ) + # The row passes its marker copy + the EXACT D4 tooltip… + cell = _fn(_js(), "makeDescCell") + assert 'pendingText: "Summary pending"' in cell + assert ( + 'pendingTitle: "No stored description yet — the next sync will generate one."' + in cell + ), "the D4 tooltip is the row's" + # …and the level passes the D4 note WITHOUT a tooltip. + js = _js() + wire = js[js.find("levelEditor = wireDescriptionEdit({") : js.find("/* ---------- view boot")] + assert 'pendingText:' in wire and "pendingTitle" not in wire, ( + "the level's

carries the note, never a tooltip (D4)" + ) + assert ( + "No description stored yet — the next sync will generate one. (You can write one yourself.)" + in wire + ), "the level's pending copy is the D4 note" + + def test_editor_swap_builds_textarea_save_cancel_and_live_region() -> None: """Edit swaps the description UI for the inline editor: a