fix(rewrite): reject/revert rewrites that drop or invent numbers (#778)

[s-annam]

Summary

Adds a deterministic reject/revert gate for the résumé rewrite feature: if a
rewrite drops or invents a concrete number relative to the input bullet, the
whole section's rewrite is reverted and the original bullets are kept —
surfaced explicitly in InlineDiff (never a silent no-op).

Scope note vs. the issue as filed: the issue's own "Decision" section specifies
drop-only reverting. Implementation + review surfaced that a drop-only gate can't
reach the issue's ~100% numbersPreservedRate target, because some failure cells
in the 2026-08-07 eval data are pure invention (the model adds a number, drops
none) — e.g. Qwen/baseline inventing 30% on a fixture with zero drops. Widened
the gate to also revert on invention, on explicit approval during this run: an
invented number is arguably worse than a dropped one for a résumé tool, since it
puts a false claim in a document the user hands an employer.

This also required extending the number-extraction logic (tilde-approximations,
hyphenated ranges, thousands-separator formatting, headcount/year/range
classification) — the multiset-vs-set diffing semantics and the drop/invention
asymmetry took three rounds of adversarial review to converge; see below.

Verification

• npm run typecheck / npm run lint clean.
• 582 tests across the affected scope (src/lib/webllm/**, InlineDiff,
  SectionRewrite, ResumeRewriteProposed, ResumeRewrite), 0 failures.
• Whole-stack: 373 files / 6106 tests, typecheck+lint clean.

Known gap, not closed by this PR: the issue's acceptance criteria include a
live npm run eval:rewrite re-run against the shipped model on a real WebGPU
browser, producing a committed report showing numbersPreservedRate reaching
~100%. That requires browser/WebGPU access this implementation run didn't have.
Everything else is unit-proven with fail-before/pass-after evidence; the live
re-run is a follow-up someone with a WebGPU machine needs to do before this can be
considered fully closed against the issue's literal acceptance criteria. The
reports README documents this explicitly and now recommends watching Reverted
rather than Numbers in the interim, since Numbers carries little signal once
reverts count as preserved by construction.

Stack position

Layer 4 of 4 (top). Base: #873. Depends on #873.

Adversarial review

This layer took three rounds — by far the highest-risk part of the batch,
since the number-diffing algorithm is the correctness core of the whole feature.

Round 1 (4 blocking): a residual "flags numbers that are literally present"
class of formatting-sensitivity, most of it — but not all — later found still
open in round 2. Also fixed: numbersPreservedRate counting toward
eval/runner.ts's model-selection composite despite being ~100% by construction;
rewrite-resume.ts telemetry silently changing meaning of existing PostHog
properties on a revert; a blank input bullet suppressing the revert label.

Round 2 (3 blocking, all regressions/gaps from round 1's own fix): the
résumé-level warning banner became unreachable on a revert (stale
!allNumbersPreserved guard, which is true-by-construction post-revert); a
thousands-separator formatting gap ("1,200" vs "1200") in the same bug class
round 1 fixed for ranges; and — the sharpest one — round 1's namespace-merge fix
(which correctly stopped false drop-reverts) opened a false-negative: an invented
headcount could escape detection if the same digit existed untracked elsewhere in
the input (e.g. "phase 5" → "5 engineers" scored clean).

Round 3 (final, all 3 fixed and independently re-verified): the warning-banner
guard now mirrors the sibling SectionRewrite component's condition; thousands
separators are stripped from the comparison key (not the display text); and the
drop/invention asymmetry is now explicit — drops keep the lenient all-atoms check
(a user-typed number re-spelled shouldn't false-positive), while invented
headcounts specifically require the value to have been a claimed atom on the
input side. Independently re-verified: the phase-5→engineers-5 case is caught, the
round-1 12-person regression still passes clean in both directions, and a probe
for the same evasion trick against year claims found the class structurally
immune (any 4-digit 1900–2099 bare integer is always claimed as a year, so the
strict/lenient distinction is a no-op there).

One documented residual, not blocking: the same evasion trick still works
against range and comma-grouped form claims specifically (not headcount or
year) — e.g. "phase 50 … section 100" → "50-100" can still slip through if
both digits happen to exist untracked in the input. Verified this is a deliberate
trade, not an oversight: strict-checking those two claim kinds would re-break the
legitimate re-spellings round 1 and round 2 exist to protect ("50 to 100" →
"50-100", "1200" → "1,200"), both of which are unclaimed on the input side by
construction. The module docblock's residual-cost note names the false-positive
half of this trade but not this false-negative half — worth a follow-up doc-only
patch, or two it("accepts …, deliberately") pins so a future strict-ening
attempt trips a red test instead of silently re-breaking Finding 2/round-1.

Final round: clean: true, 373 files / 6106 tests, typecheck + lint clean.

Closes #778

[cloudflare-workers-and-pages]

Deploying offlinecv with    Cloudflare Pages

Latest commit:	bfc8cf7
Status:	✅  Deploy successful!
Preview URL:	https://e97a281c.offlinecv.pages.dev
Branch Preview URL:	https://gh-batch-04-number-preservat.offlinecv.pages.dev

View logs

[Samhit21]

PR Review: fix(rewrite): reject/revert rewrites that drop or invent numbers

Verdict first

Not approving yet — one Blocking finding. The guardrail reverts a whole section on the single most common class of résumé rewrite there is: strengthening a weak verb in front of a number. Everything else I checked held up, and the design write-up is genuinely excellent, so this is one classification rule rather than an architectural problem.

The finding

PEOPLE_VERB_PREFIX.test(before) || PEOPLE_NOUN_FOLLOW.test(after) (preserve-numbers.ts:282) classifies a bare integer as a headcount from the verb ALONE, with no requirement that what follows denote people. So Managed 5 projects reads 5 as a headcount. Rule 3 then demands the input have claimed num:5, and if the input said Owned 5 projects — where Owned is not in the verb list — the input's 5 was unclaimed. Invention. Whole section reverted.

Run against the module on this branch:

REVERT  ["Owned 5 projects."]      -> ["Managed 5 projects."]      added=["5"]
REVERT  ["Built 8 features."]      -> ["Led 8 features."]          added=["8"]
REVERT  ["Delivered 4 programs."]  -> ["Ran 4 programs."]          added=["4"]
REVERT  ["Shipped 6 releases."]    -> ["Directed 6 releases."]     added=["6"]

The verb list is precisely the set a rewrite prompt pushes models toward (led, managed, ran, directed, headed), and "N projects / features / programs / releases / initiatives" is everywhere in résumés. The user loses the rewrite and is told "kept your original — the rewrite would have added 5", which is false: the rewrite added nothing, it changed a verb.

I isolated the cause rather than guessing — same probe, controls all correct:

ok      already a people verb on both sides:  ["Managed 5 projects."] -> ["Led 5 projects."]
ok      reverse direction (verb removed):     ["Managed 5 projects."] -> ["Owned 5 projects."]
ok      people noun on both sides:            ["Led 5 engineers."]    -> ["Managed 5 engineers."]
ok      legitimate merge / comma re-spelling / 12-person <-> 12 people
REVERT  genuine invented headcount:           ["phase 5 …"]           -> ["5 engineers …"]
REVERT  genuine drop:                         ["$4.2M"]               -> (removed)

So it fires only when the rewrite introduces a management verb the input lacked — which is the feature working as intended.

Suggested shape

The verb-only path is needed — Managed 8 across the data platform (your own test) is a headcount with the noun elided. It just shouldn't win when a non-people noun is sitting right there. Let the noun decide when there is one:

// A people noun after the digit is decisive on its own.
if (PEOPLE_NOUN_FOLLOW.test(after)) return "headcount";
// The verb alone qualifies only when NO noun follows — i.e. the digit runs
// into a preposition, punctuation, or the end of the bullet. "Managed 8
// across …" is a headcount; "Managed 5 projects" is a project count.
if (PEOPLE_VERB_PREFIX.test(before) && FUNCTION_WORD_FOLLOWS.test(after)) {
  return "headcount";
}

with FUNCTION_WORD_FOLLOWS matching end-of-string, punctuation, or a small closed list (across in on at for to from over within during and with through by per). I checked that against the existing suite by hand: Led 5 engineers… passes on the noun rule, Managed 8 across… and team of 5 across… pass on the function-word rule, Owned 8 reports… passes on the noun rule, 1 of 5 candidates and out of 10 stay unclaimed — and Managed 5 projects stops claiming.

A pin in both directions would be worth having: Owned N projects → Managed N projects scores clean, and phase 5 → 5 engineers still reverts.

What I checked that is fine

• Set-vs-multiset, comma keys, range endpoints, 12-person ↔ 12 people, sign capture — all behave as documented; the three rounds of adversarial review clearly did their job on these.
• ATOM.lastIndex reset per bullet — correct, no cross-bullet state leak.
• bareIntegerClaim's match.index == digit index assumption — holds, since every prefix decoration implies isDecorated and short-circuits earlier.
• Year-before-range ordering — 2019-2021 scores as two years, as documented.

On the disclosed residual

The body's documented false-negative (range / comma-form claims launderable when both digits exist untracked) is a defensible trade and I am not counting it against this. Worth noting it generalises slightly further than written: the strict headcount lookup consults inputClaimedKeys across all claim kinds, so an input range endpoint 5 also launders an output headcount 5. Same class, same trade — just a wider statement than the docblock makes.

Verdict

Action: COMMENT — happy to re-review and approve once the headcount classification stops firing on non-people nouns.

Reviewed by: Claude Opus 5 (1M context)

[s-annam]

Note: GitHub blocks REQUEST_CHANGES/APPROVE from the PR author's own account (s-annam opened this PR and is also the authenticated reviewer here), so this posts as COMMENT. Treat the verdict below as REQUEST_CHANGES — do not merge until Blocking #1 and #2 are addressed.

REQUEST_CHANGES — 2 Blocking, 3 Secondary, 1 Nit. Rule applied: ≥1 Blocking finding → REQUEST_CHANGES (per pr-review skill).

Blocking

1. The num: bare-integer namespace collapses ALL untracked digits of the same value into one key, which silently defeats the #778 reject gate on ordinary résumé content. See inline comment on preserve-numbers.ts:413. Verified by direct execution against the real module:

input:  ["Managed a team of 12 engineers.", "Completed certification module 12 of the curriculum."]
output: ["Led the engineering department.", "Completed certification module 12 of the curriculum."]
checkNumbersPreserved(input, output) → { ok: true, dropped: [], added: [] }

The headcount "12 engineers" is genuinely dropped, but the drop-side check (outputKeys.has(atom.key), preserve-numbers.ts:413) tests presence against every output atom's key regardless of claim — so the unrelated, unclaimed "module 12" atom masks it. This isn't the residual the module docblock or the PR's round-3 adversarial review calls out: that analysis (and the "immune" claim for headcount/year) was specifically about the invention direction (rule 3's strict headcount check). This bug is in the drop direction (rule 1), which stays uniformly lenient for every claim kind. Reproduces the same way for a dropped year:

input:  ["Founded the company in 1900.", "Worked in office suite 1900."]
output: ["Founded the company.", "Worked in office suite 1900."]
→ { ok: true, dropped: [], added: [] }

This defeats the entire point of #778 — a rewrite can drop a real headcount or year and ship with no revert and no warning, whenever the same digits appear anywhere else in the bullets (a genuinely common résumé pattern: page/module/exhibit numbers, dates, other headcounts).

2. numbersPreservedRate reaching ~100% on a committed eval report (issue #778 ACs 5 & 6) is not implemented, and the PR says Closes #778. Both the PR description ("Known gap, not closed by this PR... this can be considered fully closed against the issue's literal acceptance criteria" once someone re-runs it) and tests/fixtures/rewrite/reports/README.md ("No post-#778 run is committed yet.") say this outright. Per the review's own rule: an unimplemented AC on a PR that says Closes #N is Blocking — the merge closes the issue, and the gap ships as "done." The disclosure is honest and the reasoning for deferring is reasonable, but the fix is mechanical: drop Closes #778 to Refs #778 (or hold the merge until the WebGPU re-run lands), not carry the gap through a closing keyword.

Secondary

3. The whole-résumé warning banner conflates a genuinely-reverted section with an independently-emptied one. ResumeRewriteProposed.tsx:216's anyReverted and aggregateDrift's concatenated token list are both computed across every section uniformly. applyNumberPreservation deliberately does not revert an empty generation (post-process.ts, "An empty rewrite is deliberately NOT reverted"), so if section A is reverted (drops $4.2M, kept original) and section B's model call comes back empty (drops 40%, not reverted, content now blank), the banner still renders "Kept your original — the rewrite removed $4.2M and 40%, so I didn't apply it" — false for section B, whose content isn't preserved at all, it's gone. Needs the banner (or aggregateDrift) to track reverted-vs-emptied per section rather than one boolean + one merged list.

4. The revert/drift tone-and-badge decision is implemented three separate times with three different expressions — resultTone() in SectionRewrite.tsx:607, the inline result.allNumbersPreserved && !anyReverted in ResumeRewriteProposed.tsx:220, and the nested ternary in ResumeRewrite.tsx's CompletedList (:431). A future change to the revert/drift rule (e.g. a third state) only lands where the author remembers to update it. Worth extracting to one shared predicate (describeNumberDrift's neighbor already does this for the copy string — the tone/badge logic should follow the same pattern).

5. SectionRewrite.tsx is explicitly named in this repo's root CLAUDE.md as "known debt — do not imitate" at 607 LOC, with the instruction "If you are editing one, prefer extracting your change into a new sibling over growing the file further." This diff adds resultTone, the reverted-copy branch of NumberPreservationWarning, and related logic directly in the file, growing it to 674 LOC (verified: git show origin/main:.../SectionRewrite.tsx | wc -l = 607, current = 674) with no extraction.

6. SectionRewrite.tsx's top-of-file docblock is now stale. Unchanged by this diff, it still reads: "Number-preservation guardrail runs deterministically on every output. When a numeric token is dropped or invented, an inline warning names the specific token (non-blocking) so the user can still accept the rewrite knowingly." That described the pre-#778 behavior; the gate is now a hard block that reverts the rewrite outright — the user never sees a rewrite with a bad number to "accept knowingly." No + line to anchor this to (the docblock predates this diff), flagging in the body per the review's own scoping rule.

Nits

7. (Pre-existing, not introduced by this PR) ResumeRewrite.tsx:439 — a section whose model call returns empty (not reverted, per applyNumberPreservation's deliberate non-revert-on-empty rule) is badged "metric drift" / "A metric was altered or removed" — the same generic label used for a partial drift. Since this PR ships a sharper "kept original" vs "metric drift" distinction for the reverted case, a user who's learned that distinction may reasonably misread "metric drift" on an emptied section as "some numbers changed" rather than "this section's content is gone." This label existed before #778 (verified against origin/main) and isn't worsened here — flagging for a possible follow-up, not blocking this PR.

AC checklist (issue #778)

• Number-extraction extended for tilde approximations, hyphenated ranges (+ multipliers/at-least, found during the required audit) — well tested.
• cleanRewriteLine/applyNumberPreservation rejects a rewrite that drops a number, keeps the original — but see Blocking #1, the detection it's built on has a real gap.
• InlineDiff surfaces the reject/revert explicitly (noChangeLabel, NumberPreservationWarning) — not a silent no-op.
• Reverted rewrites count toward numbersPreservedRate (true by construction) — companion revertedRate diagnostic added and excluded from the composite score, well tested.
• numbersPreservedRate reaching ~100% on shipped model/variant, measured by a committed eval report — not done, see Blocking #2.
• Reject+surface fix measured by a committed eval report — not done; the "why not prompt-tune first" note IS present and good.
• Reports README's Numbers note updated — thorough, explains the post-#778 reading, the Reverted column, and states the gap plainly.

Gates run

• check:fixtures / PII (3a): N/A — no PDF/image fixtures touched, only markdown.
• Design-system + reuse (3b): clean — no raw <button>/dialog/input in feature code, no new files under src/components/.
• Style tokens (3c): clean — the only regex hits were #778 issue references matching the hex-color pattern, not real hex/palette/dark: usage.
• fallow audit (3d): 3 pre-existing/grown complexity findings (useSectionRewrite 18 cyclomatic/299 lines, onApply 37.1 CRAP, classifyAtom 37.1 CRAP) — report-only per this repo's verify, not gating; consistent with SectionRewrite.tsx's documented debt status (see Secondary #5).
• Command-level bugs (3e): N/A — no skill/script files touched.
• Description accuracy (3f): the PR body is unusually candid about its own gap (Blocking #2) and documents a real, separate residual (range/form invention-evasion) it found during review — good faith, but the Closes keyword still needs to match what's actually done.

Verification run this pass

• npx vitest run on all 11 touched test files: 260/260 passing.
• npm run typecheck: clean.
• npm run lint: clean.
• npx fallow audit --base origin/main: 3 complexity findings (see above), report-only.

Head SHA reviewed: 1d70b89d541aa2fcff5c811192412d2798c49834.

---

Reviewed by: Claude Sonnet 5 (high)

[s-annam]

Blocking. This drop check (outputKeys.has(atom.key)) tests presence against every output atom, claimed or not, so an unrelated unclaimed atom of the same digits masks a genuine drop. Verified by direct execution:

checkNumbersPreserved(
  ["Managed a team of 12 engineers.", "Completed certification module 12 of the curriculum."],
  ["Led the engineering department.", "Completed certification module 12 of the curriculum."]
) // -> { ok: true, dropped: [], added: [] }

The "12 engineers" headcount is genuinely dropped but the unrelated "module 12" satisfies presence via the shared num:12 key. Reproduces the same way for a dropped year ("in 1900" masked by an unrelated "suite 1900"). This is a different bug than the PR's documented round-3 residual (which is about the invention direction and specifically claims headcount/year are immune there) — this is the drop direction, which has no strict check for any claim kind. Silently defeats the #778 reject gate on ordinary content (page numbers, dates, other headcounts sharing digits with a genuinely-dropped one).

[s-annam]

Partially fixed in bfc8cf7 — leaving this open for the year half.

Headcount: fixed. The drop check is now count-aware for headcount claims — presence requires either a claimed output atom of the same key, or a NEW unclaimed occurrence that wasn't already sitting in the input (so Managed 5 engineers -> Completed phase 5 still reads as a legitimate reword, pinned since #778, while your exact repro — a dropped 12 engineers masked by an unchanged, unrelated module 12 — is now caught). Added regression tests for both.

Year: NOT fixed, and I don't think it can be with this same mechanism. Every 4-digit number in 1900-2099 auto-classifies as a year regardless of context (bareIntegerClaim, no verb/noun gate at all unlike headcount) — so suite 1900 is itself always "claimed," and there's no unclaimed state for the count-aware guard to compare against. Catching this needs a context gate on year classification itself (temporal cues like "in/since/during/by"), which is a real, separate change I don't want to bundle into this fix. Added a test pinning the current (still-masked) behavior with that explanation, and I'd like to leave this thread open rather than resolve it out from under a known gap — happy to file a follow-up issue for the year case if you'd rather track it that way.

[s-annam]

Filed #876 to track the year-masking gap described above. Leaving this thread open against that issue.

[s-annam]

Nit, pre-existing (not introduced by this diff). When a section's generation comes back completely empty, reverted is false (by applyNumberPreservation's deliberate empty-generation exemption) but numbersPreserved is false, so this branch badges it "metric drift" / "A metric was altered or removed" — same generic label as a partial drift, even though the whole section's content is gone. This exact mislabeling existed before #778 too (verified against origin/main); now that this PR ships a sharper "kept original" vs "metric drift" distinction, a user who's learned it may misread this case. Not blocking, worth a follow-up.

[s-annam]

Deferred — leaving this open. This is the pre-existing mislabel (confirmed present on origin/main before #778 too), not something this PR's diff touches, and the fix belongs to the badge logic that was just consolidated into numberDriftStatus in NumberPreservationWarning.tsx. Adding a fourth status (e.g. "emptied", distinct from "drift") is a real, separate change with its own copy decisions across three call sites, and I don't want to bundle it into this fix under time pressure. Happy to file a follow-up issue for it if that's useful to track.

[s-annam]

Filed #877 to track this pre-existing mislabel. Leaving this thread open against that issue.

[Samhit21]

PR Review (round 2): fix(rewrite): reject/revert rewrites that drop or invent numbers

Verdict

APPROVE. My Blocking finding is fixed, and so is the drop-side masking bug you found independently — which was the sharper of the two, and one I missed: I probed the invention side hard and never thought to ask the same question of the drop side.

Verified by execution, not reading

All 14 core behaviours correct on bfc8cf7:

ok      Owned 5 projects        -> Managed 5 projects        (was a false revert)
ok      Built 8 features        -> Led 8 features            (was a false revert)
ok      Delivered 4 programs    -> Ran 4 programs            (was a false revert)
REVERT  team of 12 engineers + module 12  -> department + module 12   dropped=[12]   (your masking bug)
ok      Managed 5 engineers     -> Completed phase 5         (reword away from headcount, allowed)
REVERT  phase 5                 -> 5 engineers               added=[5]     (invention still caught)
ok      Managed 8 across …      -> Owned 8 reports across …  (verb-only headcount preserved)
REVERT  Managed 8 across …      -> Managed the data platform dropped=[8]
REVERT  Led 5 engineers …       -> Led engineers …           dropped=[5]
ok      merge dedup · comma respelling · 12-person ↔ 12 people · range respelling
REVERT  $4.2M                   -> (removed)                 dropped=[$4.2M]

Plus 591 tests / 33 files green across src/lib/webllm/**, SectionRewrite, ResumeRewrite, ResumeRewriteProposed, InlineDiff; typecheck clean; eslint clean on the four changed source files.

The "NEW unclaimed occurrence" qualifier on the drop side is the right shape and I checked the two directions that make it work: module 12 surviving unchanged no longer masks a dropped 12 engineers (counts equal, so not present), while Managed 5 engineers → Completed phase 5 still passes (0 → 1 is a genuinely new occurrence). That distinction is doing real work, not decoration.

One gap my own suggestion opened — follow-up, not blocking

Tightening the verb path to require FUNCTION_WORD_FOLLOWS means an adjective between the digit and the people noun now defeats both paths, because PEOPLE_NOUN_FOLLOW requires the noun to be the FIRST token:

ok  ["Managed 12 direct reports."]  -> ["Managed the team."]   // 12 silently lost
ok  ["Led 3 senior engineers."]     -> ["Led the group."]       // 3 silently lost

Both were caught before this round (the bare verb claimed them) and are missed now. N direct reports is close to the most common headcount phrasing on a résumé after N engineers, so this is worth closing.

I am not treating it as blocking, and the reason is the asymmetry the module docblock already argues: this fails SAFE. A missed drop lets the rewrite through with a number lost — the status quo before this PR — whereas the bug it replaced destroyed a good rewrite the user had already earned. Trading a frequent false revert for a narrower false negative is the right direction, and the gate is still a large net improvement over no gate. But it should not sit unclosed.

The shape is a bounded adjective run before the noun — something like (?:[a-z]+(?:-[a-z]+)?\s+){0,2} ahead of the people-noun alternation. Worth excluding function words from that run so 12 of the engineers doesn't start claiming, given the existing 1 of 5 candidates / out of 10 pins.

On the rest of the round

NumberPreservationWarning.tsx as a shared component is a better answer than the shared predicate I would have suggested — it collapses the three drifting tone expressions AND the per-section reverted-vs-emptied conflation into one place. SectionRewrite.tsx is 611 lines, which is a wash against the 607 flagged in CLAUDE.md; the extraction moved logic out and wiring in. Not a blocker, but the file is still on the known-debt list.

Still open from the body

The live npm run eval:rewrite re-run against a real WebGPU browser remains the one acceptance criterion this PR cannot close, and the reports README documents it. Worth keeping visible — with reverts counting as preserved by construction, numbersPreservedRate carries little signal until that run happens, which is exactly what the README now says.

Reviewed by: Claude Opus 5 (1M context)