Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions openspec/changes/hackathon-analysis/apply-progress.md
Original file line number Diff line number Diff line change
Expand Up @@ -1278,3 +1278,10 @@ Checked against the repo's generated types and the official docs (developers.clo
- **Observability (`feat(hackathon-job)`, 55d6828):** `validateExtraction` returns `rejections` (field + reason code); `analyzeHackathon` attaches per-attempt diagnostics to `ExtractionFailedError` (`invalid-output` both attempts, `timeout` primary only); `runHackathonJob` adds them to the existing `hackathon-job` final-failure log as `attempts`. Domain stays free of a logger dependency. Test asserts no snippet, value or URL text reaches the log. Discovered: `createSafeLogger` copies an allowlist and was silently dropping `httpStatus` (added in the earlier fetch follow-up), so it is now allowlisted too, together with `attempts` (rebuilt key by key).
- **Prompt (`feat(hackathon-llm)`, 5b59e8f):** the system instructions ask for a short verbatim quote of at most 160 characters from a single passage and for concise values; the untrusted-page framing is untouched.
- **Docs:** llm-extraction spec ("verbatim modulo whitespace" + two scenarios) and design.md "Extraction Schema and Prompt" updated.

### Smoke-test follow-up (task 11.5): parse diagnostics and tolerant extraction

- **Finding:** the 4th smoke test (`/hackathon .../tokenized-stocks`) failed `llm:invalid-output` with both attempts `parsed:false, rejectedCount:0`: neither model's output parsed. A local REST run with a different page-text reduction parsed fine, so the production cause is unknown; content is deliberately not logged.
- **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.
2 changes: 1 addition & 1 deletion openspec/changes/hackathon-analysis/design.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ These are unchanged:

## Extraction Schema and Prompt

`Field<T> = { 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). 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 `<<<PAGE`/`PAGE>>>` 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<T> = { 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 `<<<PAGE`/`PAGE>>>` 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).

## Migration `0003_hackathon_analysis.sql`

Expand Down
146 changes: 128 additions & 18 deletions src/adapters/llm/workers-ai-extractor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { ConfigError } from "../../config-error";
import { ExtractionFailedError, LlmQuotaExceededError } from "../../domain/errors";
import type { LlmExtractor } from "../../domain/ports";
import {
ExtractionFailedError,
LlmQuotaExceededError,
type LlmParseFailureCode,
} from "../../domain/errors";
import type { LlmExtraction, LlmExtractor, LlmOutputMeta } from "../../domain/ports";
import { buildMessages } from "./prompt";

// Injected `run`: a minimal structural subset of the real Workers AI
Expand Down Expand Up @@ -92,9 +96,11 @@ function isQuotaExhausted(err: unknown): boolean {
return QUOTA_ERROR_PATTERN.test(err.message);
}

// Turns the model's raw output into the `unknown` value that
// hackathon/extraction.ts's validateExtraction is the ONLY place trusted to
// judge (ports.ts "LlmExtractor"). Accepted shapes:
// Turns the model's raw output into an LlmExtraction: `value` is the
// `unknown` that hackathon/extraction.ts's validateExtraction is the ONLY
// place trusted to judge (ports.ts "LlmExtractor"), and `meta` is safe parse
// metadata (finish reason, content length, parse-failure code — never content).
// Accepted shapes:
// - OpenAI-style chat completion `{ choices: [{ message: { content } }] }`
// (GLM-4.7-Flash and Qwen3-30B-A3B return this shape in chat mode;
// `content` is `string | null`). Only `choices[0].message.content` is
Expand All @@ -105,27 +111,122 @@ function isQuotaExhausted(err: unknown): boolean {
// - a bare JSON string, or already-structured `{ response: <object> }`.
//
// Unparseable JSON text is NOT thrown here as an ExtractionFailedError:
// this method returns `null` instead, so validateExtraction's existing
// `value` is `null` instead, so validateExtraction's existing
// "invalid-shape" rejection handles it uniformly with every other
// content-shape problem, and analyzeHackathon's already-implemented
// primary-then-fallback logic (analyze-hackathon.ts's `extractFields`)
// runs the fallback model exactly as it would for any other unusable
// response — this adapter never bypasses that fallback by throwing on a
// content problem. Throwing here is reserved for `run` itself failing
// (network/model/quota/timeout), never for shape or parse problems.
//
// Tolerant extraction: when a direct parse (after fence stripping) fails, the
// 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
// 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*$/;

function parseJsonText(text: string): unknown {
const fenced = CODE_FENCE_PATTERN.exec(text);
// A finish_reason is a short plain token ("stop", "length", "tool_calls").
// Anything else is replaced by a fixed value so no model text is ever echoed.
const FINISH_REASON_PATTERN = /^[A-Za-z_-]{1,20}$/;

function isPlainObject(value: unknown): boolean {
return value !== null && typeof value === "object" && !Array.isArray(value);
}

type BalancedScan =
| { kind: "none" } // no "{" in the text
| { kind: "unterminated" } // a "{" whose matching "}" never arrives
| { kind: "found"; text: string };

// Finds the first "{" and its matching "}", respecting JSON strings and
// backslash escapes so braces inside string values do not count.
function scanBalancedObject(text: string): BalancedScan {
const start = text.indexOf("{");
if (start === -1) return { kind: "none" };
let depth = 0;
let inString = false;
let escaped = false;
for (let i = start; i < text.length; i++) {
const ch = text[i];
if (inString) {
if (escaped) escaped = false;
else if (ch === "\\") escaped = true;
else if (ch === '"') inString = false;
continue;
}
if (ch === '"') inString = true;
else if (ch === "{") depth += 1;
else if (ch === "}") {
depth -= 1;
if (depth === 0) return { kind: "found", text: text.slice(start, i + 1) };
}
}
return { kind: "unterminated" };
}

interface ParsedText {
value: unknown;
parseFailure?: LlmParseFailureCode;
recovered?: boolean;
}

function tryParse(text: string): { ok: true; value: unknown } | { ok: false } {
try {
return JSON.parse(fenced ? (fenced[1] ?? "") : text);
return { ok: true, value: JSON.parse(text) };
} catch {
return null;
return { ok: false };
}
}

function parseJsonText(text: string): ParsedText {
if (text.trim() === "") return { value: null, parseFailure: "no-content" };
const fenced = CODE_FENCE_PATTERN.exec(text);
const body = fenced ? (fenced[1] ?? "") : text;

const direct = tryParse(body);
if (direct.ok) {
return isPlainObject(direct.value)
? { value: direct.value }
: { value: direct.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);
if (inner.ok && isPlainObject(inner.value)) {
return { value: inner.value, parseFailure: "prose-around", recovered: true };
}
}
return { value: null, parseFailure: "not-json" };
}

function parseModelOutput(raw: unknown): unknown {
if (typeof raw === "string") return parseJsonText(raw);
function withMeta(parsed: ParsedText, meta: LlmOutputMeta): LlmExtraction {
return {
value: parsed.value,
meta: {
...meta,
...(parsed.parseFailure !== undefined ? { parseFailure: parsed.parseFailure } : {}),
...(parsed.recovered === true ? { recovered: true } : {}),
},
};
}

function finishReasonOf(first: unknown): string | undefined {
if (first === null || typeof first !== "object") return undefined;
const reason = (first as { finish_reason?: unknown }).finish_reason;
if (typeof reason !== "string") return undefined;
return FINISH_REASON_PATTERN.test(reason) ? reason : "other";
}

function parseModelOutput(raw: unknown): LlmExtraction {
if (typeof raw === "string") {
return withMeta(parseJsonText(raw), { contentLength: raw.length });
}
if (raw !== null && typeof raw === "object" && "choices" in raw) {
const { choices } = raw as { choices: unknown };
const first: unknown = Array.isArray(choices) ? choices[0] : undefined;
Expand All @@ -135,22 +236,31 @@ function parseModelOutput(raw: unknown): unknown {
message !== null && typeof message === "object"
? (message as { content?: unknown }).content
: undefined;
return typeof content === "string" ? parseJsonText(content) : null;
const finishReason = finishReasonOf(first);
const base: LlmOutputMeta = {
...(finishReason !== undefined ? { finishReason } : {}),
contentLength: typeof content === "string" ? content.length : 0,
};
return typeof content === "string"
? withMeta(parseJsonText(content), base)
: withMeta({ value: null, parseFailure: "no-content" }, base);
}
if (raw !== null && typeof raw === "object" && "response" in raw) {
const { response } = raw as { response: unknown };
if (typeof response === "string") return parseJsonText(response);
if (response !== undefined) return response;
return null;
if (typeof response === "string") {
return withMeta(parseJsonText(response), { contentLength: response.length });
}
if (response !== undefined) return { value: response };
return { value: null };
}
return raw;
return { value: raw };
}

export function createWorkersAiExtractor(options: WorkersAiExtractorOptions): LlmExtractor {
const { run } = options;

return {
async extract(pageText: string, modelId: string, signal: AbortSignal): Promise<unknown> {
async extract(pageText: string, modelId: string, signal: AbortSignal): Promise<LlmExtraction> {
if (!MODEL_ID_PATTERN.test(modelId)) {
throw new ConfigError(`invalid Workers AI model id: "${modelId}"`);
}
Expand Down
24 changes: 24 additions & 0 deletions src/adapters/log/safe-logger.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,28 @@
import { LLM_PARSE_FAILURE_CODES } from "../../domain/errors";
import type { LogEvent, Logger } from "../../domain/ports";

const FINISH_REASON_PATTERN = /^[A-Za-z_-]{1,20}$/;

type Attempt = NonNullable<LogEvent["attempts"]>[number];

// Parse metadata is re-validated here (not trusted from the caller): only a
// short plain-token finish reason, a finite number, a fixed parse-failure
// code and a literal true can pass — never free text from a model.
function safeParseMeta(a: Attempt): Partial<Attempt> {
return {
...(typeof a.finishReason === "string"
? { finishReason: FINISH_REASON_PATTERN.test(a.finishReason) ? a.finishReason : "other" }
: {}),
...(typeof a.contentLength === "number" && Number.isFinite(a.contentLength)
? { contentLength: a.contentLength }
: {}),
...(a.parseFailure !== undefined && LLM_PARSE_FAILURE_CODES.includes(a.parseFailure)
? { parseFailure: a.parseFailure }
: {}),
...(a.recovered === true ? { recovered: true } : {}),
};
}

// design.md "Logging": an allowlisted field set only (event, teamId,
// membershipId, field, outcome, errorCode, reason, httpStatus, and the
// name/reason-code-only extraction attempt diagnostics). Update text and values are
Expand Down Expand Up @@ -29,6 +52,7 @@ export function createSafeLogger(): Logger {
parsed: a.parsed,
rejectedCount: a.rejectedCount,
rejected: a.rejected.map((r) => ({ field: r.field, reason: r.reason })),
...safeParseMeta(a),
})),
}
: {}),
Expand Down
22 changes: 22 additions & 0 deletions src/domain/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,34 @@ export type ExtractionFailureKind = "invalid-output" | "model-error" | "timeout"

// One model attempt's outcome, for post-mortem logging only. Carries field
// NAMES and fixed reason codes — never snippet text, values or page content.
// Why a model's text did not parse as a JSON object. Fixed codes only.
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
| "not-json" // no JSON object in the text
| "non-object"; // valid JSON, but not an object (array, number, ...)

export const LLM_PARSE_FAILURE_CODES: readonly LlmParseFailureCode[] = [
"no-content",
"unterminated",
"prose-around",
"not-json",
"non-object",
];

export interface ExtractionAttemptDiagnostics {
model: string;
// false when the response failed schema validation as a whole.
parsed: boolean;
rejectedCount: number;
rejected: FieldRejection[];
// Parse metadata (numbers and fixed codes only — never model content).
finishReason?: string;
contentLength?: number;
parseFailure?: LlmParseFailureCode;
// true when parseFailure was reported but the object was still recovered.
recovered?: boolean;
}

export class ExtractionFailedError extends DomainError {
Expand Down
28 changes: 23 additions & 5 deletions src/domain/ports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type {
import type { RepoFullName } from "./github";
import type { MemberId, MembershipId, TeamId } from "./ids";
import type { Role } from "./entities";
import type { ExtractionAttemptDiagnostics } from "./errors";
import type { ExtractionAttemptDiagnostics, LlmParseFailureCode } from "./errors";

// Every tenant-scoped method takes TeamId as its first parameter. This is a
// deliberate design constraint (see design.md "Tenancy") that makes
Expand Down Expand Up @@ -204,13 +204,31 @@ export interface PageFetcher {
fetch(url: string, signal: AbortSignal): Promise<string>;
}

// Safe metadata about one model response: numbers and fixed codes only,
// never any part of the content. Lets a failed parse be diagnosed without
// logging what the model said.
export interface LlmOutputMeta {
// choices[0].finish_reason, capped; "other" when it is not a plain token.
finishReason?: string;
contentLength?: number;
parseFailure?: LlmParseFailureCode;
// true when parseFailure was reported but the object was still recovered.
recovered?: boolean;
}

export interface LlmExtraction {
value: unknown;
meta?: LlmOutputMeta;
}

// Throws ExtractionFailedError or LlmQuotaExceededError. Otherwise returns
// the model's raw parsed JSON output — `validateExtraction` (the ONLY
// place a raw model response is trusted, hackathon/extraction.ts) decides
// whether it is usable. `signal` carries the per-attempt LLM timeout
// the model's raw parsed JSON output as `value` (null when unparseable) —
// `validateExtraction` (the ONLY place a raw model response is trusted,
// hackathon/extraction.ts) decides whether it is usable — plus optional safe
// parse `meta`. `signal` carries the per-attempt LLM timeout
// (design.md "Time budget": 45 s per LLM attempt).
export interface LlmExtractor {
extract(pageText: string, modelId: string, signal: AbortSignal): Promise<unknown>;
extract(pageText: string, modelId: string, signal: AbortSignal): Promise<LlmExtraction>;
}

export interface HackathonAnalysisRepo {
Expand Down
Loading
Loading