Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
addbd4ca08 | ||
|
|
e2d08a95a9 |
@@ -0,0 +1,58 @@
|
|||||||
|
# Phase 107 — True per-file document dates for URL git sources: full-history checkouts (revisits phase 106 D10)
|
||||||
|
|
||||||
|
**Source:** Owner bug report 2026-09-16 (chat): after the phase-106 re-sync of brain.reeseapps.com, the git source `https://gitea.reeseapps.com/services/homelab.git` shows `active/container_bifrost` created **8/16/2026** — "completely wrong, container_bifrost is > 6 months old at this point"; "This is a git source, so it should be easy to tell when a document was last edit[ed]".
|
||||||
|
|
||||||
|
**Story:** n/a (owner bug report, phase-106 follow-up — the phase's E2E suite proves the fix end to end).
|
||||||
|
|
||||||
|
**Context / verified root cause (2026-09-16, scratch + dev checkouts of the live homelab repo):** `clone_or_pull` (`scripts/git_sync.py` L63) clones URL-transport sources with `--depth 1` — phase 28's strategy, which phase 106's **D10 explicitly locked** ("No clone-strategy change… `--depth 1` stays"), predicting the consequence: "URL git sources are shallow → every file carries the repo's TIP-commit date (uniform within the repo…); revisit only if the owner later wants intra-repo recency on URL sources." In a shallow checkout git cannot see history past the shallow boundary (= the tip commit), so `file_commit_dates` (phase 106, `git log --name-only --format=@@%cI` first-sighting-wins) returns the **tip commit's date for EVERY file in the repo** — that uniform tip date is what the live site now displays for `container_bifrost` (the live clone's tip; the dev clone of the same repo shows 2026-09-07 for every file, `git rev-parse --is-shallow-repository` → `true`, exactly ONE `@@` line in the date walk). After `git fetch --unshallow` (438 commits visible) the TRUE last-commit date of `active/container_bifrost/bifrost.md` is **2026-05-05T06:26:40-04:00** — months older than the displayed date. Local-PATH git sources were never affected (git ignores `--depth` for local clones → full history → true dates — which is why the bug only surfaced on the URL source). Everything DOWNSTREAM of the checkout (the importer's `doc_dates_by_root` map + first-sighting-wins walk, D3 normalization, D4 sync semantics, the API/LLM/UI date surfaces, the recency boost) is CORRECT as built — the wrong value is produced at the checkout, so the fix is confined to `clone_or_pull` plus the docstrings/tests that enshrine the shallow assumption.
|
||||||
|
|
||||||
|
**⚠ LOCKED-DECISION REVISIT (AGENTS.md rule 3 — flagged, not silent):** this phase REVISITS phase 106's locked decision **D10** (shallow clone strategy). The revisit is owner-authorized: D10 itself names this exact trigger ("revisit only if the owner later wants intra-repo recency on URL sources") and the owner's 2026-09-16 report is precisely that request. D10's tip-date expectation is hereby SUPERSEDED; every other phase-106 decision (D1–D9 — storage/flag, provenance walk, normalization, sync semantics, LLM surfaces, recency boost, UI columns, derived folder dates) stands unchanged and simply receives true dates. No `PLAN.md` change is involved (D10 is a phase-level decision, not a PLAN §2 anchor).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Git-source checkouts keep FULL history — a fresh `clone_or_pull` clones without `--depth 1`, and any EXISTING shallow checkout (including the live + dev homelab checkouts) self-heals via `git fetch --unshallow` on its next sync — so `file_commit_dates` yields the TRUE per-file last-commit date for every git source (local and URL). A `file://` E2E proves it end to end: an old file shows its old commit date and a tip-touched file shows the tip date, in the API, the Sources tables, and the document viewer — instead of the uniform tip date the bug produced.
|
||||||
|
|
||||||
|
## Dependencies
|
||||||
|
- `106_document_dates` (complete) — the entire date pipeline this phase corrects AT THE SOURCE: `file_commit_dates` (task 03), the importer's `doc_dates_by_root` map + D4 refresh semantics (task 04), D3 normalization, and the API/LLM/UI surfaces. Only D10's shallow-clone assumption is revised; its suites are the regression gate.
|
||||||
|
- `28_git_based_sources` (complete) — `scripts/git_sync.py` (the ONLY git-invocation site, A11 — `run_git` contract, `GitSyncError` semantics) and `clone_or_pull` itself. `app/core/docs_push.py`'s own `--depth 1/100` fetches operate on the DOCS repo (phase 18/59) and are NOT touched.
|
||||||
|
- `32_admin_sync_button` (complete) — `tests/e2e/test_sync_button.py`'s app-server idiom: per-module env with `BOR_GIT_SOURCES=file://<fixture repo>` + its own `BOR_SOURCES_DIR`, a real fixture repo built via `git` subprocesses with controlled commit dates, the real in-app sync. The new E2E copies it.
|
||||||
|
- `99_kb_tree_table_and_back_nav` (complete) — the Sources RAG-view tables + `GET /api/docs/tree` the E2E asserts on (the `Created`/`Updated` columns phase 106 D8/D9 added).
|
||||||
|
|
||||||
|
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||||
|
|
||||||
|
- **The fix — `scripts/git_sync.py::clone_or_pull` (task 01; the single clone/pull entry point BOTH sync entry points call — `app/api/sync.py` L296 and `scripts/import_docs.py` L237 — so one change fixes the UI Sync button and the CLI at once):**
|
||||||
|
1. **Fresh checkout** (dest absent or without `.git`): `run_git(["git", "clone", url, str(dest)], cwd=dest.parent)` — the `--depth 1` flags are REMOVED (full history on the first clone for every transport: https/ssh/`file://`/local-path). One-time cost only — subsequent syncs are incremental; the KB repos are small homelab-docs repos. Deliberately NO env knob to restore shallow: shallow == the bug (D12).
|
||||||
|
2. **Existing checkout** (dest has `.git`): probe first — `run_git(["git", "rev-parse", "--is-shallow-repository"], cwd=dest)`; stdout.strip() == `"true"` → `run_git(["git", "fetch", "--unshallow"], cwd=dest)` (the ONE-TIME self-heal for checkouts that are already shallow — live + dev homelab included — so the fix reaches deployed sites WITHOUT a re-clone: the next sync after deploy unshallows, pulls, and re-dates), then `run_git(["git", "pull", "--ff-only"], cwd=dest)` as today. Probe `"false"` → straight to `git pull --ff-only` (the common steady-state path — one extra cheap probe per sync, no network fetch).
|
||||||
|
3. **Fail loud (D12):** ANY of the probe/unshallow/pull steps raising `GitSyncError` propagates exactly like today's clone/pull failures — the sync aborts with the named repo + reason (the `app/api/sync.py` 502 surface, the CLI traceback). NEVER a silent fallback: continuing a failed unshallow would silently re-serve tip dates (the bug), and falling back to mtimes would be worse. A broken checkout failing loudly is the phase-28 contract.
|
||||||
|
- Every git invocation still goes through `run_git` (A11 — the module docstring's git-inventory sentence lists the three new/changed commands).
|
||||||
|
- `file_commit_dates` is UNCHANGED in code — on a full-history checkout its existing newest-first, first-sighting-wins walk already returns the true per-file last commit (verified 2026-09-16: 438 commits, `bifrost.md` → 2026-05-05). ONLY its docstring + the module docstring lose the "shallow URL → uniform tip date" narrative and state the new guarantee: every `clone_or_pull` checkout is full-history → TRUE per-file dates for ALL git sources.
|
||||||
|
- **Downstream — deliberately untouched:** the importer's `doc_dates_by_root` plumbing, D1/D3/D4 (storage, normalization, sync-refresh semantics — including "a date may go OLDER", which is exactly how the wrong tip dates self-correct on the first post-fix sync: every git document's stored date is refreshed to its true commit date, `dates_updated` counts them, `sources_meta` does NOT bump — a date-only refresh is `unchanged` per D4), the admin date API, the LLM surfaces, the UI columns, the recency boost. No migration, no data fix — the sync IS the fix. `created_at_manual = true` rows keep the owner's corrections (D1) — correct as designed.
|
||||||
|
- **Stale narratives to correct (same task as the code — a comment that lies is a bug):** `scripts/git_sync.py` module docstring (the L18-28 "Per-file last-commit dates" block: the URL-shallow bullet is false after this phase), `clone_or_pull`'s docstring (L48-51 "shallow, depth 1" + the behavior list), `file_commit_dates`'s docstring (L104-111), `scripts/import_docs.py` docstring L19 ("first run, shallow ``--depth 1``" → "first run, full history"), `app/api/sync.py` per-row comment (L298-303, "shallow URL checkouts → the tip date, D10" → true per-file dates for all git sources).
|
||||||
|
|
||||||
|
## Tasks
|
||||||
|
1. `01_full_history_checkouts.md` — `clone_or_pull`: no-`--depth` clone + the shallow-probe/`fetch --unshallow` self-heal + fail-loud; the unit argv pins, the real-git integration pins (true per-file dates over `file://` + the existing-shallow self-heal), and the five stale docstring/comment sites.
|
||||||
|
2. `02_e2e_git_source_dates.md` — dedicated Playwright suite `tests/e2e/test_git_source_dates.py` (isolation): a two-commit `file://` fixture repo, a real in-app sync, true per-file dates asserted in the API + the Sources tables + the viewer badge.
|
||||||
|
3. `03_gates_and_commit.md` — full gate (unit + integration, coverage >90%, the new E2E + the three regression suites in isolation, ruff + pyright), one atomic `--no-gpg-sign` commit, phase dir → `complete/`.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit — `tests/unit/test_git_sync.py` (task 01): fresh-clone argv re-pinned to `["git", "clone", url, str(dest)]` (NO `--depth`); the existing-checkout path asserts probe-then-pull; NEW: probe `"true"` → `fetch --unshallow` THEN `pull --ff-only` (argv + order); NEW: `fetch --unshallow` failure → `GitSyncError` propagates (D12 fail-loud); the `git clone --depth 1 … failed (exit 128)` match-string updated to the new argv.
|
||||||
|
- Integration — `tests/integration/test_git_file_dates.py` (task 01, real `git`, DB-free, the git-availability skip pattern): the module docstring's D10 tip-date expectation is REPLACED with the full-history guarantee; `test_shallow_file_clone_yields_tip_date_for_every_file` (which pinned THE BUG) is REPLACED by `test_url_clone_yields_true_per_file_dates` — the same two-commit recipe (`a.md`/`docs/deep.md` committed 2020-01-02, `b.md` touched again at the 2024-06-15 tip), but `clone_or_pull(f"file://{scratch_repo}", dest)` (a URL transport, through the real function) must yield `{"a.md": DATE_A, "b.md": DATE_B, "docs/deep.md": DATE_A}` — the regression pin: pre-fix this returned `DATE_B` for all three; NEW `test_existing_shallow_checkout_self_heals` — the harness builds a `--depth 1` `file://` clone directly (simulating the deployed checkouts: dates uniform tip, `is-shallow` true), then `clone_or_pull(url, dest)` → no longer shallow + true per-file dates; the local-clone true-date test and every fail-soft test stay green unchanged.
|
||||||
|
- E2E (mandatory, A16) — `tests/e2e/test_git_source_dates.py` (task 02): `uv run pytest tests/e2e/test_git_source_dates.py -v --no-cov` with the DB up.
|
||||||
|
- Coverage: **>90%** on `app/` (`uv run pytest --cov=app --cov-report=term-missing` — the validate.sh gate; `scripts/` is outside the `app/` denominator but fully pinned by the suites above).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] A fresh `clone_or_pull` over a URL transport yields a NON-shallow checkout (unit argv pin: no `--depth`; integration: `git rev-parse --is-shallow-repository` on the `file://` clone → `false`) with TRUE per-file last-commit dates (2020 file stays 2020, tip-touched file gets the tip date — NOT uniform).
|
||||||
|
- [ ] An EXISTING shallow checkout (made with `--depth 1`, like every deployed one) unshallows on its next `clone_or_pull` (integration pin: probe → `fetch --unshallow` → pull; dates true afterwards) and a non-shallow checkout takes the plain pull path (unit argv pins); a failed unshallow aborts the sync with `GitSyncError` (fail loud, D12).
|
||||||
|
- [ ] `tests/integration/test_git_file_dates.py` green with the D10 tip-date test replaced by the true-date regression pin; `tests/unit/test_git_sync.py` green with the updated argv pins.
|
||||||
|
- [ ] E2E green in isolation: after a real in-app sync of a two-commit `file://` fixture, `GET /api/docs` carries `created_at[:10]` = the OLD commit date for the old file and the TIP date for the new file (the two DIFFER — the bug made them identical); the Sources file table's `Created` column renders the two different years; the old document's viewer `Created` badge carries the old date (ISO `title`); the folder `Updated` columns are the subtree maxes (old folder 2020, new folder 2024).
|
||||||
|
- [ ] The phase-106 regression suites green in isolation: `uv run pytest tests/e2e/test_document_dates.py -v --no-cov`, `tests/e2e/test_sync_button.py -v --no-cov`, `tests/e2e/test_git_sources_admin.py -v --no-cov`; `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||||
|
- [ ] The five stale shallow/tip-date narratives (git_sync module + two function docstrings, import_docs docstring, sync.py comment) now describe full-history checkouts — no remaining claim that URL sources carry tip dates.
|
||||||
|
- [ ] One `--no-gpg-sign` Conventional Commit; phase dir moved to `.agents/phases/complete/` by the pipeline gate.
|
||||||
|
|
||||||
|
## Locked decisions
|
||||||
|
- **D11 — Full-history git checkouts (SUPersedes phase 106 D10; owner-authorized 2026-09-16 — D10's own revisit clause).** Fresh `clone_or_pull` checkouts clone WITHOUT `--depth 1`; existing checkouts are probed with `git rev-parse --is-shallow-repository` and, while shallow, `git fetch --unshallow` before the usual `git pull --ff-only` (one-time self-heal of deployed checkouts, no re-clone). Consequence: `file_commit_dates` returns the TRUE per-file last-commit date for EVERY git source — local and URL — and the next sync after deploy refreshes every git document's stored `created_at` to its true commit date (D4's "may go older" makes this a plain date-only refresh: no `sources_meta` bump, manual corrections survive, `dates_updated` counts them).
|
||||||
|
- **D12 — Fail loud, never silently re-shallow (the house fail-loud rule applied to the self-heal).** A probe/unshallow/pull failure raises `GitSyncError` and aborts the sync exactly like any clone/pull failure; there is no env knob to restore shallow clones or to skip the self-heal — a shallow checkout would silently re-serve the uniform tip date, i.e. the bug this phase fixes.
|
||||||
|
|
||||||
|
## Commit
|
||||||
|
```bash
|
||||||
|
git add scripts/git_sync.py scripts/import_docs.py app/api/sync.py tests/ .agents/phases/ && git commit --no-gpg-sign -m "fix(git): full-history checkouts so URL sources get true per-file document dates"
|
||||||
|
```
|
||||||
@@ -0,0 +1,43 @@
|
|||||||
|
# Task 01 — `clone_or_pull` full-history checkouts: no `--depth 1` + the shallow self-heal (D11/D12)
|
||||||
|
|
||||||
|
**Phase:** `107_git_full_history_dates` · **Source:** owner bug report 2026-09-16 — URL git sources show the repo TIP date for every document (`container_bifrost` "created 8/16/2026", months off) because phase 106 D10 locked `--depth 1` shallow URL clones.
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Make every `clone_or_pull` checkout carry FULL git history — fresh clones without `--depth 1`, existing shallow checkouts self-healing via `git fetch --unshallow` before the usual fast-forward — so `file_commit_dates` (unchanged) yields true per-file last-commit dates for all git sources; and correct every docstring/comment that enshrines the old shallow assumption.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `scripts/git_sync.py::clone_or_pull` — the behavior change (keep the signature, the `dest`/`.git` dispatch, and the `GitSyncError` semantics; EVERY invocation through `run_git`, A11):
|
||||||
|
- Fresh checkout (dest absent or without `.git`): `dest.parent.mkdir(parents=True, exist_ok=True)` then `run_git(["git", "clone", url, str(dest)], cwd=dest.parent)` — the `"--depth", "1"` arguments are REMOVED (full history for every transport).
|
||||||
|
- Existing checkout (`.git` present): probe `shallow = run_git(["git", "rev-parse", "--is-shallow-repository"], cwd=dest).strip() == "true"`; if `shallow` → `run_git(["git", "fetch", "--unshallow"], cwd=dest)` (the one-time self-heal — deployed checkouts, live + dev included, become full-history on their NEXT sync with no re-clone); then `run_git(["git", "pull", "--ff-only"], cwd=dest)` as today. Probe `"false"` → straight to the pull (steady state: one cheap local probe, no network).
|
||||||
|
- **D12 fail-loud:** do NOT catch `GitSyncError` — probe, unshallow, and pull failures all propagate exactly like today's clone/pull failures (the sync aborts with the named repo + git's stderr; never a silent fallback to tip dates or mtimes).
|
||||||
|
- Docstrings (the lies are the rest of the bug):
|
||||||
|
- module docstring: L3 "(shallow, depth 1)" → full-history clone; the L18-28 "Per-file last-commit dates (phase 106, D2/D10)" block → rewrite for phase 107: a `clone_or_pull` checkout is FULL-history for every transport (fresh: no `--depth`; existing: `--is-shallow-repository` probe + one-time `git fetch --unshallow` self-heal before the `--ff-only` pull) → `file_commit_dates` yields TRUE per-file last-commit dates for ALL git sources (local AND URL); cite the 2026-09-16 verification (shallow walk = one tip commit; after `--unshallow` the true per-file dates, e.g. `bifrost.md` 2026-05-05 not the 2026-09-07 tip).
|
||||||
|
- `clone_or_pull` docstring (L48-51 + the behavior bullets): "shallow, first run" → "full history, first run"; the bullet list gains the probe/unshallow step; drop "(shallow: the KB is re-imported incrementally anyway)".
|
||||||
|
- `file_commit_dates` docstring (L104-111): the "local FULL history / URL shallow → uniform TIP date (D10)" paragraph → "every `clone_or_pull` checkout is full history → true per-file last-commit dates for all git sources (phase 107 D11 — supersedes phase 106 D10)"; the fail-soft paragraph stays.
|
||||||
|
2. `scripts/import_docs.py` — docstring L19: "(first run, shallow ``--depth 1``) or fast-forwarded" → "(first run, full history — no ``--depth``; an existing shallow checkout is unshallowed first, phase 107) or fast-forwarded". No code change here (it already calls `clone_or_pull` and then `file_commit_dates` — the fix flows through).
|
||||||
|
3. `app/api/sync.py` — per-row comment (L298-303): replace "(local-path checkouts → true per-file dates, shallow URL checkouts → the tip date, D10)" with "(full-history checkouts → true per-file last-commit dates for every git source — phase 107 D11 supersedes phase 106 D10's shallow tip-date behavior)". No code change.
|
||||||
|
4. Tests (run `uv run pytest tests/unit/test_git_sync.py tests/integration/test_git_file_dates.py -v` — DB-free):
|
||||||
|
- `tests/unit/test_git_sync.py` (subprocess fully faked, argv pins — update the module docstring's first lines too: "``git clone`` (full history, fresh dest) or a shallow-probe + optional ``git fetch --unshallow`` + ``git pull --ff-only`` (existing checkout)"):
|
||||||
|
- `test_clone_or_pull_clones_when_dest_has_no_git_dir` → `call["argv"] == ["git", "clone", url, str(dest)]` (NO `--depth`) — same cwd-parent assertion.
|
||||||
|
- `test_clone_or_pull_creates_missing_parent_before_clone` → same argv re-pin.
|
||||||
|
- `test_clone_or_pull_pulls_when_git_dir_exists` → the fake `run_git` must now answer the probe (stdout `"false\n"`) before the pull: `calls[0]["argv"] == ["git", "rev-parse", "--is-shallow-repository"]`, `calls[1]["argv"] == ["git", "pull", "--ff-only"]`, both cwd=dest.
|
||||||
|
- NEW `test_clone_or_pull_unshallows_existing_shallow_checkout` → probe stdout `"true\n"` → `calls[0]` probe, `calls[1]["argv"] == ["git", "fetch", "--unshallow"]`, `calls[2]["argv"] == ["git", "pull", "--ff-only"]` (order pinned: unshallow BEFORE pull).
|
||||||
|
- NEW `test_clone_or_pull_unshallow_failure_propagates` → probe `"true"`, unshallow raises `GitSyncError("git fetch --unshallow failed (exit 128): fatal: …")` → `pytest.raises(GitSyncError, match="--unshallow")` (D12 fail-loud) and the pull is NEVER called.
|
||||||
|
- the error-path test's match string `r"git clone --depth 1 .* failed \(exit 128\): fatal: repository not found"` → `r"git clone .* failed \(exit 128\): fatal: repository not found"`.
|
||||||
|
- The fake-run helper must dispatch by argv (clone/probe/unshallow/pull) — extend it, keep the existing `_FakeProc` shape.
|
||||||
|
- `tests/integration/test_git_file_dates.py` (real `git`, DB-free, keep the `GIT` skip mark + the `scratch_repo` recipe + every fail-soft/parser test):
|
||||||
|
- module docstring: replace the "shallow URL-transport clone → TIP date for EVERY file (D10)" bullet with the phase-107 guarantee (a `clone_or_pull` `file://` checkout is full history → true per-file dates; an existing shallow checkout self-heals) and note the D10 supersession.
|
||||||
|
- REPLACE `test_shallow_file_clone_yields_tip_date_for_every_file` with `test_url_clone_yields_true_per_file_dates(scratch_repo, tmp_path)`: `clone_or_pull(f"file://{scratch_repo}", tmp_path / "url-clone")` → `file_commit_dates(dest) == {"a.md": DATE_A, "b.md": DATE_B, "docs/deep.md": DATE_A}` (the REGRESSION PIN — pre-fix this returned `DATE_B` for all three files) AND `run_git(["git", "rev-parse", "--is-shallow-repository"], cwd=dest).strip() == "false"` (the checkout is not shallow).
|
||||||
|
- NEW `test_existing_shallow_checkout_self_heals(scratch_repo, tmp_path)`: the harness builds a shallow checkout directly (`_git(tmp_path, "clone", "-q", "--depth", "1", f"file://{scratch_repo}", str(dest))` — simulating the pre-phase deployed checkouts) → pre-heal: `is-shallow` true and `file_commit_dates` uniform `DATE_B` for all three files; then `clone_or_pull(f"file://{scratch_repo}", dest)` → `is-shallow` false and true per-file dates (same dict as the previous test).
|
||||||
|
- `test_local_clone_yields_true_per_file_dates` stays green unchanged (local path was always full history).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: argv + order pins for the fresh-clone (no `--depth`), probe-then-pull, probe→unshallow→pull, and fail-loud paths (the fake subprocess never sees the real git — deterministic).
|
||||||
|
- Integration: real `git` scratch repos — URL-transport true dates, the self-heal lifecycle, the pre-existing fail-soft battery.
|
||||||
|
- Coverage: **>90%** on `app/` (no `app/` code changes this task — the gate still passes; `scripts/` is pinned by the suites above).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `clone_or_pull` fresh-clone argv is `["git", "clone", url, str(dest)]` (unit-pinned); existing checkouts take probe → (unshallow iff shallow) → `pull --ff-only` (unit-pinned, order asserted); a failed unshallow raises `GitSyncError` before the pull runs (D12)
|
||||||
|
- [ ] `uv run pytest tests/unit/test_git_sync.py tests/integration/test_git_file_dates.py -v` green, including `test_url_clone_yields_true_per_file_dates` (per-file true dates over `file://` — the bug's regression pin) and `test_existing_shallow_checkout_self_heals`
|
||||||
|
- [ ] No remaining "shallow"/"tip date" narrative in `scripts/git_sync.py`, `scripts/import_docs.py`, or `app/api/sync.py` describing CURRENT behavior (phase-106 phase records in `complete/` are history — untouched)
|
||||||
|
- [ ] `uv run ruff check . && uv run pyright` clean; full `uv run pytest` green
|
||||||
@@ -0,0 +1,30 @@
|
|||||||
|
# Task 02 — E2E: true per-file git dates end to end (`tests/e2e/test_git_source_dates.py`)
|
||||||
|
|
||||||
|
**Phase:** `107_git_full_history_dates` · **Source:** owner bug report 2026-09-16 — the UI must show the document's true last-commit date (a `file://` URL git source is the transport-true stand-in for the live https source; pre-fix it rendered the uniform tip date).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Add the phase's dedicated Playwright suite (A16, run in isolation) that proves the fix end to end: a real `file://` git fixture with two commits of controlled dates, a real in-app admin sync, and assertions that the OLD file shows its OLD commit date and the NEW file shows the TIP date — in `GET /api/docs` / `GET /api/docs/tree` (deterministic ISO) and in the Sources tables + document viewer (locale-tolerant).
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `tests/e2e/test_git_source_dates.py` (NEW — copy the app-server + fixture idiom from `tests/e2e/test_sync_button.py` verbatim in shape: module-scoped `app_server` with per-module env, `tmp_path_factory`-scoped real git fixture, per-test fresh-KB fixture, `e2e.auth_helpers.login`, the conftest `_wait_http`/`USE_REAL_LLM` imports; module docstring: story n/a — owner bug report 2026-09-16; the isolation command; what each test pins):
|
||||||
|
- **Fixture repo** (real `git` subprocesses, controlled `GIT_COMMITTER_DATE`/`GIT_AUTHOR_DATE` + fixed identity, the `sync_git_repo` helper shape — two commits, MID-YEAR dates so the browser's locale/TZ rendering of the year is stable in any timezone, the `test_document_dates.py` L455 lesson):
|
||||||
|
- commit one @ `2020-06-15T12:00:00Z` adds `old/old-note.md` (body: a sentence about an old, stable note).
|
||||||
|
- commit two (the tip) @ `2024-06-15T12:00:00Z` adds `recent/recent-note.md` (body: a sentence about a recent note).
|
||||||
|
- App env: `BOR_GIT_SOURCES=file://<repo>` + its own `BOR_SOURCES_DIR` (fresh dir, the `test_sync_button.py` L141-143 pattern); mock LLM; the session app (no git sources) is never started in this isolated run (no port clash).
|
||||||
|
- **Per-test sync helper:** admin login → the admin Sources page → click "Sync sources" → wait for the success detail (the `test_sync_button.py` lifecycle wait, generous timeout — real clone + mock-LLM embed).
|
||||||
|
- **Tests:**
|
||||||
|
1. `test_api_created_dates_are_true_per_file` — after a sync: `GET /api/docs` (admin, the page context's request client) — the row with `path == "old/old-note.md"` has `created_at[:10] == "2020-06-15"`, the row with `path == "recent/recent-note.md"` has `created_at[:10] == "2024-06-15"`, and the two values DIFFER (the regression assertion — the phase-106 bug made every URL-source file carry the tip date, i.e. both `2024-06-15`). `GET /api/docs/tree`: the file nodes carry those `created_at` verbatim; the `old` folder's `updated_at[:10] == "2020-06-15"`, the `recent` folder's `updated_at[:10] == "2024-06-15"`, the source node's `updated_at[:10] == "2024-06-15"` (subtree max, phase 106 D9 — now over TRUE dates).
|
||||||
|
2. `test_sources_tables_render_distinct_created_dates` — the RAG view's FILE table: the row whose path cell contains `old/old-note.md` has its `Created` cell (the column phase 106 D8 placed between `Chunks` and `Indexed`) matching a regex for the year `2020`; the `recent/recent-note.md` row's `Created` cell matches `2024`; the two cells' text differs. The FOLDER table: the `old` row's `Updated` cell (between `Documents` and `Description`) matches `2020`, the `recent` row's matches `2024`. (Year-regex assertions — `toLocaleString` rendering is locale/TZ-dependent; the year is stable for the mid-year fixture dates. `textContent` reads only — never set innerHTML.)
|
||||||
|
3. `test_viewer_created_badge_is_the_true_git_date` — open the old document (click its row/title, the phase-26 same-page modal): the viewer's `.doc-created` badge (phase 106, before the `Indexed` badge) has `title == "2020-06-15T12:00:00+00:00"` (the raw ISO — `metaBadge`'s title, deterministic under any locale/TZ, the house solution to the L455 rendering trap) and its visible text starts with `Created `; close the modal (Escape) — the viewer reverts cleanly.
|
||||||
|
4. `test_page_a11y_and_no_cdn_basics` — the standard light pass (AGENTS.md rules 5/6, the `test_git_sources_admin.py` a11y test shape): landmarks on the Sources view, the `Created`/`Updated` `<th>` cells present in both tables, ≥4.5:1-free text (no new color), 3px `:focus-visible` on a table row link, same-origin assets only (no external `src`/`href`).
|
||||||
|
- Fresh-KB fixture: truncate `documents`, `chunks`, `git_sources`, `sources_meta`, `kb_overview` (the `test_sync_button.py` `clean_kb` shape) so each test's sync counts are its own.
|
||||||
|
2. Run `uv run pytest tests/e2e/test_git_source_dates.py -v --no-cov` (DB up: `podman compose up -d db`) — all four tests green in isolation.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- E2E (the task IS the test): real `file://` URL-transport clone through `clone_or_pull` (task 01's full-history path) + the real importer + the real API + the real UI — the owner's scenario, transport-true.
|
||||||
|
- Coverage: the suite is `--no-cov`; it exercises `app/` (sync, docs API, tree builder) and `scripts/git_sync.py` for real — the `app/` >90% gate is unaffected.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/e2e/test_git_source_dates.py -v --no-cov` green in isolation (DB up) — all four tests
|
||||||
|
- [ ] The regression assertion holds: the two fixture documents' `created_at` values DIFFER (2020 vs 2024) in the API, the tables, and the viewer — the uniform-tip-date bug is provably gone for URL transports
|
||||||
|
- [ ] The suite leaves no other suite's fixtures touched (isolation: the session app never starts, own `BOR_SOURCES_DIR`, per-test DB reset)
|
||||||
@@ -0,0 +1,36 @@
|
|||||||
|
# Task 03 — Full gate: suites, coverage, lint/types, regression E2Es, atomic commit
|
||||||
|
|
||||||
|
**Phase:** `107_git_full_history_dates` · **Source:** AGENTS.md rules 8/9 — the test gates are non-negotiable; one atomic, professional commit per completed phase.
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Run the complete quality gate for the phase — unit + integration green, `app/` coverage >90%, the new E2E + the three phase-106/28/35 regression E2E suites green in isolation, ruff + pyright clean — then land the single `--no-gpg-sign` Conventional Commit and move the phase directory to `complete/`.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. DB up: `podman compose up -d db` (and `uv run alembic upgrade head` if the dev DB is behind — no NEW migration this phase, so head is unchanged).
|
||||||
|
2. Unit + integration: `uv run pytest` — green.
|
||||||
|
3. Coverage: `uv run pytest --cov=app --cov-report=term-missing` — TOTAL >90% (no `app/` code changed this phase; this is the regression check on the phase-106 suites + the new E2E's `app/` exercise).
|
||||||
|
4. E2E, each in isolation (`--no-cov`):
|
||||||
|
- the NEW suite: `uv run pytest tests/e2e/test_git_source_dates.py -v --no-cov`
|
||||||
|
- regressions (the three suites whose behavior this phase touches or that pin phase-106 dates end to end):
|
||||||
|
- `uv run pytest tests/e2e/test_document_dates.py -v --no-cov`
|
||||||
|
- `uv run pytest tests/e2e/test_sync_button.py -v --no-cov`
|
||||||
|
- `uv run pytest tests/e2e/test_git_sources_admin.py -v --no-cov`
|
||||||
|
5. Lint + types: `uv run ruff check . && uv run pyright` — clean.
|
||||||
|
6. UI Structure Check (AGENTS.md rule 5): the phase adds NO UI of its own — the date columns/badge asserted by task 02 are phase-106 UI, re-verified only; confirm nothing in `frontend/` changed this phase (`git status` shows none) so the byte-identical contracts are untouched.
|
||||||
|
7. Commit (exactly one atomic commit, `--no-gpg-sign`, the 00_phase.md message):
|
||||||
|
```bash
|
||||||
|
git add scripts/git_sync.py scripts/import_docs.py app/api/sync.py tests/ .agents/phases/ && git commit --no-gpg-sign -m "fix(git): full-history checkouts so URL sources get true per-file document dates"
|
||||||
|
```
|
||||||
|
8. Move the phase directory: `mv .agents/phases/todo/107_git_full_history_dates .agents/phases/complete/` (the pipeline gate does this on success — do it only after step 7 succeeds and include the move in the SAME commit's tree if the gate script does not, per the house protocol; check `.agents/pipeline.log` / the phased-execution gate for how prior phases recorded the move and match it).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- This task runs, not writes, the gate: every command above must pass before the commit exists.
|
||||||
|
- Coverage: **>90%** on `app/` (TOTAL line of the `term-missing` report).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%
|
||||||
|
- [ ] `uv run pytest tests/e2e/test_git_source_dates.py -v --no-cov` green in isolation (DB up)
|
||||||
|
- [ ] `tests/e2e/test_document_dates.py`, `tests/e2e/test_sync_button.py`, `tests/e2e/test_git_sources_admin.py` each green in isolation
|
||||||
|
- [ ] `uv run ruff check . && uv run pyright` clean
|
||||||
|
- [ ] Exactly one new commit, message `fix(git): full-history checkouts so URL sources get true per-file document dates`, signed with `--no-gpg-sign`; `git status` clean afterwards (only gitignored runtime artifacts aside)
|
||||||
|
- [ ] Phase dir at `.agents/phases/complete/107_git_full_history_dates/`
|
||||||
@@ -0,0 +1,56 @@
|
|||||||
|
# Phase 108 — History wire check: verify (or fix) the "missing first turn" follow-up bug
|
||||||
|
|
||||||
|
**Source:** `TODO.md` L4 (owner 2026-09-16): "I've noticed at least one instance where a follow-up chat is missing the first message and response as context. So if I ask 'What is my name' and then 'What did I just ask you?' the model responds 'This is the first question you've asked'. But if I send a third message 'What was the previous question' the model responds correctly 'What did I just ask you?' Just check if there's a bug, there may not be and this was user error"
|
||||||
|
|
||||||
|
**Story:** n/a (owner bug report, phase-74 follow-up — the phase's E2E proves the wire end to end).
|
||||||
|
|
||||||
|
**Context (traced 2026-09-16):** the chat-history wire landed in phase 74 and is three layers deep: (1) the CLIENT maps the `bor.chat.v1` conversation record minus the current question into the request body — `conversation.slice(0, -1)` → `{who, text, thinking?}` per turn (`frontend/assets/app.js` L2247, invariant comment L2229-2246 — the phase-49 retry and phase-53 stale-regen paths pop the old answer before re-sending, so `slice(0,-1)` is exactly the prior turns); (2) the SERVER trims + maps — `history_to_messages` (`app/rag/prompts.py` L199-252): walks NEWEST-FIRST, keeps turns while BOTH budgets hold (`history_max_turns` default **40**, `history_max_chars` default **24_000** — `app/config.py` L94/L101), drops a whole turn on overflow, returns the kept window chronological; `user`→user, `brain`→assistant with `reasoning_content` ONLY when thinking is non-empty (A4); (3) the ENDPOINT splices the block between the system prompt and the current user message on BOTH turn branches (deflected + grounded — `app/api/chat.py` L353-358, "BOTH branches below … reuse the same block"). A short 2-turn conversation is orders of magnitude under both budgets, and both the client mapping and the trimmer READ correctly — so this phase is a deterministic three-layer VERIFICATION with a built-in fix branch, not a rewrite. The wire oracle already exists: the mock LLM's `HISTORY_TRIGGER = "echo my history"` (`tests/e2e/mock_llm.py` L597; checked at L1671 BEFORE the DEFLECT_MODE branch — the echo fires on both branches) answers with `_history_echo(body)` (L1492): a byte-stable `history: N prior messages; last answer tail: <last 24 chars of the most recent prior assistant message, or "none">; thinking: yes|no` — exactly what the owner's scenario needs. The phase-74 E2E (`tests/e2e/test_llm_history.py`) already asserts on this echo, deriving the expected tail from the localStorage record; the existing suites to extend live at `tests/unit/test_history.py` (the pure trimmer) and `tests/integration/test_chat_api.py` (the `HISTORY`/`HISTORY_MESSAGES` idiom L1620-1656 + the `_stream_chat_with_history` helper L1659).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Prove — at the trimmer, the endpoint, and the full browser wire — that a follow-up question carries the COMPLETE prior conversation (the owner's exact 2- and 3-turn scenarios, byte-exact via the history echo), and either ship the minimal fix at the layer that reproduces the missing-first-turn symptom or record the verdict "no bug — model behavior/user error" with the pins as the permanent guard.
|
||||||
|
|
||||||
|
## Dependencies
|
||||||
|
- `74_llm_chat_history` (complete) — the feature under verification: the `history` request field, the trimmer, both-branch splicing, the echo marker, and the suites this phase extends. All its pins are regression gates.
|
||||||
|
- `17_thinking_display` (complete) — the record's `thinking` key and the A4 `reasoning_content` wire convention the echo's `thinking: yes|no` term covers.
|
||||||
|
- `49_retry_answer` / `53_stale_saved_chats` (complete) — the client paths (retry, stale-regen) that POP the old answer before re-sending; the client-invariant comment names them — if the client layer ever reproduces, their pop logic is the first suspect.
|
||||||
|
|
||||||
|
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||||
|
|
||||||
|
- **The three layers, each isolating a suspect (tasks 01-02):**
|
||||||
|
1. **Unit — the trimmer** (task 01): the owner's exact shape — a 2-turn history (user Q1, brain R1) under the default budgets → ALL turns kept, chronological, roles mapped, thinking mapped (non-empty → `reasoning_content`, empty/absent → key absent). If this fails, the bug is in `history_to_messages` and nothing else needs running.
|
||||||
|
2. **Integration — the endpoint** (task 01): the SAME 2-turn history through the real `POST /api/chat` (the `test_chat_api.py::_stream_chat_with_history` idiom): the SSE turn completes AND the LLM request the turn made carries exactly `[system, user Q1, assistant R1, user Q2]` (captured per the house fake-LLM pattern). If layer 1 passes and this fails, the bug is in the endpoint plumbing (the `request.history` → `hist` → prompt splice, one of the two branches).
|
||||||
|
3. **E2E — the full client wire** (task 02): the owner's exact 3-message scenario in the browser, echo markers on turns 2 and 3 (see task 02 for the messages + expected echoes). If layers 1-2 pass and this fails, the bug is in the CLIENT record→history mapping (push/pop timing, the phase-49/53 paths, localStorage restore).
|
||||||
|
- **The verdict (task 03, D13):** all three green → NO BUG: the wire is proven complete at every layer; the reported instance is model behavior/user error (the owner's own hypothesis). The pins stay as the permanent guard (a future regression that drops the first turn fails layer 1, 2, or 3). A failure at layer N → the bug reproduces at layer N; the executor makes the MINIMAL fix in that layer, re-runs the failing layer green, and the verdict records the fix + evidence. `VERDICT.md` (NEW file inside this phase dir) is written BEFORE the commit and states: the layer outcomes, the verdict, and (if fixed) the one-line root cause.
|
||||||
|
- **NOT touched (D13/D14):** the A10 stateless contract, the budget defaults (40 turns / 24k chars), the `bor.chat.v1` record schema, the echo's format (the existing marker IS the oracle — D14: NO new mock marker this phase), any phase-74 pin (regression), `PLAN.md`, completed phases.
|
||||||
|
|
||||||
|
## Tasks
|
||||||
|
1. `01_server_wire_verification.md` — layer 1 (unit pins on the trimmer for the owner's 2-turn shape) + layer 2 (integration: real endpoint, captured LLM request = full prior history).
|
||||||
|
2. `02_client_e2e_owner_scenario.md` — layer 3: new E2E `tests/e2e/test_history_wire_check.py` (isolation) — the owner's exact 3-message scenario, byte-exact echo assertions on turns 2 and 3.
|
||||||
|
3. `03_verdict_fix_or_pin.md` — read the layer outcomes; fix the reproducing layer (or record "no bug"); `VERDICT.md`; full gate; atomic commit.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit — `tests/unit/test_history.py` (extended): the 2-turn-under-budget keep-all pin + the role/thinking mapping for that shape (the existing budget/trim pins stay green — regression).
|
||||||
|
- Integration — `tests/integration/test_chat_api.py` (extended): the 2-turn request through the real endpoint with the captured-LLM-request assertion (the house fake-LLM capture pattern; the existing phase-74 history tests stay green).
|
||||||
|
- E2E (mandatory, A16) — `uv run pytest tests/e2e/test_history_wire_check.py -v --no-cov` with the DB up.
|
||||||
|
- Coverage: **>90%** on `app/` (`uv run pytest --cov=app --cov-report=term-missing` — the validate.sh gate).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] Layer 1 green: a 2-turn history under the default budgets survives `history_to_messages` whole, chronological, correctly mapped (unit pin).
|
||||||
|
- [ ] Layer 2 green: a real `POST /api/chat` with a 2-turn history makes the LLM request `[system, user Q1, assistant R1, user Q2]` — the server wire is proven complete (or the bug is fixed here).
|
||||||
|
- [ ] Layer 3 green: the owner's scenario in the browser — turn 2's echo shows `history: 2 prior messages` + R1's exact 24-char tail; turn 3's echo shows `history: 4 prior messages` + R2's tail (or the bug is fixed at the client).
|
||||||
|
- [ ] `VERDICT.md` exists in the phase dir: layer outcomes + the verdict (fixed-at-layer-N with root cause, or "no bug — model behavior") — written before the commit.
|
||||||
|
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run pytest tests/e2e/test_history_wire_check.py -v --no-cov` green in isolation; `uv run pytest tests/e2e/test_llm_history.py -v --no-cov` (phase 74) green in isolation; `uv run ruff check . && uv run pyright` clean.
|
||||||
|
- [ ] One `--no-gpg-sign` commit (message per the verdict — see Commit); phase dir moved to `.agents/phases/complete/` by the pipeline gate.
|
||||||
|
|
||||||
|
## Locked decisions
|
||||||
|
- **D13 — Verify-or-fix protocol (owner-instructed: "Just check if there's a bug, there may not be").** The phase's deliverable is the three-layer pins + a recorded verdict. Code changes happen ONLY when a layer reproduces the missing-turn symptom, are MINIMAL, and are confined to the reproducing layer — no A10 contract change, no budget-default change, no record-schema change, no new endpoint. If no layer reproduces, the phase ships tests-only.
|
||||||
|
- **D14 — The existing echo IS the oracle.** The phase-74 `echo my history` marker (`_history_echo`) is reused unmodified — its `N prior messages` count + `last answer tail` are exactly the owner-scenario assertions; NO new mock marker is added this phase (new markers land only in phases that change prompt/tool shapes).
|
||||||
|
|
||||||
|
## Commit
|
||||||
|
```bash
|
||||||
|
# verdict = no bug (tests-only):
|
||||||
|
git add tests/ .agents/phases/ && git commit --no-gpg-sign -m "test(chat): history-wire verification pins — TODO L4 verdict: no bug (model behavior)"
|
||||||
|
|
||||||
|
# verdict = bug found (adjust <layer> to the fix site):
|
||||||
|
git add <fixed files> tests/ .agents/phases/ && git commit --no-gpg-sign -m "fix(chat): <layer> — follow-up turns carry the full prior history (TODO L4)"
|
||||||
|
```
|
||||||
@@ -0,0 +1,29 @@
|
|||||||
|
# Task 01 — Server wire verification: the trimmer (unit) + the endpoint (integration)
|
||||||
|
|
||||||
|
**Phase:** `108_history_wire_check` · **Source:** `TODO.md` L4 — "a follow-up chat is missing the first message and response as context … Just check if there's a bug."
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Prove (or disprove) the two SERVER layers of the history wire for the owner's exact 2-turn shape: a short history must survive `history_to_messages` whole and reach the LLM as the complete prior conversation on the real `POST /api/chat`.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `tests/unit/test_history.py` (EXISTING — extend, keep every pin green) — add the owner-shape pins for the DEFAULT budgets (no env overrides; construct `Settings` the file's existing `_settings()` way):
|
||||||
|
- `test_short_two_turn_history_kept_whole_and_chronological` — `history = [user "What is my name?", brain "Your name is Reese."]` (the owner's own Q1/R1) → `history_to_messages` returns exactly `[{"role": "user", "content": "What is my name?"}, {"role": "assistant", "content": "Your name is Reese."}]` — both turns, chronological, no trim, no reordering.
|
||||||
|
- `test_two_turn_history_thinking_mapping` — the same 2-turn history with the brain turn carrying a non-empty `thinking` → the assistant message gains `reasoning_content` (A4); with `thinking` empty/absent → the key is ABSENT (not an empty string).
|
||||||
|
- (If either pin fails: STOP — layer 1 reproduces the bug. Fix `app/rag/prompts.py::history_to_messages` minimally (D13), keep this task's pins + the existing suite green, and note the root cause for task 03's `VERDICT.md`. Do not touch the budget defaults.)
|
||||||
|
2. `tests/integration/test_chat_api.py` (EXISTING — extend next to the phase-74 history block, L1620-1670) — add the endpoint-layer pin:
|
||||||
|
- Reuse the file's `_stream_chat`/`_stream_chat_with_history` helpers + fake-LLM capture pattern (read the file's existing setup first — match its house idiom for capturing what the LLM was called with).
|
||||||
|
- `test_endpoint_two_turn_history_reaches_the_llm` — `POST /api/chat {message: "What did I just ask you?", history: [{who: user, text: "What is my name?"}, {who: brain, text: "Your name is Reese."}]}` → the SSE stream completes (`done`), and the chat request the turn made to the LLM carries, IN ORDER, the system prompt, `user "What is my name?"`, `assistant "Your name is Reese."`, then the current `user` question — i.e. the 2 prior turns are NOT dropped (the owner's symptom would be their absence). Assert on the captured `messages` list (roles + contents, exact).
|
||||||
|
- The turn may be LOW/deflected with an empty KB (the history block is branch-independent — pinned in phase 74) or HIGH with one seeded fixture doc (the file's existing seeding idiom) — either is fine; pick what the file's helpers make easiest and say so in a comment.
|
||||||
|
- (If this fails while layer 1 passed: the bug is in the endpoint plumbing — `app/api/chat.py`'s `request.history` → `hist` → prompt splice. Fix minimally (D13), keep this pin + the phase-74 pins green, note the root cause for task 03.)
|
||||||
|
3. Run `uv run pytest tests/unit/test_history.py tests/integration/test_chat_api.py -v` (DB up: `podman compose up -d db`) — green.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: the owner-shape trimmer pins (keep-all + mapping) alongside the existing budget pins.
|
||||||
|
- Integration: the real endpoint with a captured LLM request — the server wire proven (or fixed) at the exact layer.
|
||||||
|
- Coverage: **>90%** on `app/` (no `app/` change unless a fix is needed; the gate still passes).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `tests/unit/test_history.py` green with the two new owner-shape pins (or the trimmer fixed + pinned)
|
||||||
|
- [ ] `tests/integration/test_chat_api.py` green with `test_endpoint_two_turn_history_reaches_the_llm` (or the endpoint fixed + pinned)
|
||||||
|
- [ ] Every pre-existing pin in both files still green (no regression)
|
||||||
|
- [ ] `uv run ruff check . && uv run pyright` clean; the layer-1/layer-2 outcome is noteable for task 03's `VERDICT.md` (pass, or pass-after-fix with root cause)
|
||||||
@@ -0,0 +1,29 @@
|
|||||||
|
# Task 02 — Client E2E: the owner's exact 3-message scenario, byte-exact via the history echo
|
||||||
|
|
||||||
|
**Phase:** `108_history_wire_check` · **Source:** `TODO.md` L4 — the owner's repro: Q1 "What is my name?" → Q2 "What did I just ask you?" (model claims it's the first question) → Q3 "What was the previous question?" (model answers correctly).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Prove (or disprove) the THIRD layer — the full browser wire: the localStorage conversation record → `conversation.slice(0,-1)` mapping → request body → the LLM — using the owner's exact scenario and the phase-74 `echo my history` oracle (D14: no new marker).
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `tests/e2e/test_history_wire_check.py` (NEW — copy the app-server + fixture idiom from `tests/e2e/test_llm_history.py`: module-scoped mock-LLM app, the fixture-docs import for a non-empty KB, `e2e.auth_helpers.login`, the localStorage `bor.chat.v1` record reads, per-test conversation reset; module docstring: story n/a — owner bug report 2026-09-16, the isolation command, and what each test pins). Isolation: `uv run pytest tests/e2e/test_history_wire_check.py -v --no-cov` (DB up).
|
||||||
|
- **The echo oracle, recalled** (mock `_history_echo`, byte-stable): `history: N prior messages; last answer tail: <LAST 24 CHARS of the most recent prior assistant message's content, or "none">; thinking: yes|no` — N = non-system messages before the LAST user message (the current question excluded); checked BEFORE the DEFLECT_MODE branch, so the echo fires whatever gate branch the turn takes (the owner's questions may deflect — that's fine, the marker is in the USER message).
|
||||||
|
- **Tests:**
|
||||||
|
1. `test_cold_start_echo_shows_no_phantom_history` — fresh conversation; ask `echo my history` as the FIRST message → the answer bubble contains `history: 0 prior messages; last answer tail: none; thinking: no` (the cold-start pin: no phantom prior turns).
|
||||||
|
2. `test_owner_scenario_three_turns_carry_the_full_prior_history` — the owner's exact scenario, echo marker APPENDED to turns 2 and 3 (their words preserved verbatim as the prefix):
|
||||||
|
- T1: `What is my name?` → R1 (the mock's deterministic answer — read R1's raw text from the `bor.chat.v1` record's brain entry, NOT from the rendered DOM).
|
||||||
|
- T2: `What did I just ask you? echo my history` → R2's bubble text must contain `history: 2 prior messages; last answer tail: {R1[-24:]}; thinking: no` (R1 = the record's brain text; `thinking: no` — T1 never triggered the thinking marker). **THE regression pin: the owner's bug renders this as `0 prior messages` / `last answer tail: none`.**
|
||||||
|
- T3: `What was the previous question? echo my history` → R3's bubble text must contain `history: 4 prior messages; last answer tail: {R2[-24:]}` (R2 = the echo answer itself — also from the record).
|
||||||
|
- Read the expected tails from the localStorage record AFTER each turn persists (the `test_llm_history.py` pattern — the record the client saved IS what the client sends next, so what the record shows is what the model received).
|
||||||
|
- If this test fails while tasks 01's layers passed: the bug is in the CLIENT mapping (suspects, in order: the `conversation.slice(0,-1)` sites, the phase-49 retry / phase-53 stale-regen pop paths, the record persistence timing — `frontend/assets/app.js` L2187/L2247). Fix minimally (D13), keep this test + `tests/e2e/test_llm_history.py` green, note the root cause for task 03's `VERDICT.md`.
|
||||||
|
2. Run the suite in isolation — green (or pass-after-fix).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- E2E (the task IS the test): the full browser wire, byte-exact via the existing echo oracle.
|
||||||
|
- Coverage: `--no-cov` suite; it exercises `app/` (chat endpoint, history mapping) for real — the `app/` >90% gate is unaffected.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/e2e/test_history_wire_check.py -v --no-cov` green in isolation (DB up) — both tests
|
||||||
|
- [ ] The regression pin holds: turn 2's echo shows `2 prior messages` + R1's exact tail; turn 3 shows `4 prior messages` + R2's exact tail (or the client bug is fixed + pinned)
|
||||||
|
- [ ] `tests/e2e/test_llm_history.py` (phase 74) still green in isolation (regression)
|
||||||
|
- [ ] The layer-3 outcome is noteable for task 03's `VERDICT.md` (pass, or pass-after-fix with root cause)
|
||||||
@@ -0,0 +1,34 @@
|
|||||||
|
# Task 03 — Verdict (fix or pin), full gate, atomic commit
|
||||||
|
|
||||||
|
**Phase:** `108_history_wire_check` · **Source:** `TODO.md` L4 — "Just check if there's a bug, there may not be and this was user error"; AGENTS.md rules 8/9 — the test gates are non-negotiable.
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Record the phase's verdict with its evidence, run the complete quality gate, and land the single `--no-gpg-sign` commit — tests-only if no bug was found (the owner's expected outcome), fix + tests if one layer reproduced.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. **The verdict** — read the layer outcomes from tasks 01-02 (their test results + any fix notes):
|
||||||
|
- **All three layers green (no fix needed):** the wire is proven complete at the trimmer, the endpoint, and the full browser wire → verdict **NO BUG**: the owner's reported instance was model behavior/user error. The pins stay as the permanent guard (a future regression that drops the first turn fails layer 1, 2, or 3).
|
||||||
|
- **A layer reproduced (pass-after-fix):** the bug is fixed at that layer → verdict **BUG FOUND + FIXED at <layer>**, with the one-line root cause.
|
||||||
|
- Write `.agents/phases/todo/108_history_wire_check/VERDICT.md` BEFORE the commit: the three layer outcomes (pass / pass-after-fix + root cause / fail-should-not-occur), the verdict, and the evidence (which test names carry the pins). Keep it short — it is the durable record the owner asked for ("just check").
|
||||||
|
2. **The full gate** (DB up: `podman compose up -d db`; every command must pass before the commit):
|
||||||
|
- `uv run pytest` — green.
|
||||||
|
- `uv run pytest --cov=app --cov-report=term-missing` — TOTAL >90%.
|
||||||
|
- E2E in isolation: `uv run pytest tests/e2e/test_history_wire_check.py -v --no-cov` (NEW) and `uv run pytest tests/e2e/test_llm_history.py -v --no-cov` (phase-74 regression).
|
||||||
|
- `uv run ruff check . && uv run pyright` — clean.
|
||||||
|
- No-regression spot check: `git diff --stat` shows ONLY the files this phase may touch — `tests/**`, `VERDICT.md`, `.agents/phases/**`, and (only if a bug was fixed) the single reproducing layer's file. If the diff shows anything else, stop and fix the scope before committing.
|
||||||
|
3. **The commit** (exactly one, `--no-gpg-sign`, per the 00_phase.md branch):
|
||||||
|
- no bug: `git add tests/ .agents/phases/ && git commit --no-gpg-sign -m "test(chat): history-wire verification pins — TODO L4 verdict: no bug (model behavior)"`
|
||||||
|
- bug found: `git add <fixed files> tests/ .agents/phases/ && git commit --no-gpg-sign -m "fix(chat): <layer> — follow-up turns carry the full prior history (TODO L4)"`
|
||||||
|
4. Move the phase directory: `mv .agents/phases/todo/108_history_wire_check .agents/phases/complete/` (the pipeline gate does this on success — do it only after the commit, and match how prior phases recorded the move).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- This task runs, not writes, the gate: every command above must pass before the commit exists.
|
||||||
|
- Coverage: **>90%** on `app/` (TOTAL line of the `term-missing` report).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `VERDICT.md` in the phase dir: layer outcomes + verdict (no bug / fixed-at-<layer> + root cause) + the pin test names
|
||||||
|
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%
|
||||||
|
- [ ] `tests/e2e/test_history_wire_check.py` + `tests/e2e/test_llm_history.py` green in isolation (DB up)
|
||||||
|
- [ ] `uv run ruff check . && uv run pyright` clean; the diff is scoped to this phase's allowed files
|
||||||
|
- [ ] Exactly one new commit with the verdict-branch message, `--no-gpg-sign`; `git status` clean afterwards (only gitignored runtime artifacts aside)
|
||||||
|
- [ ] Phase dir at `.agents/phases/complete/108_history_wire_check/`
|
||||||
@@ -0,0 +1,52 @@
|
|||||||
|
# Phase 109 — Never-frozen turn: re-expanding thinking block + the persistent in-turn loader
|
||||||
|
|
||||||
|
**Source:** `TODO.md` L3 (owner 2026-09-16): "Thinking can happen after the model starts responding. This sometimes results in a the chat appearing 'frozen' because the model responds, calls a tool, then continues thinking without re-expanding the thinking block. There should be a visual that the chat is still progressing regardless of what state it's in (some kind of loader will do)."
|
||||||
|
|
||||||
|
**Story:** n/a (owner request — extends the phase 17/48/87 thinking/tool feedback under the PLAN §7.4 never-stale contract; the phase's E2E proves the reported repro no longer freezes).
|
||||||
|
|
||||||
|
**Context (traced 2026-09-16):** the turn's visible feedback is state-driven in `frontend/assets/app.js`: the `UI_STATE` machine (L340-345: `idle`/`thinking`/`streaming`/`error`) is owned by `setUiState` (L1243) — the typing bubble (a `#typing-indicator` message with the animated `.bubble.typing` dots, `addTyping` L859, the 10s elapsed-seconds clock L1137) shows ONLY in `thinking`; `inFlight = thinking|streaming` drives the Stop button (L1251-1255); `#send-status` is the sole a11y live region (L347-353; visual elements are `aria-hidden` — the L1791 house pattern). The reported freeze, exactly as the owner described it: the `delta` handler (L2357-2370) runs `setUiState(streaming)` on the FIRST delta — which `removeTyping()`s the dots — then `closeThinkingBlock(wrap)` (L905-909: "auto-collapse; idempotent, **never reopens**"). A LATER `thinking` frame (the next agent round — the model answered, called a tool, then thinks again) hits the `thinking` handler (L2268-2301), which only appends to the collapsed block's `.thinking-text` — nothing is visible: the answer text is static, the dots are gone, the scratchpad is closed → the chat reads as frozen. Every OTHER state already has a cue: pre-delta thinking = live open block + dots; tool = the `.tool-call` line with the phase-87 `(Ns)` elapsed counter (plus relabeled dots pre-delta); streaming = growing text; retry = the status line. The post-delta thinking gap is the ONE uncovered state — and the owner wants a constant cue anyway ("regardless of what state it's in"). The CSS lives in `frontend/assets/styles.css` (the typing-dots rules there; `prefers-reduced-motion` is house law, §7.2). House patterns: unit pins read the assets as text (`tests/unit/test_frontend_tool_states.py` / `test_frontend_feedback.py` already pin the thinking-block + typing behavior); the mock LLM has `THINKING_TRIGGER = "think out loud"` (mock_llm.py L500 — streams ~700 chars of `reasoning_content` ahead of content) and the multi-round `tool_calls` markers (L77-191); the house rule is that a marker/regex change lands WITH its consuming task (PLAN §4).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
The chat never reads as frozen: (1) a `thinking` frame RE-OPENS the thinking block after the answer has started (the block is open-while-thinking / closed-while-answering — the reported symptom, fixed at the handler), and (2) a compact persistent loader is visible for the ENTIRE active turn (send → terminal frame) in the composer status area — the constant progress cue the owner asked for, driven by the single `setUiState` owner so it can never go stale (§7.4).
|
||||||
|
|
||||||
|
## Dependencies
|
||||||
|
- `17_thinking_display` (complete) — the thinking block, the follow-the-tail pin contract (`THINKING_NEAR_BOTTOM_PX`), the restore-path collapsed rendering; the block's unit pins live in the `test_frontend_*` suites this phase extends.
|
||||||
|
- `48_stop_generation` (complete) — the `inFlight`/Stop-button state ownership the loader's single-owner toggle joins.
|
||||||
|
- `87_big_read_progress` (complete) — the `.tool-call` line + the `armToolLineClock`/`settleToolLine` `(Ns)` counter (the "at least one cue" inventory's tool entry).
|
||||||
|
- `06_loading_feedback` (complete) — the UI state machine + the never-stale feedback contract this phase extends (every state keeps a defined UI).
|
||||||
|
|
||||||
|
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||||
|
|
||||||
|
- **D15 — The thinking block becomes a TOGGLE (task 01).** The `thinking` SSE handler gains `block.open = true` after `ensureThinkingBlock(wrap)` (idempotent — a no-op while already open, so the pre-delta live flow is byte-identical in behavior); the `delta` handler KEEPS its `closeThinkingBlock(wrap)`. The contract flips from "never reopens" to **open-while-thinking, closed-while-answering** — the block reflects the model's current activity in every agent round. `closeThinkingBlock`'s docstring/comment updates (the "never reopens" claim is gone — the delta handler closes, the thinking handler opens). The follow-the-tail pin logic (L2293: `block.open && isThinkingNearBottom(textEl)` measured BEFORE the re-render) is UNCHANGED — it already keys off `block.open`, so a re-opened block resumes pinned tail-following exactly like the live pre-delta block. The phase-14 RESTORE path (`renderStoredMessage` L1517-1518) still renders stored blocks collapsed — untouched.
|
||||||
|
- **D16 — The turn loader (task 02): a static shell element, single-owner visibility.**
|
||||||
|
- `frontend/index.html` — ONE static element in the composer's status row (next to the `#send-status` live region): `<div id="turn-loader" class="turn-loader" aria-hidden="true" hidden></div>` — static markup, hidden by default (no JS-built HTML — the createElement/textContent house rule; no document-derived data anywhere near it).
|
||||||
|
- `frontend/assets/app.js` — `setUiState` (L1243) is the SOLE owner, exactly like the existing `is-stop` toggle: `turnLoader.hidden = !inFlight` (shown iff `uiState ∈ {thinking, streaming}`). Every terminal path funnels through `setUiState` (done → `idle`, error → `error`, stop/timeout → `error`/`idle` per the existing handlers), so the loader CANNOT be left visible in a terminal state — the §7.4 never-stale guarantee comes from the single-owner pattern, not from per-handler cleanup.
|
||||||
|
- `frontend/assets/styles.css` — `.turn-loader` next to the typing-dots rules: compact, reuses the EXISTING typing-dot animation (same keyframes/dot styling — no new animation family), provenance comment citing phase 109 + `TODO.md` L3, and a `prefers-reduced-motion` variant mirroring the typing dots' treatment (static dots, no pulse). Contrast N/A (the dots are decorative — `aria-hidden` + the `#send-status` announcer carry meaning; §7.2 "text + color, never color alone" — the state TEXT stays in `#send-status`).
|
||||||
|
- **The invariant (unit + E2E):** while a turn is active, at least one visible progress cue is ALWAYS present — the loader (constant, D16), the open thinking block (thinking frames, D15), the tool-line `(Ns)` counter (tool frames, phase 87), or the growing answer text (streaming). D16 makes it true by construction; task 03 proves the reported repro (delta → tool → thinking-after-delta) no longer freezes.
|
||||||
|
- **NOT touched:** the `UI_STATE` set, the `SEND_STATUS` copy, the typing bubble's own lifecycle (it still shows only pre-delta, per the phase-17 contract — the loader is a SEPARATE constant cue, not a re-homing of the dots), `#send-status` (unchanged — still the sole a11y announcer), the mock's EXISTING markers, the server (this is a pure UI phase — `app/` is untouched, so `app/` coverage is a regression check only), `PLAN.md`, completed phases.
|
||||||
|
|
||||||
|
## Tasks
|
||||||
|
1. `01_thinking_block_reexpand.md` — the `thinking` handler re-opens the collapsed block; the delta handler keeps closing; the "never reopens" narrative updated; unit pins (read-the-assets pattern).
|
||||||
|
2. `02_turn_active_loader.md` — the static `#turn-loader` element, the `setUiState` single-owner toggle, the CSS (reused dot animation + reduced-motion + provenance), unit pins.
|
||||||
|
3. `03_e2e_and_gate.md` — the new mock marker forcing the reported `delta → tool → thinking-after-delta` sequence (lands WITH this task), the dedicated E2E `tests/e2e/test_turn_progress_loader.py` (isolation), the a11y pass, the full gate, the atomic commit.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit — `tests/unit/test_frontend_turn_loader.py` (NEW, the house read-the-assets-as-text pattern): task 01 pins — the `thinking` handler contains the `block.open = true` re-open (after `ensureThinkingBlock`), the `delta` handler still calls `closeThinkingBlock`, `closeThinkingBlock`'s docstring no longer claims "never reopens", the restore path (`renderStoredMessage`) still sets `block.open = false`; task 02 pins — `index.html` carries exactly one `#turn-loader` with `aria-hidden="true"` + `hidden`, `setUiState` is the SOLE writer of `turnLoader.hidden` (cross-file single-owner check: `turnLoader.hidden` appears nowhere else in `app.js`), the `.turn-loader` CSS rule exists next to the typing rules with the reduced-motion variant + the phase-109 provenance comment.
|
||||||
|
- E2E (mandatory, A16) — `tests/e2e/test_turn_progress_loader.py` (task 03): `uv run pytest tests/e2e/test_turn_progress_loader.py -v --no-cov` with the DB up.
|
||||||
|
- Coverage: **>90%** on `app/` (`uv run pytest --cov=app --cov-report=term-missing` — unchanged by this UI phase; the gate is the regression check).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] A `thinking` frame after the answer has started RE-OPENS the thinking block (the block is open-while-thinking / closed-while-answering; pre-delta flow + restore path unchanged — unit-pinned).
|
||||||
|
- [ ] `#turn-loader` is visible for the entire active turn and hidden in every terminal state — owned solely by `setUiState` (unit-pinned single-owner + the E2E's start/mid/end samples).
|
||||||
|
- [ ] E2E green in isolation: the reported repro (delta → tool → thinking-after-delta) shows the re-opened scratchpad with the new thinking text, the loader visible throughout, hidden after `done`; `#send-status` carries the state text (the loader is `aria-hidden`).
|
||||||
|
- [ ] The phase-17/48/87/6 regressions green in isolation: `uv run pytest tests/e2e/test_thinking_display.py -v --no-cov`, `tests/e2e/test_stop_generation.py -v --no-cov`, `tests/e2e/test_big_read_progress.py -v --no-cov`, `tests/e2e/test_loading_feedback.py -v --no-cov`; `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||||
|
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agents/phases/complete/` by the pipeline gate.
|
||||||
|
|
||||||
|
## Locked decisions
|
||||||
|
- **D15 — The thinking block is a toggle, not a one-way door (owner-instructed: "continues thinking without re-expanding the thinking block" is THE reported defect).** `thinking` frames open the block (idempotent), `delta` frames close it; the follow-the-tail pin contract and the restore path are unchanged. No new block, no new state — the existing scratchpad reflects the model's current activity in every round.
|
||||||
|
- **D16 — The constant cue is a separate static loader, owned by `setUiState` (owner-instructed: "a visual that the chat is still progressing regardless of what state it's in — some kind of loader will do").** A static `#turn-loader` in the composer status row (reused typing-dot animation, `aria-hidden`, `#send-status` stays the sole announcer), shown iff `inFlight` — the single-owner pattern makes a stale loader impossible. The typing bubble's own pre-delta lifecycle is NOT re-homed (the phase-17 contract stands); the loader ADDS the constant the owner asked for.
|
||||||
|
|
||||||
|
## Commit
|
||||||
|
```bash
|
||||||
|
git add frontend/index.html frontend/assets/app.js frontend/assets/styles.css tests/ .agents/phases/ && git commit --no-gpg-sign -m "feat(chat): never-frozen turn — re-expanding thinking block + the persistent in-turn loader"
|
||||||
|
```
|
||||||
@@ -0,0 +1,28 @@
|
|||||||
|
# Task 01 — The thinking block becomes a toggle: re-open on `thinking` frames, close on `delta`
|
||||||
|
|
||||||
|
**Phase:** `109_turn_progress_loader` · **Source:** `TODO.md` L3 — "the model responds, calls a tool, then continues thinking without re-expanding the thinking block."
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Fix the reported freeze at its source: a `thinking` frame that arrives after the answer has started re-opens the collapsed thinking block (D15) — the scratchpad is visible exactly while the model is thinking, in every agent round — while the `delta` handler keeps closing it and nothing else about the block's lifecycle changes.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `frontend/assets/app.js` — the `thinking` SSE handler (L2268-2301): after `const block = ensureThinkingBlock(wrap);`, add `block.open = true;` (idempotent — while the block is already open (the pre-delta live flow) this is a no-op, so that flow's behavior is unchanged; after a `delta` closed it, this re-opens it for the new round's thinking). Update the handler's comment to state the toggle contract (D15): open-while-thinking, closed-while-answering — phase 109, `TODO.md` L3.
|
||||||
|
- The follow-the-tail logic below it (`const pinned = block.open && isThinkingNearBottom(textEl);`, L2293) is UNCHANGED — it already reads `block.open` before the re-render, so a re-opened block resumes pinned tail-following exactly like the live pre-delta block.
|
||||||
|
2. `frontend/assets/app.js` — `closeThinkingBlock` (L905-909): keep the function exactly as-is (the `delta` handler still calls it, L2369); update its docstring/comment — the "never reopens" claim is replaced by the toggle contract (the `thinking` handler re-opens; the `delta` handler closes).
|
||||||
|
- `renderStoredMessage` (L1517-1518 — the phase-14 restore path) is UNTOUCHED: stored blocks still render collapsed.
|
||||||
|
3. `tests/unit/test_frontend_turn_loader.py` (NEW — the house read-the-assets-as-text pattern; copy the file header/docstring conventions from `tests/unit/test_frontend_tool_states.py`):
|
||||||
|
- `test_thinking_handler_reopens_the_collapsed_block` — the `thinking` handler source contains `block.open = true` positioned AFTER the `ensureThinkingBlock(wrap)` line (order asserted — the block must exist before it opens).
|
||||||
|
- `test_delta_handler_still_closes_the_block` — the `delta` handler still calls `closeThinkingBlock(wrap)` (the close side of the toggle survives).
|
||||||
|
- `test_close_thinking_block_docstring_says_toggle_not_one_way` — `closeThinkingBlock`'s comment no longer contains "never reopens"; the toggle contract (open-while-thinking / closed-while-answering) is documented (assert the new wording, e.g. it names the `thinking` handler's re-open).
|
||||||
|
- `test_restore_path_still_collapses_stored_blocks` — `renderStoredMessage` still sets `block.open = false` (phase-14 contract regression).
|
||||||
|
- Cross-file: the `THINKING_NEAR_BOTTOM_PX` pin + `isThinkingNearBottom` logic are untouched (assert the pre-render `block.open &&` guard line still exists in the handler).
|
||||||
|
4. Run `uv run pytest tests/unit/test_frontend_turn_loader.py -v` + the existing frontend suites that pin this area (`tests/unit/test_frontend_tool_states.py tests/unit/test_frontend_feedback.py tests/unit/test_frontend_scroll.py tests/unit/test_big_read_progress.py`) — green.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: the toggle contract pinned at the source level (the house pattern for `app.js` behavior — no browser).
|
||||||
|
- Coverage: **>90%** on `app/` (unchanged — pure frontend task; the gate is the regression check).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] The `thinking` handler re-opens the block (unit-pinned, correct order); the `delta` handler still closes it; the "never reopens" narrative is gone (unit-pinned)
|
||||||
|
- [ ] The restore path + the follow-the-tail pin logic are untouched (unit-pinned regressions)
|
||||||
|
- [ ] The existing frontend unit suites green; `uv run ruff check . && uv run pyright` clean
|
||||||
@@ -0,0 +1,37 @@
|
|||||||
|
# Task 02 — The persistent in-turn loader: static `#turn-loader`, single-owner visibility in `setUiState`
|
||||||
|
|
||||||
|
**Phase:** `109_turn_progress_loader` · **Source:** `TODO.md` L3 — "There should be a visual that the chat is still progressing regardless of what state it's in (some kind of loader will do)."
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Add the constant progress cue (D16): a compact animated loader in the composer status row that is visible for the ENTIRE active turn (send → terminal frame), owned solely by `setUiState` so it can never be left stale — and hidden in every terminal state by construction.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `frontend/index.html` — ONE static element in the composer's status row (the L290-300 region holding `#char-count` / `#send-btn` / `#send-status` — inspect the actual markup and place it adjacent to `#send-status` so it reads as the status line's companion):
|
||||||
|
```html
|
||||||
|
<div id="turn-loader" class="turn-loader" aria-hidden="true" hidden></div>
|
||||||
|
```
|
||||||
|
Static markup, `hidden` by default (idle on load). No JS-built HTML anywhere (the createElement/textContent house rule — this element is never constructed in JS).
|
||||||
|
2. `frontend/assets/app.js` — the single-owner toggle (D16):
|
||||||
|
- At the top of the module with the other element lookups (near `const sendStatus = document.querySelector("#send-status");` L307): `const turnLoader = document.querySelector("#turn-loader");`
|
||||||
|
- Inside `setUiState` (L1243), next to the existing `sendBtn.classList.toggle("is-stop", inFlight);` (L1251-1255): `turnLoader.hidden = !inFlight;` — shown iff `uiState ∈ {thinking, streaming}`. This is the SOLE writer of `turnLoader.hidden` in the file: every terminal path (done → `idle`, error → `error`, stop/timeout → the existing error/idle landings) funnels through `setUiState`, so the loader is hidden in every terminal state BY CONSTRUCTION — the §7.4 never-stale guarantee, no per-handler cleanup (that is the point; comment it that way).
|
||||||
|
- Do NOT touch the typing bubble's lifecycle (`addTyping`/`removeTyping` stay exactly as-is — the phase-17 pre-delta contract stands; the loader is a separate constant cue), the `SEND_STATUS` copy, or `#send-status` (still the sole a11y announcer — the loader is `aria-hidden` decoration, the L1791 house pattern).
|
||||||
|
3. `frontend/assets/styles.css` — the `.turn-loader` rule NEXT TO the typing-dots rules (find the `.bubble.typing` / dots animation block):
|
||||||
|
- Compact horizontal three-dot indicator, REUSING the existing typing-dot keyframes/dot styling (same animation name — no new animation family; sized down for the status row).
|
||||||
|
- Provenance comment: phase 109, `TODO.md` L3 — the constant in-turn progress cue; decorative (`aria-hidden`), `#send-status` carries the meaning.
|
||||||
|
- A `prefers-reduced-motion` variant mirroring the typing dots' treatment (static dots, no pulse — §7.2 house law).
|
||||||
|
4. `tests/unit/test_frontend_turn_loader.py` (EXTEND the file task 01 created):
|
||||||
|
- `test_index_html_carries_exactly_one_turn_loader` — `frontend/index.html` contains exactly ONE `id="turn-loader"`, with `aria-hidden="true"` and the `hidden` attribute (hidden by default).
|
||||||
|
- `test_set_ui_state_is_the_sole_owner_of_the_loader` — in `app.js`: `turnLoader.hidden` appears EXACTLY ONCE, inside `setUiState` (the cross-file single-owner check — grep the file text; any second write site fails the test, keeping the never-stale guarantee structural).
|
||||||
|
- `test_loader_css_reuses_the_typing_animation_and_reduced_motion` — the `.turn-loader` rule exists in `styles.css` after (or adjacent to) the typing-dots rules, references the SAME animation name as the typing dots, and a `prefers-reduced-motion` block covers it (static, no pulse); the provenance comment names phase 109.
|
||||||
|
- `test_loader_is_aria_hidden_and_status_untouched` — the loader element is `aria-hidden`; `#send-status`'s attributes are unchanged in `index.html` (still the live region — the house a11y split).
|
||||||
|
5. Run `uv run pytest tests/unit/test_frontend_turn_loader.py tests/unit/test_frontend_tool_states.py tests/unit/test_theme_frontend.py -v` (the theme suite parses `styles.css` — a new CSS rule must not break the built-in-theme pins) — green.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: the single-owner invariant + the markup/CSS contract pinned at the source level (the house pattern).
|
||||||
|
- Coverage: **>90%** on `app/` (unchanged — pure frontend task; the gate is the regression check).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `#turn-loader` exists exactly once in `index.html` (static, `aria-hidden`, hidden by default); `setUiState` is its sole visibility owner (unit-pinned)
|
||||||
|
- [ ] The CSS reuses the typing-dot animation, has the reduced-motion variant + the phase-109 provenance comment (unit-pinned); the theme CSS-parsing suites stay green
|
||||||
|
- [ ] The typing bubble's lifecycle, `SEND_STATUS`, and `#send-status` are byte-unchanged in behavior (the existing frontend suites green)
|
||||||
|
- [ ] `uv run ruff check . && uv run pyright` clean; full `uv run pytest` green
|
||||||
@@ -0,0 +1,40 @@
|
|||||||
|
# Task 03 — E2E: the reported repro (delta → tool → thinking-after-delta) + full gate + commit
|
||||||
|
|
||||||
|
**Phase:** `109_turn_progress_loader` · **Source:** `TODO.md` L3 — "the model responds, calls a tool, then continues thinking without re-expanding the thinking block" + "a visual that the chat is still progressing regardless of what state it's in"; AGENTS.md rules 4/8/9.
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Prove the fix end to end with a dedicated Playwright suite: a deterministic mock sequence that replays the owner's exact repro (answer starts, tool call, thinking AFTER the answer) must show the re-opened scratchpad, the loader visible throughout, and clean terminal states — then run the full gate and land the atomic commit.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `tests/e2e/mock_llm.py` — ONE new marker (the house rule: marker/regex changes land WITH their consuming task — this task; document it in the module docstring next to the existing markers):
|
||||||
|
- A new `*_TRIGGER` constant + branch (checked like the other user-message markers, BEFORE the DEFLECT_MODE branch) whose question forces the reported sequence with BAKED-IN DELAYS (mid-turn windows of ≥1 s each, so Playwright assertions are deterministic — the `slow_llm.py` precedent for deliberate pacing):
|
||||||
|
- model call 1: ~2 s pre-delay (model latency — the loader's start-state window), then a short `content` delta (2-3 chunks; NO reasoning), then an `ls` `tool_calls` delta (synthetic id, no arguments — the L77-81 pattern), `finish_reason: "tool_calls"`.
|
||||||
|
- model call 2 (after the server's `tool_result`): `reasoning_content` chunks (~10 × ~0.3 s), then a `content` delta (2-3 chunks ending in a DISTINCTIVE final sentence the tests can match), then a FINAL `reasoning_content` chunk (3 × ~0.3 s), then finish.
|
||||||
|
- The server is position-independent over the wire (each `reasoning_content` chunk → a `thinking` SSE frame, each `content` chunk → a `delta` frame — `app/rag/llm.py` L572+), so the resulting SSE is exactly `delta → tool → tool_result → thinking → delta → thinking → done` — the owner's repro, deterministic.
|
||||||
|
2. `tests/e2e/test_turn_progress_loader.py` (NEW — copy the app-server + fixture idiom from `tests/e2e/test_llm_history.py`: module-scoped mock-LLM app, fixture-docs import, `login`, per-test fresh conversation; module docstring: story n/a — owner request 2026-09-16, the isolation command, the marker contract, and what each test pins). Isolation: `uv run pytest tests/e2e/test_turn_progress_loader.py -v --no-cov` (DB up). Each test sends the marker question in its OWN fresh conversation (one full turn per test):
|
||||||
|
- `test_loader_visible_from_send_through_the_tool_gap` — right after the send (inside call 1's 2 s pre-delay window): `#turn-loader` is VISIBLE (the thinking state, no frame yet); then wait for the `.tool-call` line to appear (the turn is provably in flight) → the loader is STILL visible, the tool line carries the phase-87 elapsed counter, and `#send-status` carries a state text (not empty).
|
||||||
|
- `test_thinking_block_reopens_after_delta_with_visible_loader` — wait until the thinking block is open with non-empty `.thinking-text` AND the answer bubble already carries call 1's content (i.e. the post-delta re-open — THE reported symptom's state): assert the block is `open`, the new thinking text is VISIBLE in it, and the loader is STILL visible (the frozen window is gone). Then wait for the terminal state (call 2's distinctive final sentence in the bubble, or the send button back to "Send"): the loader is HIDDEN, the block is still open (the LAST frame was thinking), `.thinking-text` is non-empty, the bubble contains BOTH call 1's and call 2's content, and the send button reads "Send" (not "Stop").
|
||||||
|
- `test_loader_a11y_and_reduced_motion` — after a full turn: the loader element is `aria-hidden="true"` in the DOM (the `#send-status` live region remains the sole announcer — assert its post-done text follows the `SEND_STATUS` idle shape, i.e. not stuck on a mid-turn label); then, in a context with `reducedMotion: "reduce"` (Playwright context option), send a second turn and assert the loader is still visible mid-turn (the reduced-motion variant renders the static dots — the CSS rule, not the visibility, is what changes).
|
||||||
|
3. Run the suite in isolation — all three tests green.
|
||||||
|
4. **The full gate** (DB up; every command passes before the commit):
|
||||||
|
- `uv run pytest` — green.
|
||||||
|
- `uv run pytest --cov=app --cov-report=term-missing` — TOTAL >90%.
|
||||||
|
- Regressions in isolation (the feedback-state history this phase extends): `uv run pytest tests/e2e/test_thinking_display.py -v --no-cov`, `tests/e2e/test_stop_generation.py -v --no-cov`, `tests/e2e/test_big_read_progress.py -v --no-cov`, `tests/e2e/test_loading_feedback.py -v --no-cov`.
|
||||||
|
- `uv run ruff check . && uv run pyright` — clean.
|
||||||
|
- Scope check: `git diff --stat` shows only `frontend/index.html`, `frontend/assets/app.js`, `frontend/assets/styles.css`, `tests/**`, `.agents/phases/**` (this is a UI phase — NO `app/` changes; if the diff shows any, stop and fix the scope).
|
||||||
|
5. **The commit** (exactly one, `--no-gpg-sign`):
|
||||||
|
```bash
|
||||||
|
git add frontend/index.html frontend/assets/app.js frontend/assets/styles.css tests/ .agents/phases/ && git commit --no-gpg-sign -m "feat(chat): never-frozen turn — re-expanding thinking block + the persistent in-turn loader"
|
||||||
|
```
|
||||||
|
6. Move the phase directory: `mv .agents/phases/todo/109_turn_progress_loader .agents/phases/complete/` (the pipeline gate does this on success — do it only after the commit, matching how prior phases recorded the move).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- E2E: the owner's repro replayed deterministically (the marker's baked delays make every window assertion race-free); the a11y split (visual loader + `#send-status` announcer) + the reduced-motion variant pinned.
|
||||||
|
- Coverage: **>90%** on `app/` (unchanged by this UI phase — the gate is the regression check).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/e2e/test_turn_progress_loader.py -v --no-cov` green in isolation (DB up) — all three tests
|
||||||
|
- [ ] The reported repro is pinned: post-delta thinking re-opens the block with visible text while the loader stays visible; terminal states are clean (loader hidden, button "Send", status not stuck)
|
||||||
|
- [ ] The regression suites (`test_thinking_display`, `test_stop_generation`, `test_big_read_progress`, `test_loading_feedback`) green in isolation; `uv run pytest` green; coverage TOTAL >90%; ruff + pyright clean
|
||||||
|
- [ ] Exactly one new commit with the phase message, `--no-gpg-sign`; the diff scoped to `frontend/` + `tests/` + `.agents/phases/`; `git status` clean afterwards
|
||||||
|
- [ ] Phase dir at `.agents/phases/complete/109_turn_progress_loader/`
|
||||||
Reference in New Issue
Block a user