From 9e9ac9ea764a9c9580020e857811836afdfcaad0 Mon Sep 17 00:00:00 2001 From: Jonas Brami Date: Fri, 29 May 2026 12:50:53 +0400 Subject: [PATCH] feat(quiz): diff coverage map in the sidebar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shows which of the PR's changed files have a question anchored to them, so the reader can see at a glance what the quiz does and doesn't probe. - web: `GET /changed-files` returns the PR's changed-file paths (`changed_files` callable, wired in server.py to the shared `_DiffProvider`). 503 when unwired. - frontend: a "Diff coverage" sidebar block (answering view) listing each changed file with a covered/uncovered marker + an "N of M files probed" count. Coverage is computed client-side by matching question anchors to changed files with the same exact / repo-relative-suffix / basename logic the server's do_file_diff uses. Display only — the "ask the host to cover this file" steer is Track B. Builds on the anchors feature (#26). Tested: /changed-files endpoint (list + 503). Verified in a real browser: exact and basename-only anchors both mark their file covered; files with no anchored question show uncovered; count is correct. Co-Authored-By: Claude Opus 4.8 --- src/cognit/mcp/assets/quiz_mcp.js | 46 +++++++++++++++++++++++++++++++ src/cognit/mcp/assets/styles.css | 8 ++++++ src/cognit/mcp/server.py | 1 + src/cognit/mcp/web.py | 13 +++++++-- tests/mcp/test_web.py | 32 +++++++++++++++++++++ 5 files changed, 98 insertions(+), 2 deletions(-) diff --git a/src/cognit/mcp/assets/quiz_mcp.js b/src/cognit/mcp/assets/quiz_mcp.js index a874d09..19fc8c8 100644 --- a/src/cognit/mcp/assets/quiz_mcp.js +++ b/src/cognit/mcp/assets/quiz_mcp.js @@ -384,6 +384,52 @@ function renderSidebar() { ` Q${i + 1} · ${TYPE_LABEL[q.type].split(" ")[0].toLowerCase()}`, ]))), ])); + renderCoverageBlock(); // async: appends "Diff coverage" once the file list is fetched +} + +// ── diff coverage map ─────────────────────────────────────────── +// Sidebar list of the PR's changed files, marked covered when some question is +// anchored to them. Display only (the "ask host to cover this" steer is Track B). +let changedFiles = null; // string[] — fetched once per page (null = not yet fetched) + +async function ensureChangedFiles() { + if (changedFiles !== null) return changedFiles; + try { + const r = await fetch("/changed-files"); + changedFiles = r.ok ? ((await r.json()).files || []) : []; + } catch (e) { + console.warn("changed-files fetch failed", e); + changedFiles = []; + } + return changedFiles; +} + +// Mirror of the server's do_file_diff path matching (exact, repo-relative suffix, +// or basename) so a covered marker lines up with what /diff would actually serve. +function fileMatchesAnchor(file, anchorPath) { + if (!anchorPath) return false; + if (file === anchorPath || file.endsWith("/" + anchorPath) || anchorPath.endsWith("/" + file)) { + return true; + } + return file.split("/").pop() === anchorPath.split("/").pop(); +} + +async function renderCoverageBlock() { + const files = await ensureChangedFiles(); + if (!files.length || !quiz) return; // no diff info, or quiz cleared while awaiting + sidebarRoot.querySelector(".side-block--coverage")?.remove(); // idempotent re-render + const anchorPaths = quiz.questions.map((q) => q.anchor && q.anchor.path).filter(Boolean); + const rows = files.map((f) => ({ path: f, covered: anchorPaths.some((ap) => fileMatchesAnchor(f, ap)) })); + const coveredCount = rows.filter((r) => r.covered).length; + sidebarRoot.appendChild(el("div", { class: "side-block side-block--coverage" }, [ + el("div", { class: "side-title", text: "Diff coverage" }), + el("div", { class: "progress-text", text: `${coveredCount} of ${rows.length} files probed` }), + el("ul", { class: "sidelist coverage" }, + rows.map((r) => el("li", { class: r.covered ? "covered" : "uncovered", title: r.path }, [ + el("span", { class: `ic ${r.covered ? "ok" : "empty"}`, text: r.covered ? "✓" : "○" }), + el("span", { class: "coverage__file", text: ` ${r.path.split("/").pop()}` }), + ]))), + ])); } function updateSidebarProgress() { diff --git a/src/cognit/mcp/assets/styles.css b/src/cognit/mcp/assets/styles.css index 3ddff69..92beb26 100644 --- a/src/cognit/mcp/assets/styles.css +++ b/src/cognit/mcp/assets/styles.css @@ -869,6 +869,14 @@ textarea.open:focus { border-color: var(--blue); outline: none; box-shadow: 0 0 .diff-line.anchor-hit { box-shadow: inset 3px 0 0 var(--blue); } .codepanel__note { padding: 10px 12px; font-size: 12px; color: var(--fg-mute); background: var(--bg-canvas); border-radius: 0 0 6px 6px; } +/* diff coverage map (sidebar) ────────────────────────────────────── */ +.sidelist .ic.empty { background: transparent; color: var(--fg-faint); box-shadow: inset 0 0 0 1.5px var(--border); } +.sidelist.coverage .coverage__file { + font-family: var(--mono); font-size: 12px; + overflow: hidden; text-overflow: ellipsis; white-space: nowrap; +} +.sidelist.coverage li.uncovered .coverage__file { color: var(--fg-mute); } + @media (prefers-reduced-motion: reduce) { .codepanel__icon { transition: none; } } diff --git a/src/cognit/mcp/server.py b/src/cognit/mcp/server.py index 6e91326..da32168 100644 --- a/src/cognit/mcp/server.py +++ b/src/cognit/mcp/server.py @@ -168,6 +168,7 @@ def _start_web( post_comment=lambda body: gh_post_comment(pr_url, body), grade=lambda: grade_state(state, llm=llm), diff_section=lambda path: do_file_diff(path, diffs.sections()), + changed_files=lambda: list(diffs.sections()), pr_url=pr_url, ) url = f"http://127.0.0.1:{port}" diff --git a/src/cognit/mcp/web.py b/src/cognit/mcp/web.py index d39d6e5..3350741 100644 --- a/src/cognit/mcp/web.py +++ b/src/cognit/mcp/web.py @@ -39,6 +39,7 @@ def build_web_app( post_comment: Callable[[str], str], grade: Callable[[], Results] | None = None, diff_section: Callable[[str], str] | None = None, + changed_files: Callable[[], list[str]] | None = None, pr_url: str = "", ) -> FastAPI: """Browser projection over `state`. @@ -47,8 +48,10 @@ def build_web_app( button. It runs the same handler-owned grading the agent's `grade` tool uses and must store the result in `state`; we return the Results to the page. `diff_section` (when provided) returns the unified-diff section for one changed file — GET /diff - serves it so the browser can show a question's anchored hunk inline. `pr_url` feeds - the page chrome (the "on GitHub" links). + serves it so the browser can show a question's anchored hunk inline. `changed_files` + (when provided) lists the PR's changed-file paths — GET /changed-files serves it so + the browser can render the diff coverage map. `pr_url` feeds the page chrome (the + "on GitHub" links). """ app = FastAPI() app.mount("/static", StaticFiles(directory=str(_ASSETS_DIR)), name="static") @@ -82,6 +85,12 @@ def get_diff(path: str = "") -> PlainTextResponse: return PlainTextResponse("diff not available", status_code=503) return PlainTextResponse(diff_section(path)) + @app.get("/changed-files") + def get_changed_files() -> JSONResponse: + if changed_files is None: + return JSONResponse({"error": "diff not available"}, status_code=503) + return JSONResponse({"files": changed_files()}) + @app.post("/grade") async def do_grade() -> JSONResponse: if grade is None: diff --git a/tests/mcp/test_web.py b/tests/mcp/test_web.py index e5bed33..753cfa0 100644 --- a/tests/mcp/test_web.py +++ b/tests/mcp/test_web.py @@ -209,6 +209,38 @@ def test_diff_endpoint_503_when_unavailable(client) -> None: assert c.get("/diff", params={"path": "x.py"}).status_code == 503 +def test_changed_files_endpoint_lists_files(tmp_path: Path) -> None: + state = QuizState(pr_number=7, snapshot_path=tmp_path / "s.json") + state.set_quiz( + Quiz( + pr_number=7, + questions=[ + MCQQuestion(id="q1", prompt="p", options=["A", "B"], answer="A", explanation="x") + ], + ) + ) + app = build_web_app( + state, + post_comment=lambda b: "http://c/1", + changed_files=lambda: ["src/a.py", "src/b.py"], + ) + port = _free_port() + server, t = _serve(app, port) + try: + r = httpx.get(f"http://127.0.0.1:{port}/changed-files") + assert r.status_code == 200 + assert r.json()["files"] == ["src/a.py", "src/b.py"] + finally: + server.should_exit = True + t.join(timeout=2) + + +def test_changed_files_503_when_unavailable(client) -> None: + # the default fixture app wires no `changed_files` + c, _state, _ = client + assert c.get("/changed-files").status_code == 503 + + def test_publish_before_grading_409(tmp_path: Path) -> None: state = QuizState(pr_number=7, snapshot_path=tmp_path / "s.json") state.set_quiz(