feat(webllm): rewrite-quality eval harness (#65) by Vaishnavi1709 · Pull Request #149 · offlinecv/OfflineCV

Vaishnavi1709 · 2026-06-23T19:14:29Z

Summary

Phase 3 of the in-browser AI rewrite epic. Adds a deterministic eval harness that scores section-rewrite outputs across (model × prompt variant × fixture) so the default model and prompt are picked from measurement rather than vibes.

Six deterministic rubric criteria — numbers preserved (reuses checkNumbersPreserved from Phase 1), one line per bullet, action-verb lead, length sanity, no-preamble leakage, dedup effectiveness. All computed without a judge model.
Two execution legs:
- Scoring — pure logic + 31 unit tests under src/lib/webllm/eval/*.test.ts; runs in default CI via npm run test.
- Inference — dev-only eval-rewrite.html page driven by npm run eval:rewrite, one model per tab to avoid the WebGPU eviction-then-reload crash path on consumer GPUs.
DRYed verb list — exports ACTION_VERBS from src/lib/score/score.ts so the eval extends the scorer's set instead of duplicating it.
LLM-judge flag-gated and off by default — slot exists (judgeEnabled plumbed through runner + report), implementation deliberately a follow-up.

Findings from the committed reports

All three MODEL_REGISTRY models ran against all four fixtures across three prompt variants. Aggregate best-variant scores:

Model	Best variant	Aggregate
Qwen 2.5 (1.5B)	Baseline = Terse	67% 🏆
Gemma 2 (2B)	Terse	58%
Llama 3.2 (3B)	Baseline	58%

The shipped DEFAULT_MODEL_ID = Qwen2.5-1.5B-Instruct-q4f16_1-MLC is confirmed. Bigger ≠ better here — Llama and Gemma both hallucinate more metrics and mangle number notation.

Follow-up bugs the eval surfaced (filed as separate issues)

These are real but out of scope for this PR:

cleanRewriteLine doesn't strip Llama's "Here are the rewritten bullets:" preamble line.
The noPreambleLeak rubric check has a blind spot when the preamble survives as a bullet (the bullet-stripping step erases it from the raw-text scan).
cleanRewriteLine doesn't strip inline **verb** markdown bold delimiters that Gemma emits on individual tokens (the existing regex only handles whole-bullet wraps).

Architecture notes

Engine-agnostic runner — RewriteFn injection lets tests use stub outputs while the browser entry uses real WebLLM engines.
No production-bundle impact — eval-rewrite.html is not in build.rollupOptions.input; the eval code is only reachable from that dev-only entry. Verified: dist/ after npm run build contains no eval artifacts.
Eval skips analytics — calls engine.chat.completions.create directly instead of rewriteSectionWithLlm so a local benchmark doesn't pollute webllm_section_rewrite_* counters.
Failure handling — a thrown RewriteFn records an error on the cell and scoring continues; the row scores 0 so failures surface in the report instead of silently disappearing.

Test plan

npm run typecheck passes
npm run lint passes
npm run test — 704/704 passing (was 673 on main; +31 new tests under src/lib/webllm/eval/)
npm run build succeeds; dist/ contains no eval code
npm run eval:rewrite opens the dev page, picks a model, runs all 12 cells, downloads JSON + Markdown reports — verified end-to-end against all three registry models on a real WebGPU device
Reports under tests/fixtures/rewrite/reports/ are valid UTF-8 (mojibake in earlier paste-throughs was a clipboard artifact only — file confirms Unicode text, UTF-8 text)

Closes #65.

🤖 Generated with Claude Code

Phase 3 of the in-browser AI rewrite epic. Adds a deterministic eval harness that scores section-rewrite outputs across (model × prompt variant × fixture) so the default model and prompt are picked from measurement, not vibes. Six rubric criteria (numbers preserved / one line per bullet / action verb lead / length sanity / no preamble leak / dedup effectiveness) all computed without a judge model. LLM-judge slot is flag-gated and off by default (slot exists, implementation is a follow-up). Two execution legs: - Scoring leg: pure logic + 31 unit tests under src/lib/webllm/eval/, runs in default CI via npm run test. - Inference leg: dev-only eval-rewrite.html page driven by npm run eval:rewrite, one model per tab to avoid the WebGPU eviction-then-reload crash path on consumer GPUs. Verb list is DRYed: exports ACTION_VERBS from score.ts so the eval extends the scorer set rather than duplicating it. Ships with all three MODEL_REGISTRY models scored under tests/fixtures/rewrite/reports/ (Qwen 67% / Gemma 58% / Llama 58% best-variant) — confirms the current DEFAULT_MODEL_ID = Qwen is the right call. Closes #65. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

+  return `${JSON.stringify(report, null, 2)}\n`;
+}
+
+export function renderMarkdownReport(report: EvalReport): string {


+  const refs = getDomRefs();
+  populateModelPicker(refs);
+
+  refs.runBtn.addEventListener("click", async () => {


s-annam

PR Review: feat(webllm): rewrite-quality eval harness (#149)

Summary

Clean, exceptionally well-documented Phase 3 eval harness. Engine-agnostic runner + deterministic 6-criterion rubric + snapshot-tested report renderer, with the inference leg quarantined to a dev-only HTML entry so zero eval code reaches the production bundle. 31 new tests, all green. Closes #65.

Gate results (on checked-out PR branch)

Gate	Result
`npm run typecheck`	✅ pass
`npm run lint`	✅ pass
`npm run test`	⚠️ 702/704 — 2 failures in `useModelSelection.integration.test.tsx` (`localStorage.clear is not a function`)
CI `verify`	✅ pass (55s)
CI `fallow`	❌ red (report-only, non-blocking)

The 2 local test failures are NOT this PR's defect. They reproduce identically on the #143 base branch this PR is stacked on (the test file comes from #143, untouched here), they're green in CI's verify, and the cause is a local jsdom/node localStorage env quirk. No action for #149.

Highlights

RewriteFn injection seam cleanly splits the Node-testable scoring leg from the WebGPU inference leg — runner.ts never imports @mlc-ai/web-llm.
Sound non-vacuous guards throughout the rubric: empty output fails oneLinePerBullet/actionVerbLead, and zero-bullet output can't trivially "win" dedup (outputBullets.length > 0 && < input.length).
Error rows score 0 and surface in the report rather than silently inflating an aggregate — genuinely defensive.
Fixtures are synthetic, PII-clean (verified per repo policy).
DRY done right: ACTION_VERBS anchored in score.ts, eval extends it.

Key Findings (no blockers)

[Suggestion] verbs.ts:43 — export const ACTION_VERBS is never imported by any other module (only startsWithActionVerb is consumed). Dropping the export keeps it module-internal and clears two fallow alerts at once (#83 never-imported + #85 duplicate-name-across-modules). Matches the team's standing practice of clearing fallow dead-export flags.
[Nit] rubric.ts:125 — replace(b.toLowerCase(), "") strips only the first occurrence of each bullet. Edge-case-only and adjacent to the noPreambleLeak-as-bullet blind spot already filed as a follow-up; fine to leave.
[Nit] report.ts:26 (renderMarkdownReport cognitive complexity 22) and run-eval-browser.ts:199 (CRAP 30) — fallow report-only flags on a snapshot-tested renderer and a dev-only entry. Acceptable as-is.

Verdict

Action: COMMENT — No blocking items. CI verify is green; the local test failures are inherited env noise, not a regression. Two small cleanups (esp. #1, a one-word fix that clears the red fallow check) before merge would be nice but aren't gating.

s-annam · 2026-06-23T19:57:21Z

+  // Lowercased substring match; the phrase list is conservative.
+  let rawMinusBullets = output.raw.toLowerCase();
+  for (const b of outputBullets) {
+    rawMinusBullets = rawMinusBullets.replace(b.toLowerCase(), "");


[Nit]: String.prototype.replace with a string arg strips only the first occurrence — if a bullet's text repeats in raw, later copies survive in rawMinusBullets. Edge-case-only, and adjacent to the noPreambleLeak-as-bullet blind spot you already filed as a follow-up. Fine to leave; flagging for the record.

Only `startsWithActionVerb` is consumed externally; `ACTION_VERBS` is used solely inside verbs.ts. De-exporting clears three fallow alerts at once — the never-imported export (#83), the verbs.ts name collision (#85), and the score.ts dual-export collision (#84) — with no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VoyggnCFiEueVMCKJ34wuF

Node 22+ ships a built-in global `localStorage` that shadows jsdom's `Storage` and lacks a `clear()` method, so the bare `localStorage.clear()` in this test's beforeEach threw on Node 25 (`localStorage.clear is not a function`) while staying green on CI's Node 20. Optional-chain the call to match the store's own defensive `globalThis.localStorage?.` access; the real per-key cleanup is already done by _resetPersistedModelSelectionForTesting. Full suite now 704/704 locally on Node 25. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VoyggnCFiEueVMCKJ34wuF

s-annam

Re-reviewed at 0a5e390. Suggestion fixed (ac25f72, fallow now green), only the rubric.ts nit left non-blocking. CI verify + fallow green, typecheck/lint clean, 704/704 tests, build clean with no eval in dist. LGTM — shipping.

…lind spot (#150, #151, #152) Three small fixes surfaced by the rewrite-quality eval committed in PR #149. #150 — cleanRewriteLine drops chat-opener preambles like "Here are the rewritten bullets:" via a narrow regex (`/^here (?:are|is) (?:the )?(?:rewritten|new|updated)\b/i`). Anchored to start-of-line so a legitimate bullet that mentions "Here" mid-text is unaffected. Caught Llama 3.2 (3B) under the terse and examples-led prompt variants emitting a leading opener that was surviving cleanup and inflating the output bullet count by one. #152 — cleanRewriteLine strips leading `**Verb**` markdown bold when followed by body text (single-word capture by design — multi-word bolds are likely deliberate phrase emphasis). Runs before the whole-line emphasis strip, which only matches when the line both starts and ends with `**`. Caught Gemma 2 (2B) under the terse variant bolding just the leading verb on every bullet. #151 — scoreRubric.noPreambleLeak now also fails when any output bullet itself contains a preamble phrase. Previously the rubric only scanned `raw - bullets`, so a preamble that survived AS a bullet had its text erased from the scan and the criterion falsely reported a pass. With #150 landing in the same PR the upstream cleanup also catches this — but the rubric was measuring the wrong thing regardless, and stays robust to any future preamble shape that slips past cleanRewriteLine. 12 new unit tests across post-process.test.ts and rubric.test.ts pin all three fixes. Full suite: 716 passing (was 704). Closes #150, #151, #152. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…lind spot (#150, #151, #152) (#154) Three small fixes surfaced by the rewrite-quality eval committed in PR #149. #150 — cleanRewriteLine drops chat-opener preambles like "Here are the rewritten bullets:" via a narrow regex (`/^here (?:are|is) (?:the )?(?:rewritten|new|updated)\b/i`). Anchored to start-of-line so a legitimate bullet that mentions "Here" mid-text is unaffected. Caught Llama 3.2 (3B) under the terse and examples-led prompt variants emitting a leading opener that was surviving cleanup and inflating the output bullet count by one. #152 — cleanRewriteLine strips leading `**Verb**` markdown bold when followed by body text (single-word capture by design — multi-word bolds are likely deliberate phrase emphasis). Runs before the whole-line emphasis strip, which only matches when the line both starts and ends with `**`. Caught Gemma 2 (2B) under the terse variant bolding just the leading verb on every bullet. #151 — scoreRubric.noPreambleLeak now also fails when any output bullet itself contains a preamble phrase. Previously the rubric only scanned `raw - bullets`, so a preamble that survived AS a bullet had its text erased from the scan and the criterion falsely reported a pass. With #150 landing in the same PR the upstream cleanup also catches this — but the rubric was measuring the wrong thing regardless, and stays robust to any future preamble shape that slips past cleanRewriteLine. 12 new unit tests across post-process.test.ts and rubric.test.ts pin all three fixes. Full suite: 716 passing (was 704). Closes #150, #151, #152. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Phase 3 in-browser AI rewrite: deterministic eval harness scoring section-rewrite output across (model × prompt × fixture). Six model-free rubric criteria, engine-agnostic runner, dev-only inference entry (zero production-bundle impact), 31 new tests. Confirms DEFAULT_MODEL_ID = Qwen2.5-1.5B. Resolves #65.

…lind spot (#150, #151, #152) (#154) Three small fixes surfaced by the rewrite-quality eval committed in PR #149. #150 — cleanRewriteLine drops chat-opener preambles like "Here are the rewritten bullets:" via a narrow regex (`/^here (?:are|is) (?:the )?(?:rewritten|new|updated)\b/i`). Anchored to start-of-line so a legitimate bullet that mentions "Here" mid-text is unaffected. Caught Llama 3.2 (3B) under the terse and examples-led prompt variants emitting a leading opener that was surviving cleanup and inflating the output bullet count by one. #152 — cleanRewriteLine strips leading `**Verb**` markdown bold when followed by body text (single-word capture by design — multi-word bolds are likely deliberate phrase emphasis). Runs before the whole-line emphasis strip, which only matches when the line both starts and ends with `**`. Caught Gemma 2 (2B) under the terse variant bolding just the leading verb on every bullet. #151 — scoreRubric.noPreambleLeak now also fails when any output bullet itself contains a preamble phrase. Previously the rubric only scanned `raw - bullets`, so a preamble that survived AS a bullet had its text erased from the scan and the criterion falsely reported a pass. With #150 landing in the same PR the upstream cleanup also catches this — but the rubric was measuring the wrong thing regardless, and stays robust to any future preamble shape that slips past cleanRewriteLine. 12 new unit tests across post-process.test.ts and rubric.test.ts pin all three fixes. Full suite: 716 passing (was 704). Closes #150, #151, #152. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Phase 3 in-browser AI rewrite: deterministic eval harness scoring section-rewrite output across (model × prompt × fixture). Six model-free rubric criteria, engine-agnostic runner, dev-only inference entry (zero production-bundle impact), 31 new tests. Confirms DEFAULT_MODEL_ID = Qwen2.5-1.5B. Resolves #65.

…lind spot (#150, #151, #152) (#154) Three small fixes surfaced by the rewrite-quality eval committed in PR #149. #150 — cleanRewriteLine drops chat-opener preambles like "Here are the rewritten bullets:" via a narrow regex (`/^here (?:are|is) (?:the )?(?:rewritten|new|updated)\b/i`). Anchored to start-of-line so a legitimate bullet that mentions "Here" mid-text is unaffected. Caught Llama 3.2 (3B) under the terse and examples-led prompt variants emitting a leading opener that was surviving cleanup and inflating the output bullet count by one. #152 — cleanRewriteLine strips leading `**Verb**` markdown bold when followed by body text (single-word capture by design — multi-word bolds are likely deliberate phrase emphasis). Runs before the whole-line emphasis strip, which only matches when the line both starts and ends with `**`. Caught Gemma 2 (2B) under the terse variant bolding just the leading verb on every bullet. #151 — scoreRubric.noPreambleLeak now also fails when any output bullet itself contains a preamble phrase. Previously the rubric only scanned `raw - bullets`, so a preamble that survived AS a bullet had its text erased from the scan and the criterion falsely reported a pass. With #150 landing in the same PR the upstream cleanup also catches this — but the rubric was measuring the wrong thing regardless, and stays robust to any future preamble shape that slips past cleanRewriteLine. 12 new unit tests across post-process.test.ts and rubric.test.ts pin all three fixes. Full suite: 716 passing (was 704). Closes #150, #151, #152. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-advanced-security Bot found potential problems Jun 23, 2026

View reviewed changes

Vaishnavi1709 requested a review from s-annam June 23, 2026 19:16

s-annam reviewed Jun 23, 2026

View reviewed changes

s-annam and others added 2 commits June 23, 2026 12:59

s-annam approved these changes Jun 23, 2026

View reviewed changes

s-annam merged commit 59bcb31 into main Jun 23, 2026
2 checks passed

s-annam deleted the vk/eval-harness-issue-65 branch June 23, 2026 20:07

Vaishnavi1709 mentioned this pull request Jun 23, 2026

fix(webllm): strip Llama preamble + leading-verb bold, close rubric blind spot (#150, #151, #152) #154

Merged

4 tasks

Navigation Menu

Uh oh!

feat(webllm): rewrite-quality eval harness (#65) - #149

feat(webllm): rewrite-quality eval harness (#65)#149
s-annam merged 3 commits into
mainfrom
vk/eval-harness-issue-65

Vaishnavi1709 commented Jun 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

s-annam left a comment

Uh oh!

Uh oh!

s-annam Jun 23, 2026

Uh oh!

s-annam left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Vaishnavi1709 commented Jun 23, 2026

Summary

Findings from the committed reports

Follow-up bugs the eval surfaced (filed as separate issues)

Architecture notes

Test plan

Uh oh!

Uh oh!

Uh oh!

Uh oh!

s-annam left a comment

Choose a reason for hiding this comment

PR Review: feat(webllm): rewrite-quality eval harness (#149)

Summary

Gate results (on checked-out PR branch)

Highlights

Key Findings (no blockers)

Verdict

Uh oh!

Uh oh!

s-annam Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

s-annam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants