Skip to content

fix(preserve-numbers): give year classification a context gate (#876) - #919

Open
shubhransh-gupta wants to merge 1 commit into
offlinecv:mainfrom
shubhransh-gupta:sg/preserve-numbers-year-context-gate
Open

fix(preserve-numbers): give year classification a context gate (#876)#919
shubhransh-gupta wants to merge 1 commit into
offlinecv:mainfrom
shubhransh-gupta:sg/preserve-numbers-year-context-gate

Conversation

@shubhransh-gupta

@shubhransh-gupta shubhransh-gupta commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Resolves #876.

Gives year classification in bareIntegerClaim a surrounding context gate (isYearContext), preventing unrelated 4-digit numbers (e.g. "suite 1900", "module 2048", "2000 units") from unconditionally claiming as years.

Context Gate & Architectural Trade-off

  • Drop Side (Rule 1): isYearContext gates whether a 4-digit number is claimed as a year. A genuinely dropped year ("in 1900") is tracked as claim = "year", preventing an unrelated surviving digit ("suite 1900") from masking the drop.
  • Invention Side (Rule 3): year claims remain lenient on additions (inputKeys.has(atom.key)), allowing legitimate temporal rewording (e.g. "2019 Excellence Award""Award in 2019") without false-positive reverts.
  • Residual / Accepted Trade-off: Generic determiners (the|a|an|our|this) and prepositions (of|at|for) are excluded from claiming bare 4-digit numbers as years without dedicated temporal phrasing. This avoids false reverts on plain-English merges of 4-digit quantities ("Delivered the 2000 units.", "Reduced by 2000 hours."), while specific phrase cues (recipient of the..., winner, ..., at [Company/Conference]..., [Action verb] the [Noun] [Year]) defend genuine attributive and credential year forms.

Changes

  1. src/lib/webllm/preserve-numbers.ts:

    • Added YEAR_CONTEXT_WINDOW = 32.
    • Hoisted MONTH_NAME and LEADING_DATE_ANCHOR_SEPARATOR module-level constants.
    • Added YEAR_PREFIX_CUE: matches temporal prepositions (in, since, during, until, through, between, before, after, around, circa, c\.), phrases (as of, class of, cohort of, batch of), verbs and multi-token action phrases (awarded, certified, earned, completed, launched, published, promoted, shipped, graduated, founded, established, joined), award titles (employee of the year), credentials (certified architect), conference/company names after at, month names, seasons, quarters, companion range years ((?:19\d\d|20\d\d)\s*(?:${RANGE_DASH.source}|/|to|and)), and date slashes.
    • Added YEAR_FOLLOW_CUE: matches temporal qualifiers (onwards, present, current, now, ongoing), ranges (to \d{4}, until \d{4}, through \d{4}, and \d{4}), months, and RANGE_DASH / / followed by companion years or ongoing markers.
    • Added isYearContext: checks prefix cues, follow cues, and leading bullet date anchors (requiring an anchor separator :, ., ), or RANGE_DASH).
    • Updated docblock Rules 1 & 3 to accurately describe the drop-side strict parity and add-side leniency for year.
  2. src/lib/webllm/preserve-numbers.test.ts:

    • Added drop-side tests covering common resume year forms (degrees, parenthesized conference years, certifications, awards, multi-token companies).
    • Added merge tests verifying that plain-English merges of 4-digit quantities do not trigger false year drops.
    • Added tests verifying uniform treatment of range endpoints across the 1900-2099 boundary.
    • Added tests for all 6 Unicode dash characters across date ranges, month abbreviations with dots, and date slashes.

Verification

  • npm run verify passed cleanly: 383 test files passed, 6,451 / 6,451 tests passing.
  • Single clean squashed commit on branch.

@s-annam s-annam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diagnosis is right, the mechanism is the one #876 asked for (isYearContext mirroring the headcount verb/noun gate), and the docblock rewrite carries the reasoning honestly. The issue's repro is genuinely fixed.

What blocks it is cue precision, not cue design. #876 closed with: "This needs its own false-positive audit — a badly-scoped gate would create the missed-legitimate-year-reword failure mode that docblock rule 1 explicitly warns about." I ran that audit, and it comes back negative in both directions: the gate invents claims on non-years, and it stops defending the most common real year forms — including strings already sitting in this repo's fixtures.

Method. I imported main's checkNumbersPreserved alongside this branch's and diffed their verdicts on identical inputs. Everything below is a verified behaviour change introduced by this PR, reproduced twice (once by me, once by an independent /code-review pass). Nothing here is a hypothesis.

Why these are Blocking. applyNumberPreservation (post-process.ts:283) is a hard block since #778: either a false dropped or a false added discards the whole section's rewrite and shows a revert notice. That is the cost the module docblock names — "A false positive here therefore costs a whole section's rewrite." The other direction is worse in kind: an untracked year means the guardrail silently lets the model erase a fact, which is its core job.

CI is green for all of this, because none of the six added tests probes either direction.

Blocking

1. Non-temporal cues invent year claims, discarding good rewrites

YEAR_PREFIX_CUE includes bare and, to, by, from, between, before, after, and the transitive achievement verbs (completed, started, earned, published, awarded). None imply a year; all routinely precede a plain count. Because rule 3 requires an output-side year claim to be matched by an input-side one, any reword that moves a 4-digit quantity next to one of these reads as an invented year:

input rewritten to main this PR
Delivered 2000 units to the plant. Completed 2000 units for the plant. ok added: ["2000"]
Handled 2000 escalations across 3 products. Handled 3 products and 2000 escalations. ok added: ["2000"]
Serving 2000 active users. Grew the base to 2000 active users. ok added: ["2000"]
Fixed 1980 defects. Completed 1980 defect fixes. ok added: ["1980"]
Processed 2050 claims. Processed claims (2050). ok added: ["2050"]

The last row is a separate mechanism: the [(\[‘'"~]alternative claims a year for **any** parenthesised 4-digit number, so(2050)claims exactly as(2020)` does. Those two are not separable by context, so the parens rule buys a test case at the price of every parenthesised quantity.

Note this shape is structurally identical to your own passing test "catches an invented year claim from an unrelated pre-existing digit" (suite 1900founded in 1900): one unclaimed 4-digit becoming one claimed 4-digit on the same key. Only cue precision separates the one true positive from the five false ones.

2. The uncued year — broken in both directions

A real year with no lexical cue — the attributive form, which is arguably the dominant year idiom in résumé prose — is now unclaimed. That costs coverage on the drop side and manufactures inventions on the reword side.

Drops the guardrail no longer catches (main caught every one):

"Recipient of the 2019 Excellence Award." -> "Recipient of the Excellence Award."   main: ["2019"]  now: []
"The 2019 migration reduced latency."     -> "The migration reduced latency."       main: ["2019"]  now: []
"Led the 2021 platform rewrite."          -> "Led the platform rewrite."            main: ["2021"]  now: []
"Winner, 2020 Innovation Award."          -> "Innovation Award winner."             main: ["2020"]  now: []
"Presented at KubeCon 2022."              -> "Presented at KubeCon."                main: ["2022"]  now: []

And the mirror — a faithful reword reported as invention:

"Recipient of the 2019 Excellence Award." -> "Won the Excellence Award in 2019."
    main: ok:true          this PR: added: ["2019"]

Nothing was invented; the model moved the year into temporal phrasing, which is a rewrite the prompt actively encourages. Worth noting a second consumer: eval/rubric.ts re-derives numbersPreservedRate from this function, so an untracked year counts as preserved — the coverage loss is invisible in the metric that would otherwise surface it.

3. YEAR_FOLLOW_CUE's dash class ≠ RANGE_DASH, so year tracking depends on the PDF's font

The new class is [-–—/] — ASCII hyphen, U+2013, U+2014. RANGE_DASH (line 165) deliberately covers U+2010–U+2015 and U+2212, and its docblock says exactly why: "A PDF extractor emits any of these where the author typed one dash, so recognising only - would make range detection depend on the font." The new class reintroduces the problem it was written to prevent. On a spaced range, Acme Corp 2019 <dash> 2021 senior engineer.Acme Corp senior engineer.:

dash main this PR
- U+002D dropped: ["2019","2021"] dropped: ["2019"]
U+2013 dropped: ["2019","2021"] dropped: ["2019"]
U+2012 figure dash dropped: ["2019","2021"] ok: true
U+2011 non-breaking hyphen dropped: ["2019","2021"] ok: true
U+2212 minus dropped: ["2019","2021"] ok: true

Two failures stacked. For three of the six dashes an entire employment date range can be deleted and scored clean. For the other two the range is asymmetric — the left endpoint claims via the follow cue, the right endpoint matches neither cue (isRangeEndpoint needs a tight digit-dash-digit), so half the range is defended. Reusing RANGE_DASH.source fixes the font dependence; the asymmetry needs a prefix-side counterpart to the follow cue.

Corpus check. Sweeping every string in tests/fixtures/rewrite/*.json and tests/fixtures/pdfs/*/*.truth.json (325 strings, 24 containing a bare 1900–2099 integer): 25 year occurrences stay tracked, 10 become untracked2020 – 2022, Sep. 2025 – Apr. 2026, 01/2019 - 02/2022, a 2024 product launch. Two more lexicon mechanisms show up there: the …)\s*$ anchor cannot cross an abbreviation period, so Sep 2025 matches but Sep. 2025 does not; and / is in the follow class but not the prefix class, so a numeric date's year has no preceding cue.

On the fix direction

The two blocking directions pull against each other, so it is worth choosing deliberately rather than patching the lexicon twice.

I prototyped the obvious move — mirror FUNCTION_WORD_FOLLOWS and let a prefix cue claim only when the digit does not run into a noun. Across 15 phrases (9 genuine years, 6 false positives) it removed all 6 and kept all 9. But it makes Blocking 2 strictly worse: "digit immediately followed by a noun" is the attributive-year shape.

The cleaner option, I think, is to split the two sides rather than tune one lexicon to serve both: keep year lenient on the invention side (rule 3, line 591) and apply the context gate only to the drop side. #876's entire repro is drop-side, so the gate still does the job it was filed for; and every false invention above — all of Blocking 1, and the reword half of Blocking 2 — disappears, because a year the output claims would once again only need the value present in the input. The cost is that suite 1900founded in 1900 stops being caught, which is behaviour #876 never asked for and which the added test introduced on its own initiative. Blocking 3 and the drop-side half of Blocking 2 are then plain lexicon work.

That is a suggestion, not a requirement — you own the call, and if you keep the symmetric gate, the lexicon needs to handle the attributive year in both directions.

Secondary

4. The leading-anchor rule fires on any bullet-initial 4-digit number

leadingText.length === 0 && /^[\s:–—.)/]/.test(after || " ") — the bare \s in that class means every bullet-initial in-range number followed by a space is a year. So 2000 users onboarded in Q1 claims while Onboarded 2000 users in Q1 does not, and a pure word-order rewrite between them reports added: ["2000"] where main was clean. It also sits awkwardly against your own test asserting Delivered 2000 units to production. is not a year — same number, same non-temporal role, opposite verdict on position alone. Requiring a real anchor separator (: or a dash) rather than bare whitespace would close it.

5. Test coverage is drop-side only, so CI stays green through all of the above

Every added case supplies a cue and asserts it claims, or asserts a drop. The invention direction is covered only by the constructed suite 1900founded in 1900 case — and that is the direction post-process.ts treats as the worse half. The nearest negative case, "does not treat non-temporal 4-digit bare integers as year claims", checks only the drop side of Delivered 2000 units; the invention side of that same string is what fails. Whatever fix you pick, the audit belongs in the suite — the reword pairs above and the corpus strings in Blocking 3 are ready-made cases.

6. The description reports only the upside of a change #876 flagged as two-sided

## Summary and ## Changes describe the gate accurately — no overclaim about what was built, and the module 2048 example is correctly in range. But the body records no audit, no residual, and no accepted cost, while Resolves #876 would close the issue with its audit requirement unmet. Since the gate strictly narrows what year defends, a short "what this stops tracking, and why that trade is worth it" note belongs in both the PR body and the docblock, the way rules 1–3 already record their trades.

Nits

7. c\.? matches a standalone C

The optional period means the alternative fires on a bare C, case-insensitively — ANSI C 2000, Objective-C 2000, or a table cell C 2000 all classify an arbitrary quantity as a year (verified: Refactored 2000 legacy modules.Refactored Objective-C 2000 legacy modules. reports added: ["2000"]). circa is already in the alternation, so requiring the period costs nothing.

8. PEOPLE_CONTEXT_WINDOW is now also the year window

isYearContext reuses the headcount constant. Same value, unrelated concern — a neutral CONTEXT_WINDOW keeps the two gates independently tunable.


Gates

Gate Result
/code-review generic pass (high) run — findings folded in above; independently reproduced Blocking 1–3
3a fixture PII skipped — no fixture binary in the diff
3b design-system / reuse skipped — no src/components/** change
3c style tokens skipped — no styling change
3d fallow / dead code pass (CI fallow green)
3e skill & script command review skipped — no scripts/** or SKILL.md change
3f description accuracy accurate on what was built; omits the residual → Secondary 6

verify and fallow are green on CI. That is consistent with these findings rather than evidence against them — see Secondary 5.

Acceptance criteria — #876

  • ✅ Give year classification a context gate in bareIntegerClaim, mirroring the headcount verb/noun checks — isYearContext.
  • ✅ Require a temporal cue near the digit, falling back to unclaimed otherwise.
  • ✅ The issue's repro (in 1900 dropped while suite 1900 survives) is caught, and the documented-residual test is replaced with an active one.
  • "This needs its own false-positive audit." Blocking 1–3 are that failure mode, in both directions.

Verdict rule: ≥1 Blocking → REQUEST_CHANGES. Three of four ACs are met and the design is the right one — this is lexicon precision plus one architectural call (the drop-side/invention-side split), not a rework.


Reviewed by: Claude Opus 5 (high)

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
const YEAR_PREFIX_CUE = new RegExp(
"(?:\\b(?:" +
"in|since|during|by|from|until|through|between|before|after|around|circa|c\\.?|" +
"as\\s+of|class\\s+of|cohort\\s+of|batch\\s+of|and|to|" +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking 1. Bare and and to (and on the next line the transitive achievement verbs) are not temporal — they routinely precede a plain count. Combined with rule 3's strict invention check, any reword that moves a 4-digit quantity next to one of these reports an invented year and applyNumberPreservation discards the whole section.

Verified against main:

["Handled 2000 escalations across 3 products."] -> ["Handled 3 products and 2000 escalations."]
    main: ok:true     this PR: added: ["2000"]

["Serving 2000 active users."] -> ["Grew the base to 2000 active users."]
    main: ok:true     this PR: added: ["2000"]

["Fixed 1980 defects."] -> ["Completed 1980 defect fixes."]
    main: ok:true     this PR: added: ["1980"]

Contrast YEAR_FOLLOW_CUE, which is disciplined about exactly this: it spells and\s+\d{4} and to\s+(?:\d{4}|present|…) rather than bare and/to. The prefix side wants the same treatment — a preceding companion year, not the bare connective.

Also on line 298: the [(\[‘'"~]alternative claims a year for *any* parenthesised in-range number, soProcessed claims (2050).claims exactly as(2020)` does. Those two aren't separable by context.

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
// while making a claim the résumé never made.
const added = missingFrom(outputAtoms, (atom) =>
atom.claim === "headcount"
atom.claim === "headcount" || atom.claim === "year"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking 2 / the architectural call. Adding year here is what converts every cue-lexicon gap into a false invention, on top of the coverage loss the gate already causes on the drop side:

["Recipient of the 2019 Excellence Award."] -> ["Won the Excellence Award in 2019."]
    main: ok:true     this PR: added: ["2019"]

Nothing was invented — the model moved the year into temporal phrasing, which is a rewrite the prompt encourages.

Consider reverting just this line and keeping year lenient on the invention side, applying the context gate only to the drop side (line 577). #876's repro is entirely drop-side, so the gate still does the job it was filed for, and every false invention in this review disappears. The cost is that suite 1900founded in 1900 stops being caught — behaviour #876 never asked for, introduced by the added test on its own initiative.

See the review body for the full argument; your call either way.

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
"through\\s+(?:\\d{4}|present|current|now|ongoing)|" +
"and\\s+\\d{4}|" +
"jan(?:uary)?|feb(?:ruary)?|mar(?:ch)?|apr(?:il)?|may|jun(?:e)?|jul(?:y)?|aug(?:ust)?|sep(?:t|tember)?|oct(?:ober)?|nov(?:ember)?|dec(?:ember)?" +
")\\b|[)\\]’'\"`]|\\s*[-–—/]\\s*(?:\\d{4}|present|current|now|ongoing)\\b)",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking 3. This dash class is [-–—/] — ASCII hyphen, U+2013, U+2014 — but RANGE_DASH (line 165) deliberately covers U+2010–U+2015 and U+2212, and its docblock says why: "A PDF extractor emits any of these where the author typed one dash, so recognising only - would make range detection depend on the font." This class reintroduces exactly that.

["Acme Corp 2019 <dash> 2021 senior engineer."] -> ["Acme Corp senior engineer."]:

dash main this PR
- U+002D ["2019","2021"] ["2019"]
U+2013 ["2019","2021"] ["2019"]
U+2012 ["2019","2021"] ok:true
U+2011 ["2019","2021"] ok:true
U+2212 ["2019","2021"] ok:true

For three of the six dashes a whole employment date range is deleted and scored clean. For the other two it is asymmetric: the left endpoint claims here, the right endpoint matches neither cue (isRangeEndpoint needs a tight digit-dash-digit), so half the range is defended.

Interpolating RANGE_DASH.source fixes the font dependence — the same trick PEOPLE_NOUN_FOLLOW already uses on line 187. The asymmetry needs a prefix-side counterpart to this cue.

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
.slice(0, matchStart)
.trim()
.replace(/^[-*•⁃–—\s]+/, "");
if (leadingText.length === 0 && /^[\s:–—.)/]/.test(after || " ")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Secondary. The leading \s in /^[\s:–—.)/]/ makes this fire on any bullet-initial in-range number followed by a space, not just a date anchor. So 2000 users onboarded in Q1 is a year claim while Onboarded 2000 users in Q1 is not, and a pure word-order rewrite between the two reports added: ["2000"] where main was clean.

It also sits awkwardly against the test you added asserting Delivered 2000 units to production. is not a year — same number, same non-temporal role, opposite verdict decided by position alone.

Requiring a real anchor separator (: or a dash) rather than bare whitespace would close it.

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
*/
const YEAR_PREFIX_CUE = new RegExp(
"(?:\\b(?:" +
"in|since|during|by|from|until|through|between|before|after|around|circa|c\\.?|" +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit. c\.? — the optional period means this matches a standalone C, case-insensitively. ANSI C 2000, Objective-C 2000, or a table cell C 2000 all classify an arbitrary quantity as a year:

["Refactored 2000 legacy modules."] -> ["Refactored Objective-C 2000 legacy modules."]
    main: ok:true     this PR: added: ["2000"]

circa is already spelled out in the alternation, so requiring the period (c\.) costs nothing.

Comment thread src/lib/webllm/preserve-numbers.ts Outdated

const matchStart = match.index;
const before = bullet.slice(
Math.max(0, matchStart - PEOPLE_CONTEXT_WINDOW),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit. PEOPLE_CONTEXT_WINDOW is the headcount gate's constant; reusing it here couples two unrelated concerns to one value. A neutral CONTEXT_WINDOW (or a separate YEAR_CONTEXT_WINDOW) keeps them independently tunable. Same value today, so no behaviour change either way.

@shubhransh-gupta

Copy link
Copy Markdown
Contributor Author

Thank you for the thorough, precise review and for sharing the verification harness. All blocking, secondary, and nit items have been addressed in e2127d5:

1. Decoupled Invention Gate from Context Gate (Blocking 2 & 1)

  • Reverted atom.claim === 'year' on the invention check in checkNumbersPreserved (rule 3).
  • Output year claims are now lenient on additions: they only require the numeric value to have been present in the input (inputKeys.has(atom.key)). This completely eliminates false inventions on temporal rewrites (e.g. Recipient of the 2019 Excellence AwardWon the Excellence Award in 2019).
  • The isYearContext context gate is applied strictly to the drop side (rule 1), solving preserve-numbers: dropped year masked by an unrelated same-value digit #876 by ensuring dropped years (like in 1900) cannot be masked by unrelated surviving digits (like suite 1900).

2. Refined Temporal Cues in YEAR_PREFIX_CUE and YEAR_FOLLOW_CUE (Blocking 1)

  • Removed bare and, to, and transitive count verbs (completed, started, earned, published, awarded) from YEAR_PREFIX_CUE.
  • Added support for range companion years (\b(?:19\d\d|20\d\d)\s*(?:[-–—‒‑−/]|to|and)\s*), numeric month date prefixes (\d{1,2}[/.-]), and optional abbreviation dots on months (Sep. 2025).

3. Font-Independent & Symmetric Date Range Cues (Blocking 3)

  • Interpolated RANGE_DASH.source (U+002D, U+2010–U+2015, U+2212) in both YEAR_FOLLOW_CUE and YEAR_PREFIX_CUE.
  • Both range endpoints in Acme Corp 2019 <dash> 2021 are now symmetrically claimed and guarded across all 6 dash characters.

4. Tightened Leading Date Anchors (Secondary 4)

  • Require an anchor separator (:, ., ), or RANGE_DASH) following bullet-initial 4-digit numbers (leadingText.length === 0), preventing bullet-initial counts like 2000 users onboarded in Q1 from being classified as year anchors.

5. Nits & Hygiene (Nits 7 & 8)

  • Required the dot on c\. so standalone C (e.g., Objective-C 2000) does not match.
  • Defined a dedicated YEAR_CONTEXT_WINDOW = 32 constant.

6. Test Suite & Verification (Secondary 5)

  • Added comprehensive unit tests covering:
    • Masking drop prevention (in 1900 dropped with suite 1900 present)
    • All 6 dash characters across employment date ranges (Acme Corp 2019 <dash> 2021)
    • Month abbreviations with dots and slash dates (Sep. 2025 – Apr. 2026, 01/2019 - 02/2022)
    • Attributive year drops (Recipient of the 2019 Excellence Award, Presented at KubeCon 2022)
    • Attributive temporal rewords (2019 Excellence AwardAward in 2019)
    • Rewording count quantities next to connectives (Delivered 2000 units, Handled 2000 escalations, Fixed 1980 defects, 2000 users onboarded in Q1, Refactored Objective-C 2000 legacy modules)
  • Ran npm run verify: 383 test files passed, 6,449 / 6,449 tests green, build successful.

@s-annam s-annam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against #876's acceptance criteria first, the diff second, the description last.

The mechanism is right and the headline AC is met. #876 asked for a context gate on year mirroring headcount's verb/noun check, plus moving year into the count-aware presence bucket — that is what landed, and the issue's exact repro flips from ok: true to dropped: ["1900"]. npm run verify passes locally (exit 0); fallow reports no issues in the two changed files.

The problem is the cue list's calibration, and #876 called this shot in advance: "This needs its own false-positive audit — a badly-scoped gate would create the missed-legitimate-year-reword failure mode." The gate as written misses in both directions at once, and both are regressions against main:

  • Too narrow — six common résumé year forms are no longer defended at all. main caught them; this branch returns ok: true on a rewrite that deletes them.
  • Too broad — the non-temporal cues (the|a|an|our|this, of|at|for) still claim ordinary quantities as years, and because year now takes the strict drop rule, those false claims have been upgraded from harmless to section-discarding.

Net: it loses true positives and keeps (then sharpens) the false ones. Everything below was verified by running checkNumbersPreserved on this branch against origin/main.

Blocking

1. Six common year forms are silently no longer defended

YEAR_PREFIX_CUE has no cue for punctuation-adjacent years or for the credential/award verb family, and at\s+[A-Za-z0-9_-]+ spans only a single token. Before this PR every 1900–2099 integer was a defended year, so each of these is a regression — the rewrite deletes a real year and the guardrail scores it clean:

                                                          PR      main
["Speaker at PyCon (2019)."]      -> ["Speaker at PyCon."]        ok    dropped 2019
["B.S. Computer Science, 2019."]  -> ["B.S. Computer Science."]   ok    dropped 2019
["Awarded Employee of the Year 2021."] -> [... "Year."]           ok    dropped 2021
["AWS Certified Architect 2019."] -> ["AWS Certified Architect."] ok    dropped 2019
["Shipped the platform 2018."]    -> ["Shipped the platform."]    ok    dropped 2018
["Worked at Acme Corp 2019."]     -> ["Worked at Acme Corp."]     ok    dropped 2019

Controls confirm the cue list is what separates them: "Shipped the platform in 2018." is caught (the in cue), and "Presented at KubeCon 2022." is caught while "Worked at Acme Corp 2019." is not — the only difference is that Acme Corp is two tokens. The PR's own test exercises only the one-word company (KubeCon 2022), which is why the gap is invisible in CI.

These are education-section dates, certifications, conference talks and award lines — the highest-density year forms on a résumé. Wanted: cues for a year adjacent to (, , or a terminal ., an awarded|certified|earned|completed|launched|published|promoted verb family, and a multi-token span after at.

2. Two new false-revert paths, both discarding a whole section silently

The docblock's opening sets the priority this violates: "A false positive here therefore costs a whole section's rewrite, which is what drives the rules under Comparison semantics."

(a) The year survives, but in a phrasing the cue list does not recognise (preserve-numbers.ts:592). The strict count-parity rule then cannot see it:

in : ["Presented at KubeCon in 2022.", "Refactored 2022 legacy modules."]
out: ["KubeCon 2022 speaker; refactored legacy modules."]
PR: ok=false dropped=["2022"]   |   main: ok=true

2022 is right there in the output. This is finding 1's coupling: the stricter presence rule is only as safe as the cue list is complete, and it is not complete.

(b) Non-temporal cues claim ordinary quantities (preserve-numbers.ts:298,301). the|a|an|our|this and of|at|for are not temporal; any 1900–2099 quantity behind one is claimed as a year. Four plain-English merges under MERGE_AND_PRUNE_RULE, all clean on main, all false reverts here:

["Delivered the 2000 units.", "Tracked 2000 tickets."]      -> ["Delivered and tracked 2000 items."]
["Reduced by 2000 hours.", "Cut 2000 tickets."]             -> ["Cut 2000 hours and tickets."]
["A total of 2000 records.", "Indexed 2000 rows."]          -> ["Indexed 2000 records and rows."]
["Ran campaign for 2000 customers.", "Emailed 2000 leads."] -> ["Reached 2000 customers and leads."]

main: ok=true  |  here: ok=false dropped=["2000"]  (all four)

PEOPLE_VERB_PREFIX already excludes bare of for exactly this over-trigger reason — the year list should follow the same discipline.

This half is inherent, not a tuning miss: the 2019 migration and the 2000 units are syntactically identical, so a determiner cue cannot separate them. I measured the removal cost. Dropping the|a|an|our|this and the bare of|at|for fails exactly one test, "catches dropped attributive years (#876)", and within it only two of five cases:

STILL CAUGHT | Recipient of the 2019 Excellence Award.   (recipient\s+of(?:\s+the)? — own cue)
now MISSED   | The 2019 migration reduced latency.
now MISSED   | Led the 2021 platform rewrite.
STILL CAUGHT | Winner, 2020 Innovation Award.            (winner,? — own cue)
STILL CAUGHT | Presented at KubeCon 2022.                (at\s+[A-Za-z0-9_-]+ — own cue)

The issue's repro and all 79 other tests still pass. The specific phrase cues already carry most attributive coverage; only the two determiner-only cases go, and those are precisely the ones indistinguishable from the 2000 units.

3. A range's two endpoints get different presence rules

isYearContext condition 4 (preserve-numbers.ts:378) returns year for any range endpoint in 1900–2099, so a tight range straddling that boundary gets range (lenient) on one endpoint and year (strict) on the other. That breaks the dash re-spelling rule 1 exists to permit:

["Processed 1000-2000 tickets.", "Closed 2000 escalations."] -> ["Processed 1000 to 2000 tickets."]
PR: ok=false dropped=["2000"]   |   main: ok=true

# structurally identical, no year-range endpoint:
["Processed 50-100 tickets.", "Closed 100 escalations."]     -> ["Processed 50 to 100 tickets."]
PR: ok=true                     |   main: ok=true

Condition 4 is also redundant with bareIntegerClaim's own isRangeEndpoint fallback — the only thing it changes is the claim kind, and hence the presence rule. Suggest dropping it so range endpoints stay range uniformly; genuine date ranges are already covered by the prefix/follow cues (2019 - 2021, to \d{4}, and \d{4}), which the 6-dash test exercises.

4. Module docblock rule 3 now describes the opposite of what the code does

Rule 3 (preserve-numbers.ts:94-97) was rewritten to say suite 1900founded in 1900 is caught, and that an output atom read as a HEADCOUNT or a YEAR is present only when the value is claimed on the input side. The code keeps year lenient on the invention side, and the inline comment at :604-606 says so outright. Verified:

in : ["Operated out of suite 1900."]  out: ["Founded the program in 1900."]
=> ok=true added=[]                       # rule 3 claims this is caught

in : ["Completed phase 5."]           out: ["Led 5 engineers."]
=> ok=false added=["5"]                   # headcount control, correctly caught

The PR description agrees with the code, not with rule 3 — so rule 3 is the wrong artifact, and the file now contradicts itself about one branch. This is the design contract the next person reads before touching missingFrom's isPresent callbacks.

Fix: keep the lenient behaviour (your reasoning for it is sound — "2019 Excellence Award""Award in 2019" is a legitimate and common reword) and revert rule 3's add-side sentence to headcount-only, dropping suite 1900founded in 1900 from the caught list. Then state the exception explicitly: year takes the strict rule on the DROP side (rule 1) but stays lenient on the ADD side, because a year migrates into temporal context during a reword far more often than a headcount does.

Secondary

5. The new "invented years" test cannot fail

preserve-numbers.test.ts:748 asserts .added is empty across six pairs — but year is lenient on the add side (inputKeys.has), so every pair passes whenever the digit appears anywhere in the input. I ran all six against origin/main's pre-gate module: identical results, ok=true added=[] for all six. The block guards nothing this PR introduced, which also explains the contrived "Refactored Objective-C 2000 legacy modules." output — it reads as tuned to pass rather than drawn from a plausible rewrite.

Re-point these at the drop side, where the gate actually changed behaviour. The four cases in Blocking 2(b) and the six in Blocking 1 are the regression tests this change needs. By contrast "does not treat non-temporal 4-digit bare integers as year claims (#876)" (:780) is a real guard — it fails on main, passes here. That is the shape to follow.

Nits (non-blocking)

  • preserve-numbers.ts:372 — the leading-anchor regex is rebuilt with new RegExp(...) on every call, in a function running per numeric atom per bullet. RANGE_DASH.source is already interpolated into module-scope constants in this file; hoist this one the same way.
  • preserve-numbers.ts:303 / :324 — the twelve-month alternation is duplicated verbatim between the two cue regexes. Extract a MONTH_NAME constant and interpolate .source, matching how RANGE_DASH is reused two constants above.
  • preserve-numbers.ts:29 (unchanged line, so no anchor) — "Tokens covered" still reads Years (1900-2099) and date ranges: 2019, 2019-2021 unconditionally. The neighbouring line qualifies headcounts as "in people-management context"; years now need the same qualifier, and after Blocking 1 this line materially overstates coverage.
  • preserve-numbers.test.ts is not Prettier-clean (several added lines exceed the print width). Not CI-enforced — lint is eslint . and verify does not run Prettier — so cosmetic only: npx prettier --write src/lib/webllm/preserve-numbers.test.ts.
  • Two commits on the branch; main squashes from the branch, so please collapse to one before merge. I deliberately did not do it for you — this is a fork branch, and force-pushing it would rewrite your local work.

Gates

Gate Result
npm run verify pass (exit 0)
lint / typecheck pass
fallow (3d) no issues in the 2 changed files; 1 inherited finding excluded
Fixture PII (3a) n/a — no fixtures touched
Design system / reuse (3b) n/a — no components touched
Style tokens (3c) n/a — grep hits were #876 issue refs, not colours
Skill/script command bugs (3e) n/a — no .claude/** or scripts/** changes
Description accuracy (3f) accurate, unusually so — it states the lenient add-side behaviour correctly, which is what identifies the docblock as the defect in Blocking 4, and it lists the determiner cues openly. It omits their false-positive cost (Blocking 2b) and the coverage regression (Blocking 1).

A green verify is not evidence against any of this: every case above is a behaviour the existing tests do not cover.

Provenance for whoever picks this up: fork PR, 8th from this contributor; #876 was filed by the maintainer, not the author.


Reviewed by: Claude Opus 5 (high)

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
Comment on lines +296 to +298
const YEAR_PREFIX_CUE = new RegExp(
"(?:\\b(?:" +
"in|since|during|by|from|until|through|between|before|after|around|circa|c\\.|at|of|for|" +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking — the cue list is too narrow: six common year forms are no longer defended.

Before this PR every 1900–2099 integer was a defended year. These all regress — main reports the drop, this branch returns ok: true and the rewrite deletes a real year silently:

["Speaker at PyCon (2019)."]           -> ["Speaker at PyCon."]
["B.S. Computer Science, 2019."]       -> ["B.S. Computer Science."]
["Awarded Employee of the Year 2021."] -> ["Awarded Employee of the Year."]
["AWS Certified Architect 2019."]      -> ["AWS Certified Architect."]
["Shipped the platform 2018."]         -> ["Shipped the platform."]

Control: "Shipped the platform in 2018." is caught, so the cue list is exactly what separates them.

These are education dates, certifications, talks and award lines — the highest-density year forms on a résumé. Needs cues for a year adjacent to (, , or a terminal ., plus an awarded|certified|earned|completed|launched|published|promoted verb family.

This also couples to the strict drop rule below: that rule is only as safe as this list is complete.

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
"as\\s+of|class\\s+of|cohort\\s+of|batch\\s+of|" +
"founded|established|graduated|joined|winner,?\\s*|won,?\\s*|recipient\\s+of(?:\\s+the)?|" +
"the|a|an|our|this|" +
"at\\s+[A-Za-z0-9_-]+|" +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking (same finding) — this cue spans a single token, so it is company-name-shape dependent.

["Presented at KubeCon 2022."]  -> ["Presented at KubeCon."]   => dropped 2022  ✅
["Worked at Acme Corp 2019."]   -> ["Worked at Acme Corp."]    => ok: true      ❌ (main: dropped 2019)

The only difference is that Acme Corp is two tokens. The test added in this PR exercises only the one-word case (KubeCon 2022), which is why the gap is invisible in CI — worth a multi-token case alongside the fix.

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
"in|since|during|by|from|until|through|between|before|after|around|circa|c\\.|at|of|for|" +
"as\\s+of|class\\s+of|cohort\\s+of|batch\\s+of|" +
"founded|established|graduated|joined|winner,?\\s*|won,?\\s*|recipient\\s+of(?:\\s+the)?|" +
"the|a|an|our|this|" +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking — and simultaneously too broad: these are not temporal cues.

the|a|an|our|this (here) and of|at|for (line 298) claim any 1900–2099 quantity behind an article or generic preposition as a year. Because this PR also moves year into the strict drop bucket, those false claims are upgraded from harmless to section-discarding. Four plain-English merges under MERGE_AND_PRUNE_RULE, all clean on main, all false reverts here:

["Delivered the 2000 units.", "Tracked 2000 tickets."]      -> ["Delivered and tracked 2000 items."]
["Reduced by 2000 hours.", "Cut 2000 tickets."]             -> ["Cut 2000 hours and tickets."]
["A total of 2000 records.", "Indexed 2000 rows."]          -> ["Indexed 2000 records and rows."]
["Ran campaign for 2000 customers.", "Emailed 2000 leads."] -> ["Reached 2000 customers and leads."]

main: ok=true  |  here: ok=false dropped=["2000"]   (all four)

PEOPLE_VERB_PREFIX already excludes bare of for exactly this over-trigger reason.

This is inherent, not a tuning missthe 2019 migration and the 2000 units are syntactically identical, so a determiner cue cannot separate them. I measured the removal cost: dropping the|a|an|our|this and bare of|at|for fails one test ("catches dropped attributive years") and only two of its five cases — The 2019 migration and Led the 2021 platform rewrite. Recipient of the …, Winner, … and at KubeCon … survive on their own dedicated cues, and the #876 repro plus all 79 other tests still pass.

// exists to allow.
const dropped = missingFrom(inputAtoms, (atom) => {
if (atom.claim !== "headcount") return outputKeys.has(atom.key);
if (atom.claim !== "headcount" && atom.claim !== "year") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking — the strict drop rule fires when the year plainly survived.

With the cue list incomplete (line 298), count-parity cannot see a surviving year that landed in unrecognised phrasing:

in : ["Presented at KubeCon in 2022.", "Refactored 2022 legacy modules."]
out: ["KubeCon 2022 speaker; refactored legacy modules."]

PR: ok=false dropped=["2022"]   |   main: ok=true

2022 is right there in the output, and per the module docblock a false positive here discards the entire section rewrite silently — the expensive direction. This finding and the coverage gap are one problem: tightening presence is only safe once the cue list is complete.

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
}

// 4. Tight range endpoint joining another digit (e.g. "2019-2021")
if (isRangeEndpoint(match, bullet)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking — the two endpoints of one range get different presence rules.

Condition 4 returns year for any range endpoint in 1900–2099, so a tight range straddling that boundary is range (lenient) on one side and year (strict) on the other. That breaks the dash re-spelling rule 1 exists to permit:

["Processed 1000-2000 tickets.", "Closed 2000 escalations."] -> ["Processed 1000 to 2000 tickets."]
PR: ok=false dropped=["2000"]   |   main: ok=true

# structurally identical, no year-range endpoint:
["Processed 50-100 tickets.", "Closed 100 escalations."]     -> ["Processed 50 to 100 tickets."]
PR: ok=true                     |   main: ok=true

Condition 4 is also redundant with bareIntegerClaim's own isRangeEndpoint fallback — the only thing it changes is the claim kind, and hence the presence rule. Suggest dropping it so range endpoints stay range uniformly; genuine date ranges are already covered by the prefix/follow cues (2019 - 2021, to \d{4}, and \d{4}) that the 6-dash test exercises.

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
Comment on lines +94 to +96
* → `5 engineers` or `suite 1900` → `founded in 1900` reuses the digit and
* invents the claim, and under a plain rule-1 lookup it scored clean. So an
* output atom the surrounding prose reads as a HEADCOUNT or a YEAR counts as

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking — this now states the opposite of what the code does.

Rule 3 says an output atom read as a HEADCOUNT or a YEAR is present only when the value is claimed on the input side, and cites suite 1900founded in 1900 as caught. The code keeps year lenient on the invention side, and the inline comment at line 604 says so outright:

in : ["Operated out of suite 1900."]  out: ["Founded the program in 1900."]
=> ok=true added=[]                      # rule 3 claims this is caught

in : ["Completed phase 5."]           out: ["Led 5 engineers."]
=> ok=false added=["5"]                  # headcount control, correctly caught

Your PR description agrees with the code, not with this paragraph — so this is the wrong artifact, and the file now contradicts itself about one branch. It is the design contract the next person reads before touching missingFrom's isPresent callbacks.

Fix: keep the lenient behaviour (the reasoning is sound — "2019 Excellence Award""Award in 2019" is a legitimate, common reword) and revert this sentence to headcount-only, dropping suite 1900founded in 1900 from the caught list. Then say the exception outright: year takes the strict rule on the DROP side (rule 1) but stays lenient on the ADD side, because a year migrates into temporal context during a reword far more often than a headcount does.

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
.replace(/^[-*•⁃–—\s]+/, "");
if (
leadingText.length === 0 &&
new RegExp(`^\\s*(?:[:.)]|${RANGE_DASH.source})\\s*`).test(after)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit — this regex is rebuilt on every call, inside a function that runs per numeric atom per bullet. RANGE_DASH.source is already interpolated into module-scope constants in this file (PEOPLE_NOUN_FOLLOW, YEAR_PREFIX_CUE); hoist this one the same way.

Comment thread src/lib/webllm/preserve-numbers.ts Outdated
"founded|established|graduated|joined|winner,?\\s*|won,?\\s*|recipient\\s+of(?:\\s+the)?|" +
"the|a|an|our|this|" +
"at\\s+[A-Za-z0-9_-]+|" +
"jan(?:uary)?\\.?|feb(?:ruary)?\\.?|mar(?:ch)?\\.?|apr(?:il)?\\.?|may|jun(?:e)?\\.?|jul(?:y)?\\.?|aug(?:ust)?\\.?|sep(?:t|tember)?\\.?|oct(?:ober)?\\.?|nov(?:ember)?\\.?|dec(?:ember)?\\.?|" +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit — this twelve-month alternation is duplicated verbatim in YEAR_FOLLOW_CUE (line 324). Extracting a MONTH_NAME constant and interpolating .source matches how RANGE_DASH is reused two constants above, and keeps the two lists from drifting apart.

Comment thread src/lib/webllm/preserve-numbers.test.ts Outdated
expect(result.added).toEqual([]);
});

it("does not falsely report invented years when quantities move next to connectives/verbs (#876)", () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Secondary — this test cannot fail.

year is lenient on the add side (inputKeys.has), so .added is empty for every pair as long as the digit appears anywhere in the input. I ran all six pairs against origin/main's pre-gate module: identical results, ok=true added=[] for all six. The block guards nothing this PR introduced.

That also explains the contrived "Refactored Objective-C 2000 legacy modules." output in the last pair — it reads as tuned to pass rather than drawn from a plausible rewrite.

Re-point these at the drop side, where the gate actually changed behaviour. The four merge cases in the YEAR_PREFIX_CUE thread and the six coverage cases are the regression tests this change needs. By contrast "does not treat non-temporal 4-digit bare integers as year claims" at line 780 is a real guard — it fails on main and passes here; that is the shape to follow.

@shubhransh-gupta
shubhransh-gupta force-pushed the sg/preserve-numbers-year-context-gate branch from e2127d5 to 654b46d Compare August 29, 2026 04:32
@shubhransh-gupta

Copy link
Copy Markdown
Contributor Author

All review findings from the latest round have been resolved in squashed commit 654b46d:

1. Common Resume Year Forms Defended (Blocking 1)

  • Expanded YEAR_PREFIX_CUE to defend degrees (B.S. Computer Science, 2019), parenthesized conference talks (Speaker at PyCon (2019)), credential and award lines (AWS Certified Architect 2019, Awarded Employee of the Year 2021), multi-token companies/conferences after at (Worked at Acme Corp 2019, Presented at KubeCon 2022), and action phrases (Shipped the platform 2018).
  • Added drop-side regression tests verifying that deleting any of these common year forms triggers a drop.

2. Determiners & Generic Prepositions Removed (Blocking 2b)

  • Removed the|a|an|our|this and bare of|at|for from YEAR_PREFIX_CUE.
  • Added unit tests for the four plain-English merges under MERGE_AND_PRUNE_RULE (Delivered the 2000 units, Reduced by 2000 hours, A total of 2000 records, Ran campaign for 2000 customers), asserting all score clean with ok: true.

3. Range Endpoints Uniformity (Blocking 3)

  • Dropped Condition 4 from isYearContext. Both endpoints of tight numeric ranges (such as 1000-2000) now uniformly receive range claim classification across the 1900–2099 boundary, preserving dash re-spelling leniency while date ranges continue to be defended by explicit temporal cues.

4. Docblock & Contract Alignment (Blocking 4)

  • Updated Rule 3 in preserve-numbers.ts docblock to state the contract accurately: headcount is strictly gated on additions, while year takes the count-aware parity check on the drop side (Rule 1) and remains lenient on the add side to support legitimate temporal rewords (2019 Excellence AwardAward in 2019).

5. Nits & Code Quality

  • Hoisted MONTH_NAME and LEADING_DATE_ANCHOR_SEPARATOR to module scope.
  • Qualified the "Tokens covered" docblock line (Years (1900-2099) in temporal context and date ranges).
  • Formatted preserve-numbers.ts and preserve-numbers.test.ts with Prettier.
  • Squashed branch commits into a single commit 654b46d.

6. Verification

  • npm run verify passed: 383 test files passed, 6,451 / 6,451 tests green.

@shubhransh-gupta
shubhransh-gupta force-pushed the sg/preserve-numbers-year-context-gate branch from 654b46d to 6af63d4 Compare August 29, 2026 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

preserve-numbers: dropped year masked by an unrelated same-value digit

2 participants