From 0eb3ea9ebf1656b5ca573568a17a99bb5696a8af Mon Sep 17 00:00:00 2001 From: Jonas Brami Date: Fri, 29 May 2026 09:48:07 +0400 Subject: [PATCH] docs: learning-UX roadmap + anchors design doc Investigates the current cognit quiz interface and lays out a 10-feature plan to accelerate the learning loop, split into two tracks: - Track A (ship now, no host-wake dependency): inline code context, immediate feedback, confidence, teach-back, card view, coverage map, cross-PR memory. - Track B (gated on a host-wake spike): in-UI steering, drill-this, activity panel. The decisive constraint: the host owns the terminal TTY and the web app (its MCP child) has no path to the host's stdin, so the host is strictly turn-based. Track B rests on a host-wake mechanism; the brief documents the validated long-poll design. 01-anchors.md specs the Track A foundation (optional per-question `anchor`). Co-Authored-By: Claude Opus 4.8 --- docs/design/learning-ux/00-brief.md | 108 ++++++++++++++++++++ docs/design/learning-ux/01-anchors.md | 138 ++++++++++++++++++++++++++ 2 files changed, 246 insertions(+) create mode 100644 docs/design/learning-ux/00-brief.md create mode 100644 docs/design/learning-ux/01-anchors.md diff --git a/docs/design/learning-ux/00-brief.md b/docs/design/learning-ux/00-brief.md new file mode 100644 index 0000000..498f3c3 --- /dev/null +++ b/docs/design/learning-ux/00-brief.md @@ -0,0 +1,108 @@ +# cognit learning-acceleration UX — brief + +## Original prompt + +> go on latest main, investigate current interface and tell me how we could improve the user interface, the goal is to accelerate the learning process and leverage as much as possible claude to accelerate learning + +Follow-up: split into multiple acceptable PR groups, write a design doc per task, test everything. + +## Current interface (summary of findings) + +`cognit take` launches Claude Code as a confined host session that reads the PR diff and renders a quiz in the browser. The UI today (`src/cognit/mcp/assets/{index.html,quiz_mcp.js,styles.css}`): + +- GitHub-styled single page; all questions listed at once. +- Flow: Waiting → Answering (all visible) → Submit → Results (per-question scorecard + explanations) → optional Publish. +- Sidebar with progress dots and per-question status. +- Steering ("harder", "skip", "focus this file") is **terminal-only** — requires context-switching out of the browser. +- Browser and host communicate only via `QuizState` (write-through JSON snapshot, plus the shared in-process object). +- No code context shown next to questions; no per-question feedback; no confidence rating; no retry/drill loop; no cross-PR memory. + +## The architectural constraint that shapes everything + +This is the load-bearing fact, verified against the code: + +- `cli/take.py:175` does `os.execvpe("claude", ...)`. So `claude` (the host) **owns the terminal TTY**, and the MCP server (`python -m cognit.mcp`) is its **child**. The FastAPI web app is a daemon thread *inside that child* (`server.py:225`). +- The web app therefore has **no handle on the host's stdin**. The only browser→host data path is the shared in-process `QuizState` (`state.py`), which the host reads *only when it chooses to call an MCP tool*. +- The host is **strictly turn-based**: the kickoff ends with "Then wait" (`launch.py:31`). After rendering the quiz it blocks and acts only when the user types in the terminal. There is **no polling loop, notification, or wake path** today. + +**Consequence.** Features whose value is "do something in the browser and the host reacts *without me switching to the terminal*" — steering chips, drill-this, activity panel — depend on a **host-wake mechanism that does not exist and is not yet designed**. Every other feature is independent of the host's turn loop and carries no such risk. + +Two more facts that reshape the feature framing: + +- **Grading already runs in the web process.** `web.py:77` (`POST /grade`) calls `grade_state` directly, bypassing the host. So immediate feedback, confidence, and teach-back need no host involvement. +- **`/state` already ships every correct answer to the browser.** `state.snapshot()` (`state.py:93`) dumps each question's `answer` field and `web.py:60` serves it verbatim. So "exam mode" / "confidence before reveal" buy **no integrity** — the answer is always on the client. That is fine for an honor-system learning tool, but the design docs must not imply otherwise, and we are explicitly *not* changing it. + +## The 10 features + +| # | Feature | One-line summary | +|---|---|---| +| 1 | **Inline code context per question** | Collapsible diff hunk next to each question, using a new optional `anchor: {path, start_line, end_line}` field on Question. | +| 2 | **Per-question immediate feedback + exam-mode toggle** | Deterministic types (mcq/tf/mermaid) grade and reveal on commit, client-side (the answer is already in the page). Open questions still grade at submit. localStorage toggle preserves the current batch flow. Cosmetic only — not an integrity boundary. | +| 3 | **Confidence rating before reveal** | "How sure? 1–5" prompt prior to reveal; results surface miscalibration (high confidence + wrong). Honor-system: does not gate answer visibility. | +| 4 | **In-UI steering chips** | Per-question chips (Harder · Different angle · Explain first · Drill this · Skip) enqueue steer intents for the host. **Requires the host-wake mechanism (Track B).** | +| 5 | **Drill-this loop on wrong answers** | After a deterministic miss, CTA asks the host for 1–2 variant questions on the same concept, anchored to the same hunk. **Requires the host-wake mechanism (Track B).** | +| 6 | **Teach-back for missed questions** | One-sentence "why is the right answer right?" prompt after a miss; graded via the web-process open-grading path (`grade_state` / `mcp/grading.py`). Production beats recognition for retention. | +| 7 | **One-question-at-a-time card view** | Focused single-card view as default, with j/k or arrow nav across questions. "Show all" toggle in localStorage. | +| 8 | **Live host activity panel** | Sidebar mirror of host activity (tool name + path only — no args, no secrets); newest-first; ring-buffer capped at 50, held **in memory only** (excluded from `QuizState._persist`). **Coupled to the host-wake mechanism (Track B).** | +| 9 | **Diff coverage map** | Sidebar list of files in the diff with covered/uncovered markers based on feature-1 anchors. Display-only in Track A; the "ask host to cover this" steer is Track B. | +| 10 | **Cross-PR weak-concept memory** | `~/.cognit/history.json` persists missed-concept tags, **written post-grade by the orchestrator/web process** (not the confined host); recent weak concepts feed the host kickoff for spaced repetition across PRs. | + +## Two-track roadmap + +The brief originally sequenced these as 7 stacked PRs treating the backend as "pure plumbing." That is backwards: the backend hides the project's only real unknown (host-wake), while most of the learning value is independent of it. So the work splits into two tracks. + +### Track A — ship now, no host-wake dependency (features 1, 2, 3, 6, 7, 9, 10) + +Each PR is independently verifiable and invisible when its feature is ignored. + +1. **PR A1 — anchors foundation (1).** Add optional `anchor` to the question models (`engine/models.py`); teach the generation prompt to emit it; backwards-compat load for old cached JSON (`state.py:_load`). Inline collapsible diff hunk in the UI via existing `file_diff` data. *Diff hunks must be DOM-built, never `innerHTML` — honor the textContent-only invariant.* +2. **PR A2 — diff coverage map (9).** Sidebar files-in-diff list with covered/uncovered markers derived from anchors; uses the existing `changed_files` overview. Display only (the "ask host to cover this" button is Track B — omit or stub). +3. **PR A3 — feedback loop (2 + 3 + 6).** Client-side immediate reveal for deterministic types; confidence prompt; teach-back grading reusing the web-process grading path (`web.py:77`, `mcp/grading.py`). +4. **PR A4 — one-question card view (7).** Pure frontend; j/k nav; "show all" localStorage toggle. +5. **PR A5 — cross-PR weak-concept memory (10).** `~/.cognit/history.json` module written post-grade by the web/orchestrator process; recent weak concepts injected into the host kickoff (`launch.py:_kickoff`). + +### Track B — gated behind a host-wake spike (features 4, 5, 8) + +All three require the host to react to a browser event without the user touching the terminal. **Run the spike before writing any design doc for them.** + +**Spike: blocking long-poll MCP tool.** Prototype an `await_steer` MCP tool that the host calls in a loop — it blocks server-side on a `QuizState` condition until the browser enqueues a steer intent (or a bounded timeout, e.g. 25s), returns the intent, the host acts (`replace_question` / `set_quiz` / a new `append_questions`), then calls `await_steer` again. Change the kickoff from "Then wait" to "call `await_steer` in a loop to receive the reader's requests." + +This is the *only* design that keeps the host driven by browser events while honoring the "browser ↔ host only via `QuizState`" invariant (no stdin injection). + +The spike must answer, with evidence: +- Does Claude Code tolerate a multi-second blocking MCP tool call without killing it or the session? What is the practical timeout ceiling? +- Will the agent reliably *re-loop* across many cycles, or does it drift/stop? How robust is the prompt? +- Token/cost behavior of a long-lived idle-blocking turn. + +**Decision gate:** +- **Go** → write `04-steering.md`, `05-drill.md`, `08-activity.md` against the long-poll design. The activity panel (8) becomes a natural piggyback (each `await_steer` return / tool call appends to the in-memory buffer). +- **No-go** → fall back to the honest minimal version: a chip writes its intent to `QuizState`, the browser shows "press Enter in your terminal to apply," and we descope the "kills the context-switch" promise. Document the fallback and move on. + +**Spike result: GO** (2026-05-28, headless `claude -p` + sonnet — see `spike/wake/`). A blocking `await_steer` MCP tool drove a full browser-only steering loop: the host rendered once, then cycled `await_steer → act` four times (harder / skip / drill / STOP) with **no terminal input**, re-looping reliably and exiting on the STOP sentinel. The blocking primitive is unit-proven (`spike/wake/test_steer_bus.py`: block, wake, timeout, re-arm, FIFO, 50-cycle no-loss). Track B is cleared to proceed. *Caveat carried into the design docs:* on a deferred-MCP-tools host the agent must reach `ToolSearch` to discover the quiz tools — launch flags must not zero out built-ins (`--tools ""`). Remaining risks (MCP tool-timeout ceiling, loop durability over long sessions, token cost, clean shutdown) are listed in `spike/wake/README.md`. + +### Sequencing + +Do **not** write docs 01–10 up front. Write `01-anchors.md` and run the wake spike first — they are independent (anchors unblocks Track A; the spike unblocks Track B). Let the spike result decide whether the Track B docs get written at all. + +## Invariants to preserve across all PRs + +- Browser ↔ host only via `QuizState`. (Track B's `await_steer` reads/blocks on `QuizState`; it does **not** inject stdin.) +- **Built-in filesystem tools unchanged** — the host's Read/Grep/Glob confinement (`confine.py`) is untouched. New capability is added as **MCP tools**, a separate surface; adding MCP tools does not violate this. +- New MCP tools validate inputs and only mutate `QuizState` — no shell, no model judgment, no disk writes outside the snapshot path (`history.json`, written by the web/orchestrator process, is the documented exception). +- Frontend uses `textContent` only for any user/agent-supplied string (no `innerHTML`); diff-hunk rendering is DOM-built. +- Publish stays human-gated in the browser. +- Existing flow keeps working when all new features are ignored. +- Backwards-compat: old cached quiz JSON files (without new optional fields) must still load. + +## Status + +The original three-subagent dispatch refused the work as too large for one turn — a signal the brief was too coarse, now addressed by the two-track split. + +Done: +- `01-anchors.md` written (Track A foundation). +- Host-wake spike run and **passed (GO)** — `spike/wake/` (blocking `await_steer` long-poll; deterministic primitive test + headless `claude -p` loop validation). + +Doc filenames follow feature numbers (so `01-anchors.md` = feature #1). Next steps: +- **Implement PR A1 (anchors)** from `01-anchors.md` — it's the spine the rest of Track A builds on. +- Write the remaining Track A docs: `09-coverage-map.md` (PR A2), `03-feedback-loop.md` covering features 2/3/6 (PR A3), `07-card-view.md` (PR A4), `10-cross-pr-memory.md` (PR A5). +- Write the Track B docs against the validated long-poll design: `04-steering.md`, `05-drill.md`, `08-activity.md`. diff --git a/docs/design/learning-ux/01-anchors.md b/docs/design/learning-ux/01-anchors.md new file mode 100644 index 0000000..1da178e --- /dev/null +++ b/docs/design/learning-ux/01-anchors.md @@ -0,0 +1,138 @@ +# 01 — Inline code context per question (anchors) + +**Track A · PR A1 · feature #1 · foundation for #5, #9** + +## Why + +Today a question floats free of the code it probes. The reader has to hold the +diff in their head or context-switch to GitHub. The generation prompt already +encourages `file:line` references in prose (`system_generate.txt:11`), but that's +unstructured text — the UI can't act on it. Adding a structured `anchor` to each +question lets the browser show the exact diff hunk inline, and gives features #5 +(drill the same hunk) and #9 (coverage map) a machine-readable target. + +This is the spine of Track A: ship it first, the rest builds on the field. + +## What + +Add an optional `anchor` to every question type, teach the generator to emit it, +serve diff hunks to the browser, and render a collapsible hunk under each +anchored question. Optional throughout → old cached quizzes and questions without +anchors keep working unchanged. + +## Design + +### 1. Model (`src/cognit/engine/models.py`) + +Add one model and an optional field on all four question types: + +```python +class Anchor(BaseModel): + path: str # repo-relative path from the diff + start_line: int = Field(ge=1) + end_line: int = Field(ge=1) + + @model_validator(mode="after") + def _ordered(self) -> "Anchor": + if self.end_line < self.start_line: + raise ValueError(f"end_line {self.end_line} < start_line {self.start_line}") + return self +``` + +Add `anchor: Anchor | None = None` to `MCQQuestion`, `MermaidQuestion`, +`OpenQuestion`, `TrueFalseQuestion`. Because it defaults to `None`: + +- **Backwards-compat is automatic.** Old snapshot JSON (`state.py:_load`) and old + cached quizzes without the field validate fine — no migration, no version bump. + `Quiz.version` stays `"1"`. +- The discriminated-union `Question` type needs no change (still keyed on `type`). + +Keep validation deliberately light: an anchor is a *hint*, not an assertion that +the lines are part of the diff. A question may legitimately anchor surrounding +unchanged context the reader needs. So **do not** reject anchors whose `path` +isn't in the changed-file set or whose lines fall outside a hunk — only the +shape/ordering above. (The coverage map in #9 treats "anchor.path ∈ changed +files" as *covered*; paths outside the diff simply don't mark anything.) + +### 2. Validation (`src/cognit/mcp/validate.py`) + +Pydantic already enforces the shape via the union parse in `validate_question`. +No new explicit checks needed — a malformed anchor surfaces through the existing +`malformed question: {e.errors()}` path and is handed back to the agent to fix. + +### 3. Generation prompt (`src/cognit/engine/prompts/system_generate.txt`) + +The generator already pulls hunks via `file_diff(path)` and is told to reference +`file:line`. Extend the output spec so it *also* emits the structured anchor when +a question targets specific lines. Add to the per-type field notes (near line +57–78) and the JSON examples: + +> Every question SHOULD include an `anchor` when it probes specific code: +> `"anchor": {"path": "", "start_line": N, "end_line": M}` +> — the line range (from the file's current/new-side numbering) the reader should +> look at. Omit `anchor` only for questions not tied to a specific location (e.g. +> a broad `open` rationale question). The anchor drives an inline code panel; it +> does not change grading. + +Anchor emission is a **generation-time** behavior (one host turn), so it does +**not** depend on the Track B host-wake mechanism. This is why PR A1 is safe. + +### 4. Serve hunks to the browser + +The browser can't call MCP tools, and the web app currently has no access to the +diff — `_DiffProvider` is created inside `_build_mcp`'s closure (`server.py:121`). +Hoist it so both the MCP surface and the web app share one instance: + +- In `server.py:main()`, construct `diffs = _DiffProvider(pr_url)` once and pass + it into both `_build_mcp(state, llm, diffs)` and `_start_web(..., diffs=diffs)`. +- `build_web_app` (`web.py:36`) gains a `diff_section: Callable[[str], str]` + param wired to `lambda path: do_file_diff(path, diffs.sections())` (reuse the + existing `do_file_diff`, `server.py:74` — it already tolerates basename/suffix + variants and refuses ambiguous matches). +- New endpoint `GET /diff?path=` → returns the file's diff section as + `text/plain` (or the existing "no changed file matches…" message). Caching is + free: `_DiffProvider` already fetches once per process and is thread-safe. + +This keeps the invariant intact: the browser reads diff text over HTTP from the +same process; the host still touches `QuizState` only via MCP tools. + +### 5. UI (`assets/quiz_mcp.js`, `styles.css`) + +For each question whose `/state` payload includes an `anchor`: + +- Render a collapsible "📄 `path`:`start`–`end`" disclosure under the prompt, + collapsed by default. +- On first expand, `fetch('/diff?path=' + encodeURIComponent(anchor.path))`, + cache the result per path, and render the file's hunks. +- **DOM-built only** — build `
`/`` nodes and set text via
+  `textContent`; never `innerHTML`. (Honors the existing textContent-only
+  invariant; diff text is agent/repo-supplied.) Color +/- lines via CSS classes
+  on per-line spans, not string-injected markup.
+- v1 shows the file's changed hunks; highlighting the exact `start_line..end_line`
+  band is a nice-to-have, not required for this PR.
+
+## Files touched
+
+- `src/cognit/engine/models.py` — `Anchor` model + optional field ×4
+- `src/cognit/engine/prompts/system_generate.txt` — emit `anchor`
+- `src/cognit/mcp/server.py` — hoist `_DiffProvider` into `main()`, thread to web
+- `src/cognit/mcp/web.py` — `diff_section` param + `GET /diff`
+- `src/cognit/mcp/assets/quiz_mcp.js`, `styles.css` — collapsible hunk panel
+
+## Verification
+
+- **Unit (`tests/mcp/`, `tests/engine/`):**
+  - `models`: a question with a valid anchor round-trips; `end_line < start_line`
+    raises; **a question dict with no `anchor` still validates** (backwards-compat);
+    old snapshot JSON without the field loads in `QuizState` (extend
+    `tests/mcp/test_state.py`).
+  - `web` (`tests/mcp/test_web.py`): `GET /diff?path=` returns the section;
+    unknown path returns the "no changed file matches" message; uses a fake
+    `diff_section` (no network).
+  - `validate`: a malformed anchor comes back through the existing failures list.
+- **e2e:** extend `tests/mcp/test_generation_e2e.py` to assert at least one
+  generated question carries a well-formed anchor whose `path` is in
+  `changed_files`.
+- **Manual:** `cognit take `; confirm each anchored question shows the
+  collapsible hunk, expands to the right file's diff, and that a quiz generated
+  before this change (cached snapshot) still loads with the panels simply absent.