Conversation
…tection (#166) A role header whose org name and closing date-year wrap onto extra physical rows ("… Community Heritage May 2023 - June" / "Museum" / "2024") left the anchor line carrying an incomplete date range, so DATE_RANGE_RE missed it, no date_range entry block opened, and the role's bullets fell into the unmatched "Other" group. Add mergeWrappedHeaderRows: scoped to the date_range anchor and run before collectAnchors, it folds the continuation rows directly below a dangling-date header back into one logical header — left-column fragments (org tail) append to the text, right-column fragments (the wrapped year) append to the date region, keyed off the date-region start so "June" and "2024" reassemble adjacently. A final DATE_RANGE_RE match gate means a header that already carries a complete range never folds, so the common "Company Dates / Title / bullets" stack is untouched. On the google-docs-skia-proxy-multiline-bullets-coursework fixture the Docent role now forms an experience[] entry (title + org + dates) and its bullets attribute to the role. Regenerated the affected corpus snapshot (experienceCount 2→3, confidence 0→0.88, suggestedEscalation ocr→none) and added targeted unit tests. Resolves #166 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018GAj62FwHV2WHunBTGCR9P
… complexity (#167) Address fallow review on PR #167: make mergeWrappedHeaderRows module-private (only parseEntryBlocks calls it) and extract tryFoldHeaderAt + foldHeaderText so the function drops below the cognitive-complexity threshold. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018GAj62FwHV2WHunBTGCR9P
Vaishnavi1709
left a comment
There was a problem hiding this comment.
Verdict: approve with minor cleanups. Tight scope, the right shape, and a strong final-match-gate safety net. A genuine improvement on its own — and notably independent of PR #165.
🟢 What's working well
- Anchor-scoped fold (
cfg.anchor === "date_range"only) — the comment inparseEntryBlocksexplains exactly why other anchors can't wrap-incomplete in the same way. Clean. - The match gate is the load-bearing safety net —
if (!hasCompleteDateRange(folded)) return nullmeans a fold only commits when it produces a complete range. That's the right invariant to lock down, and the second test ("does not fold a complete single-line header") pins the non-regression cleanly. foldHeaderTextleft/right column split keyed offdateRegionStart— the "June" + "2024" reassemble-adjacently insight is exactly the geometry/content combo this needs.- Helper extraction (
tryFoldHeaderAt,foldHeaderText) responded to fallow — function bodies now read straight through, no deep nesting. Nice cleanup. - Snapshot delta is in the intended direction only. I verified the coursework fixture flips
confidence 0 → 0.88,suggestedEscalation ocr → none,experienceCount 2 → 3. The Docent role lands as its own experience entry, its bullets attribute correctly, and (by recovering its chars intoexperience.description) the extracted/raw ratio climbs back aboveEXTRACTION_RATIO_FLOOR. Net: this fixture is fully recovered.
A note for sequencing context: this PR does not address the student-projects-activities-singlecol snapshot delta I flagged on PR #165 — different fixture, different cause. They're independent. If both PRs touch entry-blocks.ts near the imports / parseEntryBlocks body, expect a small textual conflict on merge order, but no semantic interaction — mergeWrappedContinuations folds bullet bodies, mergeWrappedHeaderRows folds header rows. They're complementary layers.
🟡 Cleanups (non-blocking)
1. The lastIndex comment in hasCompleteDateRange is more defensive than the regex actually requires
// non-global but `.test` advances `lastIndex` on some engines; reset so calls are idempotent.
DATE_RANGE_RE is built with just "i" (no g, no y); per spec, non-global non-sticky regexes leave lastIndex untouched across .test() / .exec() calls. PRESENT_RE is the same. The reset itself is harmless, but the comment's reasoning isn't quite right — could you reword to something like "defensive reset in case the flag changes later"? The resets inside dateRegionStart on MONTH_YEAR_RE / NUMERIC_MONTH_YEAR_RE / YEAR_RE are genuinely needed — those three are global, so those are good as-is.
2. Worth bounding the continuation rows consumed
tryFoldHeaderAt's while loop consumes any non-bullet, non-complete-range line until a bullet or complete-date anchor. The match gate prevents committing an arbitrary fold, but a shape like:
"Title at Org Jan 2020 - Dec" ← anchor (incomplete, dateIdx > 0)
"Subtitle text" ← prose, not a real continuation
"Long description" ← more prose
"2024" ← genuine date tail (bare year, content-classified to right)
"● Bullet"
…would pass the match gate (final text "Title at Org Subtitle text Long description Jan 2020 - Dec 2024" matches DATE_RANGE_RE) and collapse four physical lines into one header — absorbing real subtitle/description content. Probably uncommon in the corpus, but the existing cfg.headerLookback ?? 0 config already encodes the intent of bounded lookahead. Worth capping continuations at cfg.headerLookback || 2 to match, or noting in the docstring why no cap is needed. Small change, nice predictability win.
3. Wrapped Present case isn't supported
If Present wraps onto its own line:
"Senior Engineer Jan 2022 -"
"Present"
"● Bullet"
…tryFoldHeaderAt enters the while loop, but hasCompleteDateRange("Present") is true (PRESENT_RE matches), so the loop exits immediately with conts = []. Result: no fold. Probably rare in practice (most renderers don't wrap a six-letter word), but worth either acknowledging in the docstring, or refining the gate to distinguish "this line completes the in-progress fold" from "this line starts a new complete anchor."
4. Two more tests would tighten the contract
The current two tests pin the motivating shape and the common-shape non-regression. Two additions would round it out:
- A section with a mix of complete and incomplete anchors back-to-back (multi-role) — to confirm the fold doesn't extend across role boundaries.
- A case where the continuation rows don't complete a range — to confirm the match gate rejects and the lines pass through untouched.
Both are small additions to the existing xSection-based tests.
Verdict
Approve — clean fix, contained scope, snapshot delta is exactly what the PR body says. The cleanups above are all minor; address when convenient.
Address the four non-blocking cleanups from the PR #167 approval: - Reword the hasCompleteDateRange lastIndex comment — DATE_RANGE_RE / PRESENT_RE are non-global non-sticky, so the reset is a defensive no-op, not required. - Bound the continuation gather at the section's headerLookback (maxConts) so a fold can't vacuum a stray subtitle + description into a header when a bare year sits a few lines down. - Support a wrapped open-ended range: a bare "Present"/year tail on its own line is now read as a continuation (startsNewAnchor distinguishes it from a new standalone role anchor), so "… Jan 2022 -" / "Present" reassembles. - Add three tests: wrapped Present, no-fold across a role boundary, and the match-gate passthrough when continuations never complete a range. Refs #166
Address the four non-blocking cleanups from the PR #167 approval: - Reword the hasCompleteDateRange lastIndex comment — DATE_RANGE_RE / PRESENT_RE are non-global non-sticky, so the reset is a defensive no-op, not required. - Bound the continuation gather at the section's headerLookback (maxConts) so a fold can't vacuum a stray subtitle + description into a header when a bare year sits a few lines down. - Support a wrapped open-ended range: a bare "Present"/year tail on its own line is now read as a continuation (startsNewAnchor distinguishes it from a new standalone role anchor), so "… Jan 2022 -" / "Present" reassembles. - Add three tests: wrapped Present, no-fold across a role boundary, and the match-gate passthrough when continuations never complete a range. Refs #166
Address the four non-blocking cleanups from the PR #167 approval: - Reword the hasCompleteDateRange lastIndex comment — DATE_RANGE_RE / PRESENT_RE are non-global non-sticky, so the reset is a defensive no-op, not required. - Bound the continuation gather at the section's headerLookback (maxConts) so a fold can't vacuum a stray subtitle + description into a header when a bare year sits a few lines down. - Support a wrapped open-ended range: a bare "Present"/year tail on its own line is now read as a continuation (startsNewAnchor distinguishes it from a new standalone role anchor), so "… Jan 2022 -" / "Present" reassembles. - Add three tests: wrapped Present, no-fold across a role boundary, and the match-gate passthrough when continuations never complete a range. Refs #166
Summary
A role header whose org name and closing date-year wrap onto extra physical rows (
… Community Heritage May 2023 - June/Museum/2024) left the anchor line carrying an incomplete date range, soDATE_RANGE_REmissed it, nodate_rangeentry block opened, and the role's bullets fell into the unmatched "Other" group.New
mergeWrappedHeaderRows(scoped to thedate_rangeanchor, run beforecollectAnchors) folds the continuation rows below a dangling-date header back into one logical header — left-column fragments (org tail) append to the text, right-column fragments (wrapped year) append to the date region, keyed off the date-region start soJune+2024reassemble adjacently. A finalDATE_RANGE_REmatch gate means a header that already carries a complete range never folds, so the commonCompany Dates / Title / bulletsstack is untouched.On the
google-docs-skia-proxy-multiline-bullets-courseworkfixture the Docent role now forms anexperience[]entry (title + org + dates) and its bullets attribute to the role. Independent of #162/#163/#164 (the header layer, wheremergeWrappedContinuationsdoes not reach).Closes #166
Test plan
npm run typecheckcleannpm run testgreen (747 passed, +2 new unit tests)experienceCount2→3,confidence0→0.88,suggestedEscalationocr→none); no score-dimension change🤖 Generated with Claude Code