diff --git a/openspec/changes/hackathon-analysis/apply-progress.md b/openspec/changes/hackathon-analysis/apply-progress.md index 896cf17..c3f462d 100644 --- a/openspec/changes/hackathon-analysis/apply-progress.md +++ b/openspec/changes/hackathon-analysis/apply-progress.md @@ -1285,3 +1285,19 @@ Checked against the repo's generated types and the official docs (developers.clo - **Diagnostics (`feat(hackathon-llm)`, 9cac8b0):** seam chosen: `LlmExtractor.extract` returns `{ value, meta }` (not a sentinel), so the never-throw-for-content contract and `value: null` for unparseable output stay, and the domain builds `attempts` from plain data. `meta` is `finishReason` (plain token <= 20 chars, else `other`), `contentLength` and `parseFailure` (`no-content|unterminated|prose-around|not-json|non-object`). The safe logger re-validates each key (pattern, finite number, fixed-code set); tests prove no content, snippet or hostile finish_reason reaches the log. - **Tolerant extraction (`feat(hackathon-llm)`, c178c8c):** on a failed direct parse the first balanced `{...}` is parsed (string/escape-aware). Decision: a recovery reports `parseFailure: "prose-around"` AND `recovered: true`, so diagnostics still show the model wraps its JSON. Recovered values still pass through `validateExtraction` unchanged (tests: garbage and off-page snippets rejected). Fence, whitespace and `reasoning_content` tests are unchanged and green. - **Docs:** design.md "Extraction Schema and Prompt" updated. + +### Root-cause fix: production-faithful evidence (branch `fix/hackathon-extraction-root-cause`) + +- **Evidence:** a harness running the REAL modules on Cloudflare (`wrangler dev --remote`, real AI and BROWSER, Cloudflare egress) against .../tokenized-stocks showed: (1) the static fetch gets 403, so production always uses the rendered path; its text has TABs (`innerText` table cells). (2) Qwen copies TAB-containing snippets into JSON strings; `JSON.parse` throws "Bad control character", reported as `parseFailure: not-json`; escaping in-string control chars parses all 11 keys. (3) GLM returns unfindable fields as `{value:null,snippet:null,confidence:0}` and the all-or-nothing shape check failed the whole response as `invalid-shape`. (4) `parsed:false` was reported for any `ok:false`, even when JSON parsing succeeded. +- **Decisions:** repair reported as a NEW code `parseFailure: "control-chars"` + `recovered: true` (one signal, allowlisted through the shared code list; inside prose it stays `prose-around`). A MISSING key counts as rejected (`wrong-shape`), not null: treating it as null would let `{"unrelated":true}` become a usable all-null success. `parsed` = JSON parsing succeeded (`value !== null` or `non-object`); `shape: "invalid"` only when parsed and the top level is not a plain object. `THIN_STATIC_TEXT_THRESHOLD` and `isUsable` are now exported so the harness shares the production rule. + +| Work unit | Commit | RED | GREEN | Notes | +|---|---|---|---|---| +| 1 Page text normalization | c04cb08 | 6 failing (tabs in rendered and static output, `normalizePageText` missing) | 770 passed | triangulated: table tabs, control chars/CR, blank-line runs, TEXT_MAX cap | +| 2 Control-char tolerance | 34a6d42 | 6 failing (real TAB shape, CR/LF, prose-around, fence, non-object) | 76+ passed | also asserts structural whitespace untouched and no content in meta | +| 3 Per-field shape | 2841998 | 20 failing | 798 passed | GLM null object, array value, string teamSize, top-level array, majority; old whole-response tests rewritten to per-field; spec + design updated | +| 4 Diagnostics label | 7e077c9 | 4 failing | 803 passed | logger asserts fixed `shape` literal and no value leak | +| 5 Harness | 6dbbcd5 | N/A: needs live Cloudflare bindings (run by the orchestrator, not vitest) | typecheck clean, harness type-checked separately, `wrangler deploy --dry-run` bundles it | excluded from root tsconfig, vitest and the main bundle | + +- **Work Unit Evidence:** focused commands `npx vitest run ` per unit (results above); runtime harness: N/A in this batch (run by the orchestrator against real URLs); rollback boundary: one commit per unit. +- **Verification:** `npm test` 803 passed (70 files), `npm run typecheck` clean. diff --git a/openspec/changes/hackathon-analysis/design.md b/openspec/changes/hackathon-analysis/design.md index af7d7c8..4dfb3a9 100644 --- a/openspec/changes/hackathon-analysis/design.md +++ b/openspec/changes/hackathon-analysis/design.md @@ -74,6 +74,11 @@ index.queue ─ buildHackathonConsumer(env) ─ runHackathonJob(msg) └─ transient error → attempts ≤ 2 ? retry(30 s) : as permanent ("temporary error") ``` +### Worker egress reality + +Verified with the harness (`npm run harness`, real modules on Cloudflare with real AI and BROWSER bindings) against https://www.bnbchain.org/en/hackathons/tokenized-stocks: from Cloudflare egress the static fetch gets HTTP 403, so production always takes the rendered (browser) path, whose text is 2.5k-3.4k chars. Rendered `innerText` separates table cells with TAB characters, and Qwen copies snippets containing raw TABs into JSON string literals, which `JSON.parse` rejects ("Bad control character in string literal"). Consequences in the design: (1) both fetch paths pass their text through `normalizePageText` (every ASCII control character except ` +` becomes a space, space runs collapse per line, 3+ newlines collapse to a blank line; the 22,000 cap still applies), and (2) the extractor repairs control characters in string literals as a second line of defense. GLM returns fields it cannot find as `{ "value": null, "snippet": null, "confidence": 0 }`, handled by the per-field validation above. + The rendered fetcher calls `page.setRequestInterception(true)`. Each request goes through the pure `browserRequestPolicy`: http(s) only, the same host guard, images, fonts, media and stylesheets aborted, and at most 100 requests. After `goto`, it re-checks `page.url()`, reads `innerText` (capped), and calls `browser.close()` in `finally`. A 429 raises `BrowserQuotaExceededError`. The static fetcher uses `redirect: "manual"`. It follows at most 3 hops, re-guarding each one, and streams up to 2 MB. It accepts only a 200 `text/html` or `text/plain` response. `HTMLRewriter` drops noise elements and keeps the title, the meta and OG description and `ld+json` (up to 4 KB). The text is capped at 22,000 chars. @@ -88,7 +93,11 @@ These are unchanged: ## Extraction Schema and Prompt -`Field = { value; snippet; confidence } | null`, where the prompt asks for a snippet of at most 160 characters copied verbatim from a single passage and for concise values, and validation accepts a snippet that is verbatim modulo whitespace (every whitespace run, including line breaks, tabs and NBSP, collapsed to one space and trimmed on both sides; the page is normalized once per response) with a 200-character cap on the normalized snippet, which is the form stored. Each rejection carries a reason code (`empty-snippet`, `snippet-too-long`, `not-verbatim`, `value-too-long`); `ExtractionFailedError` carries per-attempt `{ model, parsed, rejectedCount, rejected: [{ field, reason }] }` diagnostics that the job logs (names and codes only, never snippet text, values or the URL). Each attempt also carries safe parse metadata (`finishReason` capped to a plain token or `other`, `contentLength`, and `parseFailure` in `no-content|unterminated|prose-around|not-json|non-object`), returned by the extractor as `{ value, meta }` and re-validated by the safe logger; no model text is logged. Parsing is tolerant: when a direct parse (after fence stripping) fails, the first balanced JSON object (string- and escape-aware) is parsed instead and, if it is an object, used with `parseFailure: "prose-around"` and `recovered: true`; the recovered value still goes through `validateExtraction` unchanged. Unchanged:, covering name, format, location, team size, four dates, prizes, tracks and eligibility. Invalid fields become null, and so do fields whose snippet is not found in the page. The fallback model is tried when the output is unparseable or more than half of its fields are invalid. The page is framed as untrusted between `<<>>` with those tokens stripped, and the call uses `temperature: 0` and `max_tokens: 2500` (both models are reasoning models whose thinking consumes completion tokens). The input is chat `messages`: fixed instructions in the system message and the framed page in the user message, because a bare `prompt` makes these models do raw text completion. GLM models additionally get `chat_template_kwargs: { enable_thinking: false }` (with thinking on, GLM-4.7-Flash exhausts max_tokens and returns truncated JSON; Qwen3-30B returns null content if thinking is disabled, so only GLM gets it). +`Field = { value; snippet; confidence } | null`, where the prompt asks for a snippet of at most 160 characters copied verbatim from a single passage and for concise values, and validation accepts a snippet that is verbatim modulo whitespace (every whitespace run, including line breaks, tabs and NBSP, collapsed to one space and trimmed on both sides; the page is normalized once per response) with a 200-character cap on the normalized snippet, which is the form stored. Validation is per field: only a top-level value that is not a plain object fails the whole response (`invalid-shape`). A field object whose `value` is null (GLM returns `{ "value": null, "snippet": null, "confidence": 0 }` for what it cannot find) is a model-null and is not counted as rejected. A missing key, a non-object field, a wrong value type, a non-string snippet or a non-number confidence rejects that field alone with `wrong-shape` (nulled and counted in `rejectedCount`); a missing key counts as rejected, not null, so a garbage object such as `{ "unrelated": true }` cannot become a usable all-null success. Each rejection carries a reason code (`empty-snippet`, `snippet-too-long`, `not-verbatim`, `value-too-long`, `wrong-shape`); `ExtractionFailedError` carries per-attempt `{ model, parsed, rejectedCount, rejected: [{ field, reason }] }` diagnostics that the job logs (names and codes only, never snippet text, values or the URL). Each attempt also carries safe parse metadata (`finishReason` capped to a plain token or `other`, `contentLength`, and `parseFailure` in `no-content|unterminated|prose-around|control-chars|not-json|non-object`), returned by the extractor as `{ value, meta }` and re-validated by the safe logger; no model text is logged. Parsing is tolerant: when a direct parse (after fence stripping) fails, the first balanced JSON object (string- and escape-aware) is parsed instead and, if it is an object, used with `parseFailure: "prose-around"` and `recovered: true`; the recovered value still goes through `validateExtraction` unchanged. Control-character repair: when the direct parse fails, raw TAB/CR/LF inside JSON string literals are escaped by a string-aware scan (structural whitespace and existing escapes untouched) and the parse is retried once; success is reported as `parseFailure: "control-chars"` with `recovered: true` (a repair inside a prose-wrapped object stays `prose-around`). The safe logger allowlists the code from the same fixed list. Diagnostics: `parsed` means ONLY that JSON parsing succeeded; a top-level validation failure on a parsed value is reported as `shape: "invalid"` (logged as a fixed literal), and per-field problems appear in `rejected[]` with their reason code. Unchanged:, covering name, format, location, team size, four dates, prizes, tracks and eligibility. Invalid fields become null, and so do fields whose snippet is not found in the page. The fallback model is tried when the output is unparseable or more than half of its fields are invalid. The page is framed as untrusted between `<<>>` with those tokens stripped, and the call uses `temperature: 0` and `max_tokens: 2500` (both models are reasoning models whose thinking consumes completion tokens). The input is chat `messages`: fixed instructions in the system message and the framed page in the user message, because a bare `prompt` makes these models do raw text completion. GLM models additionally get `chat_template_kwargs: { enable_thinking: false }` (with thinking on, GLM-4.7-Flash exhausts max_tokens and returns truncated JSON; Qwen3-30B returns null content if thinking is disabled, so only GLM gets it). + +### Verification + +Local tests fake the network, the AI binding and the browser, so they cannot show what production sees. Any change to the fetch, LLM or validation path MUST be checked with `npm run harness` (`scripts/hackathon-harness/`) against real URLs before merge. It costs Workers AI neurons and Browser Rendering time. ## Migration `0003_hackathon_analysis.sql` diff --git a/openspec/changes/hackathon-analysis/specs/llm-extraction/spec.md b/openspec/changes/hackathon-analysis/specs/llm-extraction/spec.md index 07086f3..0e57e2d 100644 --- a/openspec/changes/hackathon-analysis/specs/llm-extraction/spec.md +++ b/openspec/changes/hackathon-analysis/specs/llm-extraction/spec.md @@ -8,7 +8,7 @@ Turns a fetched page's reduced text into a strict, nullable-field hackathon reco ### Requirement: Strict Schema Output -The system MUST request extraction against a fixed schema (format, team-size cap, dates, prizes, tracks, name) and MUST validate the model's response against that schema before it reaches the domain layer, rejecting any response that fails validation. +The system MUST request extraction against a fixed schema (format, team-size cap, dates, prizes, tracks, name) and MUST validate the model's response against that schema before it reaches the domain layer. A response that is not a JSON object is rejected as a whole; a malformed field is rejected individually. #### Scenario: Well-formed response passes validation @@ -18,11 +18,25 @@ The system MUST request extraction against a fixed schema (format, team-size cap #### Scenario: Malformed response is rejected -- GIVEN the model returns a response that does not match the fixed schema (missing required shape, wrong types, or unparseable output) +- GIVEN the model returns a response whose top level is not a plain JSON object (unparseable output, an array, a string, ...) - WHEN the adapter validates it -- THEN the system MUST treat this as an extraction failure +- THEN the system MUST treat this as an extraction failure (`invalid-shape`) - AND MUST NOT pass partial or malformed data to the domain layer +#### Scenario: Malformed field is rejected individually + +- GIVEN the response is a JSON object but one field is malformed (a missing key, a field that is not an object, a wrong value type, a non-string snippet or a non-number confidence) +- WHEN the adapter validates it +- THEN only that field MUST be rejected with the reason `wrong-shape`, stored as null and counted in `rejectedCount` +- AND the other fields MUST be validated normally +- AND the response is usable only while no more than half of the fields are rejected (otherwise the fallback model runs) + +#### Scenario: A field the model reports as not found is null, not rejected + +- GIVEN the model returns a field object whose `value` is null (for example `{ "value": null, "snippet": null, "confidence": 0 }`) +- WHEN the adapter validates it +- THEN the field MUST be null and MUST NOT count as rejected, whatever its snippet or confidence hold + ### Requirement: Null Over Guess for Every Field The system MUST represent a field the page does not clearly state as null rather than an invented value, for every field in the schema. diff --git a/package.json b/package.json index ed7b6c3..e0872df 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,8 @@ "test": "vitest run", "test:watch": "vitest", "typecheck": "tsc --noEmit", - "cf-typegen": "wrangler types" + "cf-typegen": "wrangler types", + "harness": "wrangler dev --remote --config scripts/hackathon-harness/wrangler.jsonc --port 8799" }, "dependencies": { "@cloudflare/puppeteer": "1.4.0", diff --git a/scripts/hackathon-harness/README.md b/scripts/hackathon-harness/README.md new file mode 100644 index 0000000..a3f320c --- /dev/null +++ b/scripts/hackathon-harness/README.md @@ -0,0 +1,48 @@ +# Hackathon analysis harness + +A dev-only Worker that runs the REAL `src/` modules (static fetcher, rendered +fetcher, Workers AI extractor, `validateExtraction`) on Cloudflare with real +`AI` and `BROWSER` bindings and Cloudflare egress. Unit tests fake all of +these, so they cannot show what production sees (for example, a static fetch +that gets HTTP 403 from Cloudflare egress, or a model copying raw TABs from +rendered page text). This harness can. + +It is never deployed, and it is not part of the main Worker bundle, vitest or +`npm run typecheck` (`tsconfig.json` only includes `src`, `test` and +`vitest.config.ts`). + +## Cost + +Every request runs `wrangler dev --remote`, so it consumes real Workers AI +neurons (one call per model) and Browser Rendering time, billed to your +Cloudflare account. It needs `wrangler login` (or `CLOUDFLARE_API_TOKEN`; set +`CLOUDFLARE_ACCOUNT_ID` if you have several accounts). No DB, queue or +secrets are used. + +## Run + +``` +npm run harness +# in another terminal: +curl "http://localhost:8799/?url=https://www.bnbchain.org/en/hackathons/tokenized-stocks" +``` + +Optional: `&models=@cf/model-a,@cf/model-b` overrides the model IDs (the +default is the pair in the root `wrangler.jsonc`). + +## Output + +JSON with: + +- `static` / `rendered`: ok, text length, or the error kind and HTTP status. +- `chosen`: `static` or `rendered`, using the same rule as + `analyzeHackathon` (rendered when the static fetch is bot-walled or thin). +- `pageText`: length and the first 1500 characters of the chosen text. +- `attempts[]`, one per model: `finish` reason, extractor `meta` + (`parseFailure`, `recovered`, `contentLength`), the first 2500 characters of + the model content, the `validateExtraction` result and `usable`. + +## When to use it + +Any change to the fetch, LLM or validation path MUST be checked with +`npm run harness` against real URLs before merge. diff --git a/scripts/hackathon-harness/harness.ts b/scripts/hackathon-harness/harness.ts new file mode 100644 index 0000000..ba373dd --- /dev/null +++ b/scripts/hackathon-harness/harness.ts @@ -0,0 +1,119 @@ +// Production-faithful harness for the hackathon analysis path. Run it with +// `npm run harness` (wrangler dev --remote): it executes the REAL src modules +// on Cloudflare, with real Workers AI and Browser Rendering bindings and +// Cloudflare egress, and reports every intermediate artifact. Never deployed +// and never part of the main Worker bundle, vitest or `npm run typecheck`. +import { launchPuppeteerBrowser } from "../../src/adapters/browser/puppeteer-launch"; +import { createRenderedFetcher } from "../../src/adapters/browser/rendered-fetcher"; +import { createStaticFetcher } from "../../src/adapters/http/safe-fetcher"; +import { createWorkersAiExtractor } from "../../src/adapters/llm/workers-ai-extractor"; +import { validateExtraction } from "../../src/domain/hackathon/extraction"; +import { isUsable, THIN_STATIC_TEXT_THRESHOLD } from "../../src/domain/usecases/analyze-hackathon"; + +interface Env { + AI: { run: (model: string, inputs: unknown, options?: unknown) => Promise }; + BROWSER: unknown; +} + +// Keep in sync with HACKATHON_MODEL_PRIMARY / HACKATHON_MODEL_FALLBACK in the +// root wrangler.jsonc (a JSONC file with comments cannot be imported here). +// `?models=a,b` overrides them for one request. +const DEFAULT_MODELS = ["@cf/zai-org/glm-4.7-flash", "@cf/qwen/qwen3-30b-a3b-fp8"]; + +const PAGE_HEAD_CHARS = 1500; +const CONTENT_HEAD_CHARS = 2500; + +function describeError(err: unknown) { + if (err instanceof Error) { + const e = err as Error & { kind?: string; status?: number; reason?: string }; + return { name: e.name, message: e.message.slice(0, 200), kind: e.kind, status: e.status, reason: e.reason }; + } + return { value: String(err) }; +} + +function asRecord(value: unknown): Record | null { + return value !== null && typeof value === "object" ? (value as Record) : null; +} + +export default { + async fetch(request: Request, env: Env): Promise { + const params = new URL(request.url).searchParams; + const target = params.get("url"); + if (!target) return new Response("?url= required", { status: 400 }); + const models = params.get("models")?.split(",").filter(Boolean) ?? DEFAULT_MODELS; + const report: Record = { url: target }; + + // 1. Static fetch, exactly as production. + let staticText = ""; + let staticOk = false; + try { + staticText = await createStaticFetcher({ fetch: (i, init) => fetch(i, init) }).fetch( + target, + AbortSignal.timeout(15_000), + ); + staticOk = true; + report.static = { ok: true, length: staticText.length }; + } catch (err) { + report.static = { ok: false, error: describeError(err) }; + } + + // 2. Rendered fetch, always, so both texts can be compared. + let renderedText = ""; + try { + renderedText = await createRenderedFetcher({ + launch: launchPuppeteerBrowser, + binding: env.BROWSER, + }).fetch(target, AbortSignal.timeout(40_000)); + report.rendered = { ok: true, length: renderedText.length }; + } catch (err) { + report.rendered = { ok: false, error: describeError(err) }; + } + + // 3. Same selection rule as analyze-hackathon's resolvePageText: the + // rendered text is used when the static fetch failed (bot wall) or its + // text is thin. + const useRendered = !staticOk || staticText.length < THIN_STATIC_TEXT_THRESHOLD; + const pageText = useRendered ? renderedText : staticText; + report.chosen = useRendered ? "rendered" : "static"; + report.pageText = { length: pageText.length, head: pageText.slice(0, PAGE_HEAD_CHARS) }; + + // 4. Each model through the REAL extractor, capturing the raw binding result. + const attempts: unknown[] = []; + for (const model of models) { + let rawResult: unknown = null; + const extractor = createWorkersAiExtractor({ + run: async (m, inputs, options) => { + rawResult = await env.AI.run(m, inputs, options); + return rawResult; + }, + }); + const t0 = Date.now(); + try { + const out = await extractor.extract(pageText, model, AbortSignal.timeout(90_000)); + const validated = validateExtraction(out.value, pageText); + const raw = asRecord(rawResult); + const choice = asRecord((raw?.choices as unknown[] | undefined)?.[0]); + const message = asRecord(choice?.message); + const content = + typeof message?.content === "string" + ? message.content + : JSON.stringify(raw?.response ?? message?.content ?? null); + attempts.push({ + model, + ms: Date.now() - t0, + finish: choice?.finish_reason, + meta: out.meta, + contentHead: content.slice(0, CONTENT_HEAD_CHARS), + validate: validated.ok + ? { ok: true, rejectedCount: validated.rejectedCount, rejections: validated.rejections } + : validated, + usable: isUsable(validated), + }); + } catch (err) { + attempts.push({ model, ms: Date.now() - t0, threw: describeError(err) }); + } + } + report.attempts = attempts; + return Response.json(report); + }, +}; diff --git a/scripts/hackathon-harness/wrangler.jsonc b/scripts/hackathon-harness/wrangler.jsonc new file mode 100644 index 0000000..852b2f4 --- /dev/null +++ b/scripts/hackathon-harness/wrangler.jsonc @@ -0,0 +1,13 @@ +{ + // Production-faithful harness for the hackathon analysis path. Dev-only: + // never deployed. No DB, queue or secrets; only the two remote bindings. + "name": "hack-bot-harness", + "main": "harness.ts", + // Same compatibility date and flags as the root wrangler.jsonc, so the + // real src modules behave as they do in production. + "compatibility_date": "2026-08-22", + "compatibility_flags": ["nodejs_compat"], + "workers_dev": false, + "ai": { "binding": "AI" }, + "browser": { "binding": "BROWSER" } +} diff --git a/src/adapters/browser/rendered-fetcher.ts b/src/adapters/browser/rendered-fetcher.ts index 1d1b0b4..bd2cb99 100644 --- a/src/adapters/browser/rendered-fetcher.ts +++ b/src/adapters/browser/rendered-fetcher.ts @@ -1,7 +1,7 @@ import { BrowserQuotaExceededError, PageFetchFailedError, UnsafeUrlError } from "../../domain/errors"; import { assertSafeUrl } from "../../domain/hackathon/url"; import type { PageFetcher } from "../../domain/ports"; -import { TEXT_MAX } from "../http/html-to-text"; +import { normalizePageText, TEXT_MAX } from "../http/html-to-text"; // Browser (Cloudflare Browser Rendering / @cloudflare/puppeteer) page fetch // for JS-heavy pages the static fetcher's text falls short on (design.md @@ -176,7 +176,7 @@ export function createRenderedFetcher(options: RenderedFetcherOptions): PageFetc } throw new PageFetchFailedError("rendered page text extraction failed", "network"); } - return innerText.slice(0, TEXT_MAX); + return normalizePageText(innerText).slice(0, TEXT_MAX); } finally { // Always released, even on refusal/timeout/quota (design.md "calls // browser.close() in finally"). A failing close must not replace the diff --git a/src/adapters/http/html-to-text.ts b/src/adapters/http/html-to-text.ts index a46f017..504053f 100644 --- a/src/adapters/http/html-to-text.ts +++ b/src/adapters/http/html-to-text.ts @@ -29,6 +29,23 @@ const NOISE_TAGS = [ const LD_JSON_MAX = 4_000; // design.md: "ld+json (up to 4 KB)" export const TEXT_MAX = 22_000; // design.md: "capped at 22,000 chars" +// Normalizes page text before it reaches the LLM. Rendered `innerText` +// separates table cells with TAB characters; models copy snippets containing +// raw TABs into JSON string literals, which JSON.parse rejects ("Bad control +// character in string literal"). So every ASCII control character except +// "\n" (TAB, CR, NUL, ...) becomes a space, runs of spaces collapse within a +// line, lines are trimmed, and 3+ consecutive newlines collapse to a single +// blank line. Paragraph breaks ("\n\n") are kept. +export function normalizePageText(text: string): string { + return text + .replace(/[\u0000-\u0009\u000B-\u001F\u007F]/g, " ") + .split("\n") + .map((line) => line.replace(/ {2,}/g, " ").trim()) + .join("\n") + .replace(/\n{3,}/g, "\n\n") + .trim(); +} + export async function htmlToText(response: Response): Promise { let title = ""; let metaDescription = ""; @@ -110,5 +127,5 @@ export async function htmlToText(response: Response): Promise { const visibleText = body.join("").replace(/\s+/g, " ").trim(); if (visibleText) parts.push(visibleText); - return parts.join("\n\n").slice(0, TEXT_MAX); + return normalizePageText(parts.join("\n\n")).slice(0, TEXT_MAX); } diff --git a/src/adapters/llm/workers-ai-extractor.ts b/src/adapters/llm/workers-ai-extractor.ts index 8f10692..51a9f41 100644 --- a/src/adapters/llm/workers-ai-extractor.ts +++ b/src/adapters/llm/workers-ai-extractor.ts @@ -124,7 +124,11 @@ function isQuotaExhausted(err: unknown): boolean { // first balanced JSON object in the text is parsed instead (string- and // escape-aware). If that yields an object it is returned with meta // { parseFailure: "prose-around", recovered: true } — the failure case is still -// reported so the diagnostics show the model wraps its JSON in prose. The +// reported so the diagnostics show the model wraps its JSON in prose. When +// the text (or the balanced object) only fails because string literals hold +// raw TAB/CR/LF characters, they are escaped and the parse retried once; that +// is reported as { parseFailure: "control-chars", recovered: true } (inside +// prose it stays "prose-around"). The // recovered value is as untrusted as any other and still goes through // validateExtraction unchanged. const CODE_FENCE_PATTERN = /^\s*```[A-Za-z]*\s*\n([\s\S]*?)\n?\s*```\s*$/; @@ -182,6 +186,40 @@ function tryParse(text: string): { ok: true; value: unknown } | { ok: false } { } } +// Escapes raw TAB, CR and LF characters that appear INSIDE JSON string +// literals (models copy TAB-separated table text verbatim into a snippet, +// which JSON.parse rejects as a "Bad control character"). A string-aware +// scan: whitespace outside string literals is structural and stays +// untouched, and an existing backslash escape is copied through so it is +// never double-escaped. +const CONTROL_ESCAPES: Record = { "\t": "\\t", "\r": "\\r", "\n": "\\n" }; + +function escapeControlCharsInStrings(text: string): string { + let out = ""; + let inString = false; + let escaped = false; + for (const ch of text) { + if (!inString) { + if (ch === '"') inString = true; + out += ch; + continue; + } + if (escaped) { + escaped = false; + out += ch; + } else if (ch === "\\") { + escaped = true; + out += ch; + } else if (ch === '"') { + inString = false; + out += ch; + } else { + out += CONTROL_ESCAPES[ch] ?? ch; + } + } + return out; +} + function parseJsonText(text: string): ParsedText { if (text.trim() === "") return { value: null, parseFailure: "no-content" }; const fenced = CODE_FENCE_PATTERN.exec(text); @@ -194,10 +232,19 @@ function parseJsonText(text: string): ParsedText { : { value: direct.value, parseFailure: "non-object" }; } + // One repair retry: raw control characters inside string literals. + const repaired = tryParse(escapeControlCharsInStrings(body)); + if (repaired.ok) { + return isPlainObject(repaired.value) + ? { value: repaired.value, parseFailure: "control-chars", recovered: true } + : { value: repaired.value, parseFailure: "non-object" }; + } + const scan = scanBalancedObject(body); if (scan.kind === "unterminated") return { value: null, parseFailure: "unterminated" }; if (scan.kind === "found") { - const inner = tryParse(scan.text); + let inner = tryParse(scan.text); + if (!inner.ok) inner = tryParse(escapeControlCharsInStrings(scan.text)); if (inner.ok && isPlainObject(inner.value)) { return { value: inner.value, parseFailure: "prose-around", recovered: true }; } diff --git a/src/adapters/log/safe-logger.ts b/src/adapters/log/safe-logger.ts index 92d51d8..726e12e 100644 --- a/src/adapters/log/safe-logger.ts +++ b/src/adapters/log/safe-logger.ts @@ -50,6 +50,8 @@ export function createSafeLogger(): Logger { attempts: entry.attempts.map((a) => ({ model: a.model, parsed: a.parsed, + // Fixed literal only; anything else is dropped. + ...(a.shape === "invalid" ? { shape: "invalid" as const } : {}), rejectedCount: a.rejectedCount, rejected: a.rejected.map((r) => ({ field: r.field, reason: r.reason })), ...safeParseMeta(a), diff --git a/src/domain/errors.ts b/src/domain/errors.ts index a8cb433..289047d 100644 --- a/src/domain/errors.ts +++ b/src/domain/errors.ts @@ -129,6 +129,7 @@ export type LlmParseFailureCode = | "no-content" // null, missing or empty content | "unterminated" // has a "{" but no matching closing "}" (truncated) | "prose-around" // a parsable JSON object exists, wrapped in other text + | "control-chars" // parsed only after escaping raw TAB/CR/LF inside string literals | "not-json" // no JSON object in the text | "non-object"; // valid JSON, but not an object (array, number, ...) @@ -136,14 +137,20 @@ export const LLM_PARSE_FAILURE_CODES: readonly LlmParseFailureCode[] = [ "no-content", "unterminated", "prose-around", + "control-chars", "not-json", "non-object", ]; export interface ExtractionAttemptDiagnostics { model: string; - // false when the response failed schema validation as a whole. + // true ONLY when the model's text was parsed as JSON (possibly after + // tolerant recovery). It says nothing about whether the shape validated. parsed: boolean; + // "invalid" when JSON parsed but the top level failed validation as a + // whole (not a plain object). Absent otherwise; per-field shape problems + // show up in `rejected` with reason "wrong-shape" instead. + shape?: "invalid"; rejectedCount: number; rejected: FieldRejection[]; // Parse metadata (numbers and fixed codes only — never model content). diff --git a/src/domain/hackathon/extraction.ts b/src/domain/hackathon/extraction.ts index fe0f88e..a8cef68 100644 --- a/src/domain/hackathon/extraction.ts +++ b/src/domain/hackathon/extraction.ts @@ -1,10 +1,11 @@ // Strict schema validation for the LLM's extracted hackathon fields (spec // llm-extraction: "Strict Schema Output", "Null Over Guess for Every // Field", "Bounded Source Snippet Per Non-Null Field"). This is the ONLY -// place a raw model response is trusted to become domain data — invalid -// shape rejects the whole response (design.md "Validation"), while a -// per-field problem (an unfindable or oversized snippet) demotes only that -// field to null rather than failing the whole extraction. +// place a raw model response is trusted to become domain data — a top-level +// non-object rejects the whole response (design.md "Validation"), while a +// per-field problem (a wrong-shape field, an unfindable or oversized +// snippet) demotes only that field to null rather than failing the whole +// extraction. // A field is either present with a bounded, verbatim-checkable snippet, or // entirely absent (null over guess). @@ -36,13 +37,14 @@ export interface ExtractedFields { // returned as `null`. The use case layer uses it to decide whether the // fallback model should be tried or preferred (analyze-hackathon.ts // "more than half of its fields are invalid"). -// Why content validation nulled a field. Fixed codes only — safe to log +// Why validation nulled a field. Fixed codes only — safe to log // (never carries snippet or value text). export type RejectionReason = | "empty-snippet" | "snippet-too-long" | "not-verbatim" - | "value-too-long"; + | "value-too-long" + | "wrong-shape"; // missing key, non-object field, wrong value/snippet/confidence type export interface FieldRejection { field: string; @@ -108,30 +110,17 @@ export function validateExtraction( const fields = {} as ExtractedFields; const rejections: FieldRejection[] = []; for (const name of FIELD_NAMES) { - if (!(name in record)) { - return { ok: false, reason: "invalid-shape" }; - } - const rawField = record[name]; - if (rawField === null) { + const shaped = shapeField(name, record[name]); + if (shaped.kind === "null") { fields[name] = null; continue; } - if (typeof rawField !== "object" || Array.isArray(rawField)) { - return { ok: false, reason: "invalid-shape" }; - } - const candidate = rawField as Record; - if ( - !("value" in candidate) || - typeof candidate.snippet !== "string" || - typeof candidate.confidence !== "number" || - !hasValidFieldType(name, candidate.value) - ) { - return { ok: false, reason: "invalid-shape" }; + if (shaped.kind === "wrong-shape") { + rejections.push({ field: name, reason: "wrong-shape" }); + fields[name] = null; + continue; } - const sanitized = sanitizeField( - { value: candidate.value, snippet: candidate.snippet, confidence: candidate.confidence }, - normalizedPage, - ); + const sanitized = sanitizeField(shaped.field, normalizedPage); // The model supplied a non-null field, but content validation nulled // it (RELI-001) — distinct from a field the model itself returned as // null, which is never counted as rejected. @@ -146,10 +135,42 @@ export function validateExtraction( return { ok: true, fields, rejectedCount: rejections.length, rejections }; } -// Checks `value` against the declared type of the field (spec -// llm-extraction: "Malformed response is rejected" — wrong types reject the -// whole response, RELI-001/RESI-001). `teamSize` is the only numeric field; -// every other field is a string. +type ShapedField = + | { kind: "null" } + | { kind: "wrong-shape" } + | { kind: "field"; field: ExtractedField }; + +// Per-field shape check (spec llm-extraction: "Malformed field is rejected +// individually"). Only a top-level non-object fails the whole response; a +// problem with one field costs that field alone. +// - explicit `null` is the model saying "not found" -> null, not rejected; +// - an object whose `value` is null is the same statement in the shape GLM +// actually emits ({ value: null, snippet: null, confidence: 0 }), so it +// is a model-null whatever its snippet or confidence hold; +// - a MISSING key, a non-object, a wrong value type (`teamSize` is the +// only numeric field; every other field is a string), a non-string +// snippet or a non-number confidence is "wrong-shape": nulled and +// counted in rejectedCount. A missing key counts as rejected rather than +// null so a garbage object such as { "unrelated": true } cannot become a +// usable all-null success: null over guess is an explicit null. +function shapeField(name: keyof ExtractedFields, rawField: unknown): ShapedField { + if (rawField === null) return { kind: "null" }; + if (typeof rawField !== "object" || Array.isArray(rawField)) return { kind: "wrong-shape" }; + const candidate = rawField as Record; + if (candidate.value === null) return { kind: "null" }; + if ( + typeof candidate.snippet !== "string" || + typeof candidate.confidence !== "number" || + !hasValidFieldType(name, candidate.value) + ) { + return { kind: "wrong-shape" }; + } + return { + kind: "field", + field: { value: candidate.value, snippet: candidate.snippet, confidence: candidate.confidence }, + }; +} + function hasValidFieldType(name: keyof ExtractedFields, value: unknown): boolean { if (name === "teamSize") { return typeof value === "number" && Number.isFinite(value); diff --git a/src/domain/usecases/analyze-hackathon.ts b/src/domain/usecases/analyze-hackathon.ts index a2295da..82dff0c 100644 --- a/src/domain/usecases/analyze-hackathon.ts +++ b/src/domain/usecases/analyze-hackathon.ts @@ -21,7 +21,7 @@ import type { HackathonAnalysisRepo, IdGen, LlmExtractor, - LlmOutputMeta, + LlmExtraction, PageFetcher, RepoTopicLinkRepo, } from "../ports"; @@ -29,7 +29,7 @@ import type { // design.md "Fallback policy": below this many chars of static text, the // rendered (browser) fetch is tried (spec page-fetch: "Browser Rendering // Fallback on Thin Static Text"). -const THIN_STATIC_TEXT_THRESHOLD = 800; +export const THIN_STATIC_TEXT_THRESHOLD = 800; // spec page-fetch "Browser Rendering Quota Exhaustion Degrades or Fails // Based on Static Text Length": the floor above which a 429 degrades to @@ -210,7 +210,7 @@ async function resolvePageText( // as null. const MAJORITY_REJECTED_THRESHOLD = Math.floor(FIELD_COUNT / 2); -function isUsable( +export function isUsable( result: ValidateExtractionResult, ): result is Extract { return result.ok && result.rejectedCount <= MAJORITY_REJECTED_THRESHOLD; @@ -219,11 +219,24 @@ function isUsable( function diagnosticsOf( model: string, result: ValidateExtractionResult, - meta: LlmOutputMeta | undefined, + out: LlmExtraction, ): ExtractionAttemptDiagnostics { + const { meta } = out; + // `parsed` means ONLY that JSON parsing succeeded. The extractor hands over + // `value: null` when the text did not parse; a parsed non-object (array, + // number, ...) keeps its value, or is a JSON `null` flagged "non-object". + const parsed = out.value !== null || meta?.parseFailure === "non-object"; const base: ExtractionAttemptDiagnostics = result.ok - ? { model, parsed: true, rejectedCount: result.rejectedCount, rejected: result.rejections } - : { model, parsed: false, rejectedCount: 0, rejected: [] }; + ? { model, parsed, rejectedCount: result.rejectedCount, rejected: result.rejections } + : { + model, + parsed, + // Only a parsed value can fail on shape; an unparseable text is + // reported by parseFailure alone. + ...(parsed ? { shape: "invalid" as const } : {}), + rejectedCount: 0, + rejected: [], + }; return { ...base, ...(meta?.finishReason !== undefined ? { finishReason: meta.finishReason } : {}), @@ -257,7 +270,7 @@ async function extractFields( throw new ExtractionFailedError( "Primary model output was unusable and too little time remains for the fallback", "timeout", - [diagnosticsOf(input.primaryModel, primary, primaryOut.meta)], + [diagnosticsOf(input.primaryModel, primary, primaryOut)], ); } @@ -278,8 +291,8 @@ async function extractFields( "Both the primary and fallback model produced too many invalid fields", "invalid-output", [ - diagnosticsOf(input.primaryModel, primary, primaryOut.meta), - diagnosticsOf(input.fallbackModel, fallback, fallbackOut.meta), + diagnosticsOf(input.primaryModel, primary, primaryOut), + diagnosticsOf(input.fallbackModel, fallback, fallbackOut), ], ); } diff --git a/test/adapters/browser/rendered-fetcher.test.ts b/test/adapters/browser/rendered-fetcher.test.ts index 40f0b93..2b17238 100644 --- a/test/adapters/browser/rendered-fetcher.test.ts +++ b/test/adapters/browser/rendered-fetcher.test.ts @@ -6,6 +6,7 @@ import { type BrowserPage, type InterceptedRequest, } from "../../../src/adapters/browser/rendered-fetcher"; +import { TEXT_MAX } from "../../../src/adapters/http/html-to-text"; import { BrowserQuotaExceededError, PageFetchFailedError, UnsafeUrlError } from "../../../src/domain/errors"; // task 7.1 (page-fetch spec: "Same Guard Applies to the Browser Fallback", @@ -149,6 +150,25 @@ describe("createRenderedFetcher", () => { expect(isClosed()).toBe(true); }); + it("returns inner text from a table with tabs without any tab characters", async () => { + const { browser } = fakeBrowser({ innerText: "Prize\tAmount\n1st\t$5,000\n2nd\t\t$2,000" }); + const fetcher = createRenderedFetcher({ launch: async () => browser, binding: {} }); + + const text = await fetcher.fetch("https://example.com/event", new AbortController().signal); + + expect(text).toBe("Prize Amount\n1st $5,000\n2nd $2,000"); + }); + + it("still caps normalized rendered text at TEXT_MAX", async () => { + const { browser } = fakeBrowser({ innerText: "word\t".repeat(10_000) }); + const fetcher = createRenderedFetcher({ launch: async () => browser, binding: {} }); + + const text = await fetcher.fetch("https://example.com/event", new AbortController().signal); + + expect(text.length).toBeLessThanOrEqual(TEXT_MAX); + expect(text).not.toContain("\t"); + }); + it("refuses the initial URL before launching the browser (spec: Same Guard Applies to the Browser Fallback)", async () => { let launched = false; const fetcher = createRenderedFetcher({ diff --git a/test/adapters/http/html-to-text.test.ts b/test/adapters/http/html-to-text.test.ts index 71e1e8f..09ad63e 100644 --- a/test/adapters/http/html-to-text.test.ts +++ b/test/adapters/http/html-to-text.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { htmlToText, TEXT_MAX } from "../../../src/adapters/http/html-to-text"; +import { htmlToText, normalizePageText, TEXT_MAX } from "../../../src/adapters/http/html-to-text"; // task 6.3 (design.md "HTMLRewriter drops noise elements and keeps the // title, the meta and OG description and ld+json (up to 4 KB). The text is @@ -95,4 +95,29 @@ describe("htmlToText", () => { expect(text.length).toBeLessThanOrEqual(TEXT_MAX); }); + + it("removes tabs and control characters from the static output", async () => { + const ld = '{"@type":"Event",\t"name":"Hack\u0007Night"}'; + const html = `

Body copy.

`; + + const text = await htmlToText(htmlResponse(html)); + + expect(text).not.toMatch(/[\t\u0000-\u0008\u000B-\u001F\u007F]/); + expect(text).toContain("Body copy."); + }); +}); + +describe("normalizePageText", () => { + it("replaces tabs from table-like innerText with single spaces and keeps newlines", () => { + const raw = "Prizes\n\n1st place\t$5,000\t\tCash\n2nd place\t$2,000"; + expect(normalizePageText(raw)).toBe("Prizes\n\n1st place $5,000 Cash\n2nd place $2,000"); + }); + + it("replaces other ASCII control characters and CR, collapses space runs, trims lines", () => { + expect(normalizePageText("a\u0000b\u001Fc\r\n d e \u007F f ")).toBe("a b c\nd e f"); + }); + + it("keeps paragraph breaks but collapses runs of 3+ blank lines to one blank line", () => { + expect(normalizePageText("one\n\n\n\n\ntwo\n\nthree")).toBe("one\n\ntwo\n\nthree"); + }); }); diff --git a/test/adapters/llm/workers-ai-extractor.test.ts b/test/adapters/llm/workers-ai-extractor.test.ts index 9ca0a90..989ca93 100644 --- a/test/adapters/llm/workers-ai-extractor.test.ts +++ b/test/adapters/llm/workers-ai-extractor.test.ts @@ -129,7 +129,8 @@ ${PAGE_END}` }, const extractor = createWorkersAiExtractor({ run }); const { value: result } = await extractor.extract(PAGE_TEXT, VALID_MODEL, neverAborts()); - expect(validateExtraction(result, PAGE_TEXT)).toEqual({ ok: false, reason: "invalid-shape" }); + const validated = validateExtraction(result, PAGE_TEXT); + expect(validated.ok && validated.rejectedCount).toBe(11); // every key missing: unusable }); it("maps a quota-exhaustion error from run() to LlmQuotaExceededError", async () => { @@ -468,7 +469,8 @@ describe("createWorkersAiExtractor tolerant extraction", () => { it("recovered garbage is still rejected by validateExtraction (untrusted content path unchanged)", async () => { const out = await extractContent('Sure: {"unrelated": true} bye'); expect(out.value).toEqual({ unrelated: true }); - expect(validateExtraction(out.value, PAGE_TEXT)).toEqual({ ok: false, reason: "invalid-shape" }); + const validated = validateExtraction(out.value, PAGE_TEXT); + expect(validated.ok && validated.rejectedCount).toBe(11); // every key missing: unusable }); it("a recovered object still cannot smuggle a snippet that is not on the page", async () => { @@ -478,3 +480,70 @@ describe("createWorkersAiExtractor tolerant extraction", () => { expect(validated.ok && validated.fields.name).toBeFalsy(); }); }); + +// Production evidence (Cloudflare egress, bnbchain page): Qwen copies snippets +// that contain raw TAB characters (table cells in rendered innerText) into +// JSON string literals, and JSON.parse throws "Bad control character in +// string literal". Raw \t \r \n inside string literals are escaped and the +// parse retried once; the recovery is reported as parseFailure "control-chars" +// with recovered: true (fixed code, allowlisted by the safe logger). +describe("createWorkersAiExtractor control-character repair", () => { + const extractContent = (content: string) => + createWorkersAiExtractor({ + run: async () => ({ choices: [{ message: { content }, finish_reason: "stop" }] }), + }).extract(PAGE_TEXT, VALID_MODEL, neverAborts()); + + it("recovers a string literal that contains a raw TAB and reports control-chars + recovered", async () => { + const content = '{"prizes":{"value":"1st\t$5,000","snippet":"1st\t$5,000","confidence":0.9}}'; + expect(() => JSON.parse(content)).toThrow(); + const out = await extractContent(content); + expect(out.value).toEqual({ + prizes: { value: "1st\t$5,000", snippet: "1st\t$5,000", confidence: 0.9 }, + }); + expect(out.meta).toMatchObject({ parseFailure: "control-chars", recovered: true }); + }); + + it("escapes raw CR and LF inside string literals too", async () => { + const out = await extractContent('{"a":"line1\r\nline2","b":"x\ty"}'); + expect(out.value).toEqual({ a: "line1\r\nline2", b: "x\ty" }); + expect(out.meta?.parseFailure).toBe("control-chars"); + }); + + it("leaves structural whitespace outside strings untouched and does not double-escape", async () => { + const content = '{\n\t"a": "x\ty",\r\n\t"b": "p\tq"\n}'; + const out = await extractContent(content); + expect(out.value).toEqual({ a: "x\ty", b: "p\tq" }); + }); + + it("does not report a repair for valid JSON with structural newlines and tabs", async () => { + const out = await extractContent('{\n\t"a": "x"\n}'); + expect(out.value).toEqual({ a: "x" }); + expect(out.meta).not.toHaveProperty("parseFailure"); + expect(out.meta).not.toHaveProperty("recovered"); + }); + + it("repairs control characters inside JSON that is wrapped in prose", async () => { + const out = await extractContent('Here you go:\n{"a":"p\tq"}\nDone.'); + expect(out.value).toEqual({ a: "p\tq" }); + expect(out.meta).toMatchObject({ recovered: true }); + expect(out.meta?.parseFailure).toBe("prose-around"); + }); + + it("repairs a fenced block with raw TABs in a string", async () => { + const out = await extractContent('```json\n{"a":"p\tq"}\n```'); + expect(out.value).toEqual({ a: "p\tq" }); + expect(out.meta?.parseFailure).toBe("control-chars"); + }); + + it("still fails as not-json when the text is not repairable", async () => { + const out = await extractContent('{"a": p\tq}'); + expect(out.value).toBeNull(); + expect(out.meta?.parseFailure).toBe("not-json"); + }); + + it("a repaired non-object is still non-object, and never leaks content in meta", async () => { + const out = await extractContent('["a\tb"]'); + expect(out.meta?.parseFailure).toBe("non-object"); + expect(JSON.stringify(out.meta)).not.toContain("a\tb"); + }); +}); diff --git a/test/adapters/log/safe-logger.test.ts b/test/adapters/log/safe-logger.test.ts index ddb7cf5..c5a9878 100644 --- a/test/adapters/log/safe-logger.test.ts +++ b/test/adapters/log/safe-logger.test.ts @@ -165,4 +165,67 @@ it("logs the safe parse metadata and drops anything else a model could smuggle i expect(line).not.toContain("IGNORE"); spy.mockRestore(); }); + + it("passes the control-chars parse-failure code through as a fixed value", () => { + const spy = vi.spyOn(console, "log").mockImplementation(() => {}); + createSafeLogger().log({ + event: "hackathon-job", + outcome: "error", + attempts: [ + { + model: "@cf/primary", + parsed: true, + rejectedCount: 0, + rejected: [], + parseFailure: "control-chars", + recovered: true, + }, + ], + }); + const logged = JSON.parse(spy.mock.calls[0]?.[0] as string); + expect(logged.attempts[0]).toMatchObject({ parseFailure: "control-chars", recovered: true }); + spy.mockRestore(); + }); + + it("logs shape: \"invalid\" as a fixed value and drops any other shape value", () => { + const spy = vi.spyOn(console, "log").mockImplementation(() => {}); + const logger = createSafeLogger(); + const base = { model: "@cf/primary", parsed: true, rejectedCount: 0, rejected: [] }; + + logger.log({ + event: "hackathon-job", + outcome: "error", + attempts: [ + { ...base, shape: "invalid" }, + { ...base, shape: "SECRET page text" as unknown as "invalid" }, + ], + }); + + const line = spy.mock.calls[0]?.[0] as string; + const attempts = JSON.parse(line).attempts; + expect(attempts[0]).toEqual({ ...base, shape: "invalid" }); + expect(attempts[1]).toEqual(base); + expect(line).not.toContain("SECRET"); + spy.mockRestore(); + }); + + it("logs a wrong-shape rejection as field name and code only, never the value", () => { + const spy = vi.spyOn(console, "log").mockImplementation(() => {}); + createSafeLogger().log({ + event: "hackathon-job", + outcome: "error", + attempts: [ + { + model: "@cf/primary", + parsed: true, + rejectedCount: 1, + rejected: [{ field: "prizes", reason: "wrong-shape", value: "SECRET value" } as never], + }, + ], + }); + const line = spy.mock.calls[0]?.[0] as string; + expect(JSON.parse(line).attempts[0].rejected).toEqual([{ field: "prizes", reason: "wrong-shape" }]); + expect(line).not.toContain("SECRET"); + spy.mockRestore(); + }); }); diff --git a/test/domain/hackathon/extraction.test.ts b/test/domain/hackathon/extraction.test.ts index 24a1a45..79ea767 100644 --- a/test/domain/hackathon/extraction.test.ts +++ b/test/domain/hackathon/extraction.test.ts @@ -3,6 +3,20 @@ import { validateExtraction } from "../../../src/domain/hackathon/extraction"; const pageText = "Meridian 2026 starts on 2026-03-01. Team size up to 4 people."; +const ALL_NULL = { + name: null, + format: null, + location: null, + teamSize: null, + submissionDeadline: null, + startDate: null, + endDate: null, + resultsDate: null, + prizes: null, + tracks: null, + eligibility: null, +}; + describe("validateExtraction", () => { it("accepts a well-formed response matching the fixed schema (spec llm-extraction: Well-formed response passes validation)", () => { const result = validateExtraction( @@ -72,9 +86,11 @@ describe("validateExtraction", () => { expect(result.ok && result.fields.teamSize).toBeNull(); }); - it("rejects a response missing required shape (spec llm-extraction: Malformed response is rejected)", () => { + it("counts every missing key as a wrong-shape rejection instead of guessing (a bare { name: { value } } is not usable)", () => { const result = validateExtraction({ name: { value: "Meridian" } }, pageText); - expect(result).toEqual({ ok: false, reason: "invalid-shape" }); + expect(result.ok).toBe(true); + expect(result.ok && result.rejectedCount).toBe(11); + expect(result.ok && result.rejections[0]).toEqual({ field: "name", reason: "wrong-shape" }); }); it("rejects unparseable input", () => { @@ -159,64 +175,37 @@ describe("validateExtraction", () => { }); }); - it("rejects a response where teamSize is a string instead of a number (RELI-001/RESI-001)", () => { + it("rejects only teamSize, as wrong-shape, when it is a string instead of a number (RELI-001/RESI-001)", () => { const result = validateExtraction( { - name: null, - format: null, - location: null, + ...ALL_NULL, + name: { value: "Meridian 2026", snippet: "Meridian 2026", confidence: 0.9 }, teamSize: { value: "4", snippet: "Team size up to 4 people", confidence: 0.8 }, - submissionDeadline: null, - startDate: null, - endDate: null, - resultsDate: null, - prizes: null, - tracks: null, - eligibility: null, }, pageText, ); - expect(result).toEqual({ ok: false, reason: "invalid-shape" }); + expect(result.ok).toBe(true); + expect(result.ok && result.rejections).toEqual([{ field: "teamSize", reason: "wrong-shape" }]); + expect(result.ok && result.rejectedCount).toBe(1); + expect(result.ok && result.fields.teamSize).toBeNull(); + expect(result.ok && result.fields.name).not.toBeNull(); }); - it("rejects a response where name is a number instead of a string (RELI-001/RESI-001)", () => { + it("rejects only name, as wrong-shape, when it is a number instead of a string (RELI-001/RESI-001)", () => { const result = validateExtraction( - { - name: { value: 2026, snippet: "Meridian 2026", confidence: 0.9 }, - format: null, - location: null, - teamSize: null, - submissionDeadline: null, - startDate: null, - endDate: null, - resultsDate: null, - prizes: null, - tracks: null, - eligibility: null, - }, + { ...ALL_NULL, name: { value: 2026, snippet: "Meridian 2026", confidence: 0.9 } }, pageText, ); - expect(result).toEqual({ ok: false, reason: "invalid-shape" }); + expect(result.ok && result.rejections).toEqual([{ field: "name", reason: "wrong-shape" }]); + expect(result.ok && result.fields.name).toBeNull(); }); - it("rejects a response where a field's value is undefined (RELI-001/RESI-001)", () => { + it("rejects only the field whose value is undefined (no value key at all) as wrong-shape (RELI-001/RESI-001)", () => { const result = validateExtraction( - { - name: { value: undefined, snippet: "Meridian 2026", confidence: 0.9 }, - format: null, - location: null, - teamSize: null, - submissionDeadline: null, - startDate: null, - endDate: null, - resultsDate: null, - prizes: null, - tracks: null, - eligibility: null, - }, + { ...ALL_NULL, name: { value: undefined, snippet: "Meridian 2026", confidence: 0.9 } }, pageText, ); - expect(result).toEqual({ ok: false, reason: "invalid-shape" }); + expect(result.ok && result.rejections).toEqual([{ field: "name", reason: "wrong-shape" }]); }); it("nulls a field whose snippet is empty (RISK-001: an empty snippet trivially 'matches' any page)", () => { @@ -457,4 +446,128 @@ describe("validateExtraction", () => { expect(result.ok && result.rejections).toEqual([]); }); }); + + // Production evidence: GLM returns a field it cannot find as + // { value: null, snippet: null, confidence: 0 } instead of null, which used + // to fail the WHOLE response as invalid-shape. + describe("per-field shape validation", () => { + it("treats GLM's { value: null, snippet: null, confidence: 0 } as a model-null field, not a rejection", () => { + const result = validateExtraction( + { + ...ALL_NULL, + name: { value: "Meridian 2026", snippet: "Meridian 2026", confidence: 0.9 }, + prizes: { value: null, snippet: null, confidence: 0 }, + tracks: { value: null, snippet: null, confidence: 0 }, + }, + pageText, + ); + expect(result.ok).toBe(true); + expect(result.ok && result.rejectedCount).toBe(0); + expect(result.ok && result.fields.prizes).toBeNull(); + expect(result.ok && result.fields.tracks).toBeNull(); + expect(result.ok && result.fields.name).not.toBeNull(); + }); + + it("treats a null value as null whatever its snippet or confidence are", () => { + const result = validateExtraction( + { + ...ALL_NULL, + prizes: { value: null, snippet: "SOMETHING", confidence: 0.9 }, + tracks: { value: null }, + eligibility: { value: null, snippet: 42, confidence: "high" }, + }, + pageText, + ); + expect(result.ok && result.rejectedCount).toBe(0); + expect(result.ok && result.fields.eligibility).toBeNull(); + }); + + it("rejects an array value alone, as wrong-shape", () => { + const result = validateExtraction( + { + ...ALL_NULL, + prizes: { value: ["a", "b"], snippet: "Meridian 2026", confidence: 0.9 }, + name: { value: "Meridian 2026", snippet: "Meridian 2026", confidence: 0.9 }, + }, + pageText, + ); + expect(result.ok && result.rejections).toEqual([{ field: "prizes", reason: "wrong-shape" }]); + expect(result.ok && result.fields.name).not.toBeNull(); + }); + + it.each([ + ["a non-object field (string)", "Meridian 2026"], + ["a non-object field (array)", []], + ["a numeric field", 5], + ["an object with a non-string snippet", { value: "x", snippet: 3, confidence: 0.5 }], + ["an object with a missing snippet", { value: "x", confidence: 0.5 }], + ["an object with a non-number confidence", { value: "x", snippet: "Meridian 2026", confidence: "0.9" }], + ["an object with a missing confidence", { value: "x", snippet: "Meridian 2026" }], + ])("rejects %s individually as wrong-shape and nulls it", (_n, bad) => { + const result = validateExtraction({ ...ALL_NULL, location: bad }, pageText); + expect(result.ok).toBe(true); + expect(result.ok && result.rejections).toEqual([{ field: "location", reason: "wrong-shape" }]); + expect(result.ok && result.fields.location).toBeNull(); + }); + + it("rejects a non-finite teamSize number as wrong-shape", () => { + const result = validateExtraction( + { ...ALL_NULL, teamSize: { value: NaN, snippet: "Team size up to 4 people", confidence: 1 } }, + pageText, + ); + expect(result.ok && result.rejections).toEqual([{ field: "teamSize", reason: "wrong-shape" }]); + }); + + it("counts a missing key as wrong-shape (a missing key is not the model saying null)", () => { + const { eligibility: _drop, ...withoutEligibility } = ALL_NULL; + const result = validateExtraction(withoutEligibility, pageText); + expect(result.ok && result.rejections).toEqual([{ field: "eligibility", reason: "wrong-shape" }]); + }); + + it.each([ + ["an array", []], + ["a nested array of objects", [{ name: null }]], + ["null", null], + ["a string", "text"], + ["a number", 7], + ])("returns invalid-shape for the whole response only when the top level is %s", (_n, raw) => { + expect(validateExtraction(raw, pageText)).toEqual({ ok: false, reason: "invalid-shape" }); + }); + + it("keeps content checks running on well-shaped fields alongside wrong-shape ones", () => { + const result = validateExtraction( + { + ...ALL_NULL, + name: { value: "Meridian 2026", snippet: "not on the page", confidence: 0.9 }, + teamSize: { value: "4", snippet: "Team size up to 4 people", confidence: 0.8 }, + }, + pageText, + ); + expect(result.ok && result.rejections).toEqual([ + { field: "name", reason: "not-verbatim" }, + { field: "teamSize", reason: "wrong-shape" }, + ]); + }); + + it("counts every wrong-shape field toward rejectedCount so the majority rule can fall back", () => { + const wrong = { value: ["x"], snippet: "Meridian 2026", confidence: 1 }; + const result = validateExtraction( + { + name: wrong, + format: wrong, + location: wrong, + teamSize: wrong, + submissionDeadline: wrong, + startDate: wrong, + endDate: null, + resultsDate: null, + prizes: null, + tracks: null, + eligibility: null, + }, + pageText, + ); + expect(result.ok && result.rejectedCount).toBe(6); // > floor(11 / 2) + }); + }); }); diff --git a/test/domain/usecases/analyze-hackathon.test.ts b/test/domain/usecases/analyze-hackathon.test.ts index 563cd0c..b1f4f34 100644 --- a/test/domain/usecases/analyze-hackathon.test.ts +++ b/test/domain/usecases/analyze-hackathon.test.ts @@ -322,10 +322,57 @@ describe("analyzeHackathon: primary-then-fallback LLM call", () => { rejectedCount: 6, rejected: FIELD_ORDER.slice(0, 6).map((field) => ({ field, reason: "empty-snippet" })), }, - { model: "@cf/fallback", parsed: false, rejectedCount: 0, rejected: [] }, + // Parsing succeeded (a JSON value arrived) but the top level is not an + // object: parsed stays true and shape reports the validation failure. + { model: "@cf/fallback", parsed: true, shape: "invalid", rejectedCount: 0, rejected: [] }, ]); }); + it("parsed means only that JSON parsing succeeded: a parsed non-object reports shape invalid, an unparseable text does not", async () => { + const deps = makeDeps(); + deps.llmExtractor = fakeLlmExtractor([ + { raw: [1, 2], meta: { finishReason: "stop", contentLength: 5, parseFailure: "non-object" } }, + { raw: null, meta: { finishReason: "stop", contentLength: 9, parseFailure: "not-json" } }, + ]); + + const thrown = (await analyzeHackathon(makeInput(), deps).catch( + (e: unknown) => e, + )) as ExtractionFailedError; + + expect(thrown.attempts?.[0]).toMatchObject({ parsed: true, shape: "invalid", parseFailure: "non-object" }); + expect(thrown.attempts?.[1]).toMatchObject({ parsed: false, parseFailure: "not-json" }); + expect(thrown.attempts?.[1]).not.toHaveProperty("shape"); + }); + + it("a majority of wrong-shape fields falls back and is reported as wrong-shape rejections, not as parsed:false", async () => { + const deps = makeDeps(); + const wrongShape: Record = {}; + for (const field of FIELD_ORDER) wrongShape[field] = { value: ["x"], snippet: "Meridian", confidence: 1 }; + deps.llmExtractor = fakeLlmExtractor([{ raw: wrongShape }, { raw: wrongShape }]); + + const thrown = (await analyzeHackathon(makeInput(), deps).catch( + (e: unknown) => e, + )) as ExtractionFailedError; + + expect(thrown.kind).toBe("invalid-output"); + expect(thrown.attempts?.[0]).toMatchObject({ parsed: true, rejectedCount: 11 }); + expect(thrown.attempts?.[0]?.rejected[0]).toEqual({ field: "name", reason: "wrong-shape" }); + expect(thrown.attempts?.[0]).not.toHaveProperty("shape"); + }); + + it("GLM-style null field objects do not fail the response and do not trigger the fallback", async () => { + const deps = makeDeps(); + const raw = validRaw("Meridian") as Record; + raw.prizes = { value: null, snippet: null, confidence: 0 }; + const tracked = fakeLlmExtractor([{ raw }]); + deps.llmExtractor = tracked; + + const result = await analyzeHackathon(makeInput(), deps); + + expect(tracked.calls).toHaveLength(1); + expect(result.fields.prizes).toBeNull(); + }); + it("copies the extractor's safe parse metadata into each attempt's diagnostics", async () => { const deps = makeDeps(); deps.llmExtractor = fakeLlmExtractor([