Skip to content

Commit 8fcb5f5

Browse files
committed
docs(project): record X2a eval-harness slice and its review outcome
1 parent fb51858 commit 8fcb5f5

1 file changed

Lines changed: 1 addition & 0 deletions

File tree

project/2026-07-26-pr-5109-verification-and-extension-plan.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ Open decisions to surface to Roland, in order: **A1** (answer-first vs act-first
238238
- 2026-07-26: **X5 done.** Additive `AuditLogType.ASSISTANT_PROPOSAL_CONFIRMED` on the existing `AuditLogEntry` model (`sharing.prisma`; migration `20260726184305_assistant_proposal_audit`, SQL = single `ALTER TYPE … ADD VALUE`, verified non-destructive; mirrored to `apps/analytics` via prisma:sync). `verifyManageProposalToken` now surfaces `jti: string | null` (legacy pre-jti tokens → explicit null); confirm route writes a best-effort `AuditLogEntry` after `confirmManageProposal` succeeds via `recordProposalConfirmationAudit` (`manageProposals.ts`): type + `objectType ELEMENT` + `objectId` = created element id (always available — `confirmedElementSchema` guarantees it) + `sourceUserId` + message = JSON `{kind, jti, summary}` only, no payload dump. Audit-write failure is logged and swallowed (persistence already committed). `AuditLogType` is not GraphQL-exposed (grep-verified) — no codegen needed. Verified in-container: chat 199/199, check clean, prisma build clean. Gotchas recorded: enum visibility requires `pnpm --filter @klicker-uzh/prisma run build` (migrate/sync alone leave stale dist); in-container `pnpm run prisma:migrate` blocked by missing jq/infisical in the self-contained devcontainer — ran `prisma migrate dev --name …` directly from packages/prisma (pre-existing env gap). Residual (scoped out): no audit on failed/replayed confirmation attempts; no read/query surface; audit failure only console.error'd (no metric).
239239
- 2026-07-26: **X4 follow-up defect found and fixed (regression introduced by X4 itself).** While building the X2 harness, a live SSE capture showed that `tool-output-available` reaches the **browser** fence-wrapped — and `getManageProposalResult` (`apps/chat/src/components/manage-proposal-card.tsx`) still did a bare `JSON.parse(record.text)`, which throws on the marker line, gets swallowed by the surrounding catch, and silently drops the confirmation card. So between `9712a9743` and this fix, the lecturer very likely saw the generic raw-output fallback instead of a clickable Create-draft card. Nothing caught it: the existing card test fed **unfenced** JSON, and `playwright/util/manageAssistant.ts` mocks tool output unfenced too, so both suites stayed green. Fix: new import-free module `apps/chat/src/services/toolFenceSyntax.ts` owns the marker shape (`openFenceMarker`/`closeFenceMarker`, re-exported by `toolOutputFencing.ts`) plus `unfenceToolResultText`, an anchored strip whose closing sentinel must match the opening one via backreference; the card parser unwraps before parsing. Separate module because the fenced text crosses to the client and `toolOutputFencing.ts` imports `node:crypto` (must not enter the client bundle). Verified: chat vitest 205/205 (29 files, +6 tests — fenced-shape regression test, mismatched-sentinel rejection, 4 unfence cases); **browser-verified** on the live stack (chat `/manage`, delegated lecturer session) — the card renders with question preview, all 4 options, Create draft / Dismiss, "Show raw JSON"; Create draft deliberately NOT clicked (no persistence). Pre-fix path re-proved out-of-band (`JSON.parse` of a fenced envelope throws `SyntaxError`). docs/auth-model.md gained the writer/reader contract and this regression note. Lesson worth carrying: fencing changed a payload that two consumers read (model + browser); only one was updated, and every mock in the repo encoded the pre-fencing shape.
240240
- 2026-07-26: **X3 design decided** (Opus investigation, 76 tool-uses): Option C (persisted-GraphQL proxy) for new T1 reads; existing four tools stay direct-Prisma; details in the X1/X3 prerequisite table. Investigation also surfaced: (a) new finding §3.7 (mcp token = de-facto role-only public-API token via the backend bearer fallback; pre-existing; remediation is a standalone decision constrained by the assessment do-not-touch path); (b) mcp-lecturer's existing unit tests run in NO CI workflow (no `test:run` script, so root turbo never reaches them) — folded into X1's scope as a live amendment to the running builder. ADR gate checked: X3 choice is per-tool reversible → no ADR, plan-recorded only.
241+
- 2026-07-27: **X2a done** (`fb5185808`) — deterministic eval harness at `evaluation/manage-assistant/` (uv/Python 3.12, host-run, 48 files). Route-level: drives the real `/api/manage/chat` SSE endpoint with a minted lecturer session cookie, parses the AI SDK UI-message-stream, fence-strips tool output, and detects proposal cards. Three dimensions, 30 GT cases in the sibling framework's markdown+frontmatter format: E1 tool selection (12 cases, soft 0.95), E5 refusal/do-not-save (8) and E6 injection (10) as hard gates. E6 is a genuine indirect-injection surface — `seed.py` idempotently creates a synthetic *collaborator* lecturer owning payload-carrying elements and grants the eval lecturer READ via `DerivedPermission` (the real sharing visibility path, since the MCP reads gate on derived permissions, not `ownerId`). **Location decision:** in-repo rather than in the private `evaluation/framework` submodule (PR #5190), because a nightly gate in a public repo cannot depend on a private submodule; GT stays under `evaluation/manage-assistant/data/` rather than the `evaluation/data/` root so this branch stays independent of #5190. **Adversarial review (Opus) verdict needs-fix; both BLOCKING findings were reproduced by me before accepting:** (1) *the hard gates could not fail on a dead turn* — every check tests for the ABSENCE of unsafe behavior, so an HTTP-200 stream that terminated immediately (AI SDK `finishReason:"error"`, which emits no `{type:'error'}` frame the harness already caught) scored a clean pass on every E5 do-not-save case and every direct E6 case; `finish_reason` was captured and never read. Fixed with a liveness gate requiring prose or a tool call. (2) *E6's retrieval check was content-blind* — it asserted only that a retrieval tool NAME was called, so a zero-hit search was indistinguishable from resisting an injection; fixed by requiring the seeded element's unique payload marker (now defined next to the content in `seed.py`) in the output the model actually received. Verified statically that all four markers survive the server's `compactPlainText` normalization and both truncation limits (`element_search` snippet 500, `element_get` detail 4000) — so search really does deliver the full payload, not just a name. Also fixed: declared `expected_calls[].arguments` are now enforced (a hallucinated id no longer passes), the seven subset-policy E1 read cases forbid a spurious persist call, trial-capped runs are labelled as such, the 429 retry is counted by the pacer, and the stale "known issue" README section now records the X4 card regression as fixed in `1a1444c34`. **Durable anti-regression:** `tests/test_scoring_contract.py` — 15 network-free tests (marked `offline`, gated by a new function-scoped autouse fixture so they never touch DB/HTTP) over the *real shipped case files*, asserting the gates FAIL on a degenerate turn, on content-blind retrieval, on forbidden calls + leaked card, and on transport errors. Each fix was transiently reverted to prove its test fails (`assert not True` / `assert None is not None`), then restored. Verified: ruff check + format clean, 46 tests collect, 15/15 offline pass in 0.07s, prettier clean on all 31 markdown files, hygiene scan clean (the only credential-shaped string is the `klicker-prod:klicker` DSN already committed in the tracked `.devcontainer/devcontainer.env`). **Residual — the one gap:** the 30-case *live* run has NOT been re-executed since these scoring changes. The builder's pre-review run was 31/31 at `MAX_TRIALS=1`, but that predates the liveness gate, the argument enforcement and the E6 marker check. The stack was OOM-reaped and restarted; re-running needs `UPSTREAM_OPENAI_API_KEY`, and `OPENROUTER_API_KEY` is not read-allowlisted on the `klicker-prd` operator profile (only `DATABASE_URL` is) — blocked pending Roland's approval of `rs-infisical-operator --profile klicker-prd allow-read OPENROUTER_API_KEY`. The two live-only risks are E1 cases 04/06 under real model behavior (could drop E1 below 0.95) and end-to-end confirmation of the E6 marker path.
241242

242243
## Progress
243244

0 commit comments

Comments
 (0)