Skip to content

fix(hackathon-extraction): root-cause fix for invalid-output, found with a production-faithful harness - #37

Merged
TOMOKI977 merged 6 commits into
mainfrom
fix/hackathon-extraction-root-cause
Sep 29, 2026
Merged

TOMOKI977 merged 6 commits into
mainfrom
fix/hackathon-extraction-root-cause

Conversation

@TOMOKI977

Copy link
Copy Markdown
Contributor

Summary

This fixes the root cause of the production llm:invalid-output failures for change 3, hackathon-analysis.

PRs #33–#36 were driven by local simulations, which fetched HTML from a developer machine and reduced it with a regex. Those simulations could never reproduce production. This PR is based on a production-faithful harness instead. The harness runs the real src modules on Cloudflare (wrangler dev --remote) with the real AI and Browser bindings and Cloudflare egress.

Root cause (observed on Cloudflare, bnbchain tokenized-stocks)

  1. Cloudflare egress gets a static 403, so production always takes the browser path. The browser's innerText separates table cells with TABs.
  2. Qwen copied those raw TABs into JSON strings, which makes the JSON invalid: Bad control character in string literal.
  3. GLM returns missing fields as { "value": null, "snippet": null, "confidence": 0 } instead of null. validateExtraction was all-or-nothing on shape, so one such field rejected the whole response.
  4. The diagnostics labeled shape failures as parsed: false, which made them look like parse failures.

Changes

  • src/adapters/http/html-to-text.ts, src/adapters/browser/rendered-fetcher.ts: a new normalizePageText turns tabs and control characters into spaces and collapses runs, while keeping paragraph breaks. It covers the static HTML path and the rendered path.
  • src/adapters/llm/workers-ai-extractor.ts: when a parse fails, raw \t, \r and \n inside JSON string literals are escaped by a string-aware scan, and the parse is retried once. The result is reported as parseFailure: "control-chars" with recovered: true.
  • src/domain/hackathon/extraction.ts: validation is now per field.
    • A { value: null, ... } object counts as null, not rejected.
    • A malformed field is rejected alone, with reason wrong-shape.
    • A missing key counts as wrong-shape, so garbage still falls back through the majority rule.
    • invalid-shape is reserved for a non-object top level.
    • The snippet and value checks against hallucination are unchanged.
  • Diagnostics: parsed now means "JSON parsed". Top-level shape failures log shape: "invalid".
  • scripts/hackathon-harness/ and npm run harness: the harness is now a repo tool. It is kept out of the Worker bundle, vitest and the root typecheck. The design doc now requires fetch, LLM and validation changes to pass the harness before merge.
  • Docs: the llm-extraction spec (per-field rejection) and design.md (Worker-egress reality, verification rule).

Harness verification (real Cloudflare, before review)

Page Path GLM Qwen
bnbchain tokenized-stocks (was failing) static 403 → browser ✅ usable, 0 rejected ✅ usable, 1 rejected
bnbchain bnb-ai-hack static 403 → browser ✅ 0 rejected ✅ 2 rejected
ETHGlobal New York 2025 static ✅ 1 rejected ✅ 1 rejected
Devpost listing static ✅ ✅

Review

A bounded review ran at medium risk with the reliability lens on all 20 changed paths. It found no severe findings.

Advisory follow-ups, none of which affected the harness runs:

ID Finding Status
R3-001 An all-null response counts as usable, and the fallback is not tried the spec is silent
R3-003 No test for an escaped quote next to a raw tab test gap
R3-002 The static text/plain path (safe-fetcher.ts) is not normalized out of scope: pre-existing, file not in this diff

Known operational constraint

On the Workers Free plan, the Browser Rendering daily quota is small. bnbchain always needs the browser from Cloudflare egress.

Tests

  • Strict TDD.
  • The full suite passes (803 tests), and the typecheck is clean.

🤖 Generated with Claude Code

@TOMOKI977 TOMOKI977 added the size:exception PR over the 400-line review budget, accepted explicitly label Sep 29, 2026
@TOMOKI977
TOMOKI977 merged commit bf43602 into main Sep 29, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:exception PR over the 400-line review budget, accepted explicitly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant