Fix(eval): Each developer case statement names the interface its withheld tests import (#1750)

* Fix(eval): Developer case statements name what withheld tests import

* Fix(eval): Compare Interface fields as parsed; refuse failed searches

Author: daniboomerang

apps/cli/specs/evals.md

@@ -256,7 +256,7 @@ A glob with no wildcard covers that path and everything under it; `*` matches wi
 
 ### Banned terms
 
-`banned-terms.ts` holds the one list of process terms a neutral task statement may not contain: `Surface`, `objective`, `brief` and `verdict` (any case, singular or plural); every reviewer severity name as the scales write it (`BLOCKER`, `MAJOR`, `MINOR`, `CRITICAL`, `HIGH`, `MEDIUM`, `LOW`); an objective id such as `O3`; a finding id such as `F2` or `R2-CR-3`; and the role names `principal`, `planner`, `architect`, `archivist`, `developer`, `reviewer` and `operator`. `checkTaskStatement` refuses a statement containing any of them as `banned_term`, naming the term, and accepts one free of them; `findBannedTerms` lists every term found. Severity names are matched only in capitals, so lowercase `blocker`, `major` or `minor` — and `blocking` — are accepted: banning them in any case would refuse ordinary English such as "a major release" or "low latency". A statement that names a severity in lowercase is therefore not caught here.
+`banned-terms.ts` holds the one list of process terms a neutral task statement may not contain: `Surface`, `objective`, `brief` and `verdict` (any case, singular or plural); every reviewer severity name as the scales write it (`BLOCKER`, `MAJOR`, `MINOR`, `CRITICAL`, `HIGH`, `MEDIUM`, `LOW`); an objective id such as `O3`; a finding id such as `F2` or `R2-CR-3`; and the role names `principal`, `planner`, `architect`, `archivist`, `developer`, `reviewer` and `operator`. `checkTaskStatement` refuses a statement containing any of them as `banned_term`, naming the term, and accepts one free of them; `findBannedTerms` lists every term found, outside the code spans of an Interface block that ends the statement (see the layer-`G` re-cut). Severity names are matched only in capitals, so lowercase `blocker`, `major` or `minor` — and `blocking` — are accepted: banning them in any case would refuse ordinary English such as "a major release" or "low latency". A statement that names a severity in lowercase is therefore not caught here.
 
 <!-- AEG:CLAIM: apps/cli/src/lib/dev-review-eval/banned-terms.ts contains:export function checkTaskStatement(statement: string, path = 'taskStatement'): Validated<string> { -->
 
@@ -305,7 +305,9 @@ A glob with no wildcard covers that path and everything under it; `*` matches wi
 - **Source citation** — each development case becomes an `answer` case at the case file's repository commit, its scope empty, each source a delivered document pinned by the SHA-256 of its content with its own `relevant` flag, and `read_document` granted exactly when a source is delivered by retrieval tool. The held-out source-citation cases map the same way, from `source-citation-held-out.json` to `source-citation-generic-held-out.json`, and so every neutral suite has a held-out split.
 - **Whole loop** — each developer capability case, development and held-out, also becomes a `loop` case: the same base revision, task statement, authorized scope, delivered documents, hidden tests and reference-solution digest as its `change` case, granting the capabilities the developer, both reviewers and the publication need (`LOOP_CAPABILITIES`). Its cases live in their own files, `whole-loop-generic-cases.json` and `whole-loop-generic-held-out.json`, with their own digests and held-out index; the held-out file is cut from the developer held-out cases under the developer approval. The suite's entry in each file carries a `variants` object beside its `cases` and `layerPOnly`, never inside a case, mapping each live variant to the sorted ids of the cases it applies to: `source-requiring` when a case delivers a document, and `reviewer-requested-revision` when the committed `whole-loop-revision-facts.json` records at least one review round that asked its source change for a revision. That file lists every developer capability case whose rounds were read; a loop case it does not list refuses the re-cut.
 
-A development case's task statement is the one `generic-task-statements.json` holds for its id, committed verbatim; a missing statement refuses the re-cut. A loop case's statement is its developer capability case's, copied unchanged. A held-out case's statement is generated by the script from the case's own fields — its withheld tests' count and its scope, the file its change touches, or the count of documents a source-citation case delivers — leaving out any field that would carry a banned term. The held-out run needs the pinned digest of each of the three layer-`P` held-out files (developer, reviewer and source citation), reads each case by field, writes only the derived files and prints only case counts, ids and digests; a derived case that is not valid and renderable is reported by id and digest only, and nothing is written.
+A development case's task statement is the one `generic-task-statements.json` holds for its id, committed verbatim, a developer case's followed by its Interface block (below); a missing statement refuses the re-cut. A loop case's statement is its developer capability case's, copied unchanged. A held-out case's statement is generated by the script from the case's own fields — its withheld tests' count and its scope, the file its change touches, or the count of documents a source-citation case delivers — leaving out any field that would carry a banned term. The held-out run needs the pinned digest of each of the three layer-`P` held-out files (developer, reviewer and source citation), reads each case by field, writes only the derived files and prints only case counts, ids and digests; a derived case that is not valid and renderable is reported by id and digest only, and nothing is written.
+
+A developer capability case's statement, development and held-out, and so its loop case's, ends with the case's Interface block, so that a correct solution written independently of the reference change can pass the withheld tests. `interfaceItems` derives it from the case's fail-to-pass test files and its reference solution files, each read by its pinned blob, and the base revision's tree, parsing every source file with the TypeScript compiler API. It names each export a fail-to-pass test imports, a default import naming the module's `default` export, with that export's declared signature — the parameter list, without default values, and the return annotation as the reference source writes them, or the declaration's kind alone (`const`, `class`, `interface`, `type`) where nothing is written; a type alias's or enum's literal members; never a type the compiler infers, so a parameter with a default value is written optional, with its type only when the source annotates it — and each module the tests import that the reference change adds; then each environment variable the reference solution reads, by property, by index or by destructuring `process.env`, each `--flag` it names, each event it passes to `.on`, `.emit` or `.once`, and each field a named interface, class or object type alias declares as a direct member, with the field's written value type and the literal members of a same-file alias or enum it names, that the fail-to-pass tests also name; a type written across lines is written on one line by the compiler's printer, which keeps the member separators a line break stood for; and each string literal a fail-to-pass test passes to `expect(...)` or a matcher that the reference solution's files contain and that holds a letter. An export the base revision's module at the imported path already exports with the same signature is left out, and so is every other literal some tracked base file outside the fail-to-pass test files already contains: the base copies of the files the reference changes are checked first — a field by its parsed owner, name and type, so a type written across lines is found too — and the rest by a fixed-string `git grep -F` of the base revision, at most two hundred patterns a call; a search that fails for any reason but finding nothing refuses the re-cut. No function body, test name, exported name the tests do not import, or number is named. The block is one space, `Interface:`, then the items, each `` <kind> `<text>` `` with kind `export`, `file`, `env`, `flag`, `event`, `field` or `string`, separated by `; `, listed by kind then text, and a full stop, all on the statement's one line; a case with no item keeps its statement byte for byte. The held-out blocks are derived only once the developer approval has opened the held-out cases, and no run prints a block or any part of one. `findBannedTerms` skips the code spans of a well-formed Interface block that ends the statement, after its last ` Interface: ` marker, since a span names an identifier the tests use, and checks the rest of the statement as written.
 
 ```text
 bun apps/cli/tests/eval/recut-generic-cases.ts development
@@ -925,7 +927,7 @@ bun apps/cli/src/lib/dev-review-eval/harbor-tasks.ts --out <dir> [--archive <fil
 A folder holds:
 
 - `task.toml` — Harbor `0.24.0`, the suite and the Vinaya renderer version under `[metadata]`, with, for a quarantined case, its quarantine's `quarantine_cause`, `quarantine_reason`, `quarantine_reason_version` and `quarantine_adjudication_digest`, and a network policy per phase. Conversion defaults to `--adapter claude-code`: the agent phase allows `api.anthropic.com` alone, with `CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC=1` so Claude makes no telemetry or update call. Conversion with `--adapter codex` allows `api.openai.com` and `chatgpt.com` instead, the Codex subscription hosts. The verifier phase, which runs in the agent's container, has no network. Image build and agent install run on the public network; only the Codex install path stages a login. No allowlist names `github.com` or `codeload.github.com`.
-- `instruction.md` — the case's statement from `generic-task-statements.json`.
+- `instruction.md` — the neutral case's `taskStatement`, its Interface block included, in the development split as in the held-out split.
 - `environment/Dockerfile` — Bun `1.4.2` and Node 22 in `/usr/local/bin`; `gh` from the Debian package; Claude Code installed with `npm install --global`, its `/usr/local/bin/claude` removed and `claude` linked at `/opt/claude/bin/claude` to the installed package's `bin/claude.exe`, with `/opt/claude/bin` first on `PATH`, a directory holding neither `bun` nor `node`, so a test that drops every `PATH` directory holding `claude`, `codex` or `gemini` still finds both; and the non-root user `tester` (uid `1001`, home `/home/tester`). Then the case repository at `/app`, built by `environment/prepare-repository.sh`: a partial clone of the case's real base commit with its full commit history and no blob fetched up front, so a test that reads the base's history finds it, a non-cone sparse checkout that leaves out every `apps/cli/tests/eval/*.json` and every `*held-out*.json` (a cone pattern cannot leave out a file pattern), so those files' contents are in neither the working tree nor the object store, then, for a `change` case only, `bun install --frozen-lockfile` in `/app` while the remote still exists, then the remote removed so no blob can be fetched later. A `change` case installs because its verifier has no network and runs the hidden tests with vitest from `/app/node_modules`; a `review` or `answer` case runs no hidden test and its role runs no repository code, so its image installs no workspace dependencies in `/app`. A task converted with `--archive` also holds the Vinaya CLI archive as `environment/vinaya.tar`; the image unpacks it at `/opt/vinaya`, runs `bun install --frozen-lockfile` there while the build still has the public network, records the archive's SHA-256 in `/harbor/vinaya-archive.sha256` and removes the archive. A task converted without it builds with no Vinaya CLI and still runs Harbor's oracle and `nop` agents. `HEAD` stays the real base commit, which the change-bound grader compares with the case's base revision; a rebuilt repository would have another id. A `review` case then commits the change under review as `HEAD` (`environment/commit-review.sh`): its head commit, fetched by the clone, or, for a seeded change whose head commit no repository holds, the case's inline patch applied with `git apply --unidiff-zero` and committed with the seeded identity, which reproduces the head commit's id. Last, as root, `/app`, `/case`, `/logs` (with `/logs/agent` and `/logs/verifier`) and, with `--archive`, `/opt/vinaya` are made `tester`'s, `/app` is trusted by git for any user, `HOME` is `/home/tester`, and the image ends with `USER tester`, after the clone and the review commit: the role, the verifier and the oracle run as `tester`.
 - `environment/case.json` — the neutral case and only what the run needs from the layer-`P` case of the same id: a `change` case's pull request number, a `review` case's patch, an `answer` case's documents. It holds no hidden test and no reference outcome; those are written only under `tests/` and `solution/`, which the role never sees.
 - `tests/test.sh`, `tests/grade.js`, `tests/grading.json` and `tests/hidden/` — the verifier, below.

apps/cli/src/lib/dev-review-eval/banned-terms.ts

@@ -64,11 +64,34 @@ export interface BannedTermHit {
   index: number
 }
 
+/** The marker an Interface block opens with. */
+const INTERFACE_MARKER = ' Interface: '
+
+/** A whole Interface block: items `<kind> `<text>`` separated by `; `, ending the statement with a full stop. */
+const INTERFACE_BLOCK =
+  /^ Interface: (?:export|file|env|flag|event|field|string) `[^`]*`(?:; (?:export|file|env|flag|event|field|string) `[^`]*`)*\.$/
+
+/**
+ * The statement with the code spans of a closing Interface block blanked out:
+ * a span names an identifier the task's tests use, which may hold a process
+ * word (a `--surface-*` flag) without stating the task in process terms. Only
+ * a block after the last marker that ends the statement counts; the rest of
+ * the statement is checked as written, and each index stays where it was.
+ */
+function withoutInterfaceSpans(statement: string): string {
+  const at = statement.lastIndexOf(INTERFACE_MARKER)
+  if (at < 0) return statement
+  const block = statement.slice(at)
+  if (!INTERFACE_BLOCK.test(block)) return statement
+  return statement.slice(0, at) + block.replace(/`[^`]*`/g, (span) => `\`${' '.repeat(span.length - 2)}\``)
+}
+
 /** Every banned term the statement contains, in the order the list names them. */
 export function findBannedTerms(statement: string): BannedTermHit[] {
+  const checked = withoutInterfaceSpans(statement)
   const hits: BannedTermHit[] = []
   for (const { term, pattern } of BANNED_TERMS) {
-    const found = pattern.exec(statement)
+    const found = pattern.exec(checked)
     if (found !== null) hits.push({ term, match: found[0], index: found.index })
   }
   return hits

apps/cli/src/lib/dev-review-eval/harbor-tasks.ts

@@ -8,7 +8,8 @@
  *
  *  - `task.toml` — the Harbor version, the suite, the Vinaya renderer version
  *    and the network policy of each phase;
- *  - `instruction.md` — the case's neutral task statement;
+ *  - `instruction.md` — the case's neutral task statement, its `taskStatement`
+ *    in either split;
  *  - `environment/` — the image: the case repository at `/app` at its real
  *    base commit, with every evaluation case file and every held-out file left
  *    out of its working tree and object store, and `case.json`, the neutral
@@ -45,7 +46,7 @@ import {
   type ReadBytes,
   referenceEntry
 } from '../../../tests/eval/generic-suites'
-import { GENERIC_SUITE_IDS, SEEDED_COMMIT_IDENTITY, STATEMENTS_PATH } from '../../../tests/eval/recut-generic-cases'
+import { GENERIC_SUITE_IDS, SEEDED_COMMIT_IDENTITY } from '../../../tests/eval/recut-generic-cases'
 import {
   loadDevelopment as loadReviewerDevelopment,
   loadHeldOut as loadReviewerHeldOut,
@@ -704,10 +705,6 @@ const HELD_OUT_FAMILY: Partial<Record<GenericSuiteKey, string>> = {
   sourceCitation: DERIVED_FILE_DIGESTS.sourceCitation.heldOut
 }
 
-function statementsOf(): Record<string, string> {
-  return JSON.parse(readFileSync(STATEMENTS_PATH, 'utf8')) as Record<string, string>
-}
-
 /** The layer-`P` counterparts a split converts with, by suite. */
 function counterparts(split: 'development' | 'held-out', keys: readonly GenericSuiteKey[]) {
   const developer: DeveloperCapabilityCase[] = []
@@ -762,7 +759,6 @@ export async function convertHarborTasks(options: ConvertOptions): Promise<Conve
       throw new HarborTasksRefused(`the quarantine record names ${stray.taskId}, which ${suite} does not hold`)
   }
   const script = await bundleGradeScript()
-  const statements = split === 'development' ? statementsOf() : {}
   const layerP = counterparts(split, keys)
   const url = options.repositoryUrl ?? GITHUB_REPOSITORY_URL
   // The archive is written first: every task image installs the Vinaya CLI from it at build.
@@ -793,7 +789,7 @@ export async function convertHarborTasks(options: ConvertOptions): Promise<Conve
       const sources: CaseSources = {
         key,
         case: c,
-        statement: split === 'development' ? (statements[c.id] ?? '') : c.taskStatement,
+        statement: c.taskStatement,
         developer: layerP.developer.find((p) => p.id === c.id),
         reviewer: layerP.reviewer.find((p) => p.id === c.id && p.role === role),
         citation: layerP.citation.find((p) => p.id === c.id),

apps/cli/tests/eval/banned-terms.test.ts

@@ -65,6 +65,29 @@ describe('banned-terms checker', () => {
     ])
   })
 
+  it("skips the code spans of a closing Interface block, and still reports every term in the statement's prose", () => {
+    const block = ' Interface: flag `--surface-only`; field `Report.reviewer: string`; string `the brief is ready`.'
+    expect(findBannedTerms(`${NEUTRAL}${block}`)).toEqual([])
+    expect(checkTaskStatement(`${NEUTRAL}${block}`).ok).toBe(true)
+    // The same words in the prose are reported, at the index the statement holds them.
+    const prose = 'Keep the surface flag working for the reviewer.'
+    expect(findBannedTerms(`${prose}${block}`)).toEqual([
+      { term: 'Surface', match: 'surface', index: prose.indexOf('surface') },
+      { term: 'reviewer', match: 'reviewer', index: prose.indexOf('reviewer') }
+    ])
+    // Only a well-formed block that ends the statement is skipped: a marker in the middle, an unknown kind or a
+    // missing full stop leaves every span checked.
+    expect(findBannedTerms(`${NEUTRAL}${block} Then stop.`).map((hit) => hit.term)).toContain('Surface')
+    expect(findBannedTerms(`${NEUTRAL} Interface: name \`--surface-only\`.`).map((hit) => hit.term)).toEqual([
+      'Surface'
+    ])
+    expect(findBannedTerms(`${NEUTRAL} Interface: flag \`--surface-only\``).map((hit) => hit.term)).toEqual(['Surface'])
+    // A span before the last marker is prose.
+    expect(findBannedTerms(`Use \`--surface\`.${' Interface: env `PATH`.'}`).map((hit) => hit.term)).toEqual([
+      'Surface'
+    ])
+  })
+
   it('names each term once', () => {
     const terms = BANNED_TERMS.map((t) => t.term)
     expect(new Set(terms).size).toBe(terms.length)