fix(heuristics): two-column flatten keeps flat-bullet awards as achievements (#131) - #144
Conversation
… keeps achievements (#131) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
s-annam
left a comment
There was a problem hiding this comment.
PR Review: two-column flatten keeps flat-bullet awards as achievements (#131)
Summary
Clean, strictly-additive fix. The zero-anchor flat bullet list (awards where every item is a bullet, no header line) was dropped entirely; now each top-level bullet becomes an entry, marker-less lines fold into the title, and deeper sub-bullets become the body. Well-scoped, well-tested, and re-seats correctly on the #127 bulletMarkerX/isWrappedContinuation helpers rather than duplicating them.
Gates (run on checked-out branch): npm run typecheck clean · npm run test 628/628 · PR diff touches only the 2 target snapshots (no masked corpus regression).
Spec alignment (#131)
| AC | Status |
|---|---|
Two-column Awards parse as 2 achievements (weasyprint-cairo, google-docs-skia-proxy) |
✅ achievementsCount 0→2 |
| No single-column regression | ✅ only 2 snapshots differ, rest byte-identical |
| Reuse #127 helpers, no duplication | ✅ |
Stay under cognitive-complexity bar (split buildBulletEntry) |
✅ |
Highlights
- Trigger is tightly gated (
anchors.length === 0 && cfg.anchor === "first_line") so other anchors keep returning[]and real-header sections are untouched — genuinely additive. - Top-level vs sub-bullet split via
x <= markerX + 2matches the existingisWrappedContinuationtolerance — consistent margin logic. - Two targeted unit tests (flat split + sub-bullet-as-body) plus a clean corpus rebake.
Key findings (non-blocking)
- [Suggestion]
buildBulletEntryroutes every marker-less line to the title regardless of position relative to sub-bullets — a wrapped continuation of a body sub-bullet would be pulled up into the title. See inline. Unreachable for the current award fixtures; forward-looking hardening. - [Informational] The fallback fires for any zero-anchor
first_linesection, projects included — not just achievements. Almost certainly an improvement (no fixture regressed), but the docstring frames it as awards-specific; a one-line note that it's generic tofirst_linewould match the behavior.
Verdict
APPROVE — no blocking items; correct, additive, gates green, snapshots clean.
…t awards (#131) Address review on PR #144: in buildBulletEntry a marker-less line was folded into the entry title regardless of position. Now a marker-less line is a title continuation only before the first sub-bullet (the on-its-own-line year case); after a sub-bullet it joins that bullet's text in the body. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
s-annam
left a comment
There was a problem hiding this comment.
APPROVE — 0 blocking findings.
Gates (checked-out branch, fresh npm install)
- ✅
tsc -b --noEmitclean - ✅ vitest 629/629; new
entry-blocks.test.ts15/15 - ✅ Only the 2 target snapshots change (
weasyprint-cairo-two-column,google-docs-skia-proxy-two-column),achievementsCount0→2 — matches acceptance criteria - ✅ No PDF fixtures touched → PII-safe (count-only snapshot deltas)
- ✅ 150/3 LOC, single concern, reuses #127 helpers (
bulletMarkerX/isBulletLine) instead of duplicating
Logic spot-check
buildBulletEntry title-vs-body state machine verified: marker-less line before the first sub-bullet folds into the title; after a sub-bullet it joins that bullet's tail. Windowing anchorIdx+1 .. nextIdx correct, no off-by-one. The 3 new unit tests cover title-fold, sub-bullet-as-body, and wrapped-tail.
Non-blocking follow-ups (not gating this PR)
- Date-only bullet → empty-title achievement. A bullet that is only a year (
• 2023, no name) yields{title:"", year:"2023"}with score 0, emitted unfiltered (achievementFromBlock). This is a pre-existing systemic behavior shared by all three extractors, not introduced here. Cheap hardening if wanted: filter empty-headerLinesblocks inextractAchievements/extractProjects. Low priority (unrealistic input). - Greedy year-strip on joined titles.
parseDateRange'sYEAR_REfallback consumes any bare 4-digit year mid-phrase (e.g.• ACM Fellow, Class of 2019). Pre-existing shared-parser limitation, now exercised at a new call site — worth a one-line JSDoc note.
The two parseBulletList [] guards (!isFinite markerX, zero top-level anchors) are unreachable given the caller gate + markerX = min(x) — safe defensive code, no action needed.
Ship it.
What
Closes #131. A two-column résumé whose Awards/Achievements section is a flat bullet list (every item is itself a bullet, no name-led header line) was dropped entirely:
collectAnchorsfound zerofirst_lineanchors andparseEntryBlocksreturned[].This adds a bullet-anchored fallback: when a
first_linesection yields no anchors, each top-level bullet (one sitting at the bullet-marker margin) becomes its own entry. Marker-less lines below it (a bare year, a wrapped award name) fold into that entry's title; deeper-indented sub-bullets become its body.The fallback is strictly additive — only
first_linesections that previously returned[]are affected. Sections with a real header keep ≥1 anchor and are untouched.Why this shape
The issue's original framing (two bullet margins interleaved in one section, global
bulletMarkerXrejecting lines) was stale.detectColumnBoundariesalready bands these fixtures into single-column reading order before sectioning, so each section is one column's clean list. The remaining bug was purely the zero-anchor flat list. This fix also re-seats cleanly on top of the #127 segmentation rework that landed on main (bulletMarkerX/isWrappedContinuation), reusing those helpers rather than duplicating them.Acceptance criteria
weasyprint-cairo-two-columnAwards parse as two distinct achievements (Globex Engineering Excellence — 2021,Acme Innovation Prize — 2023) —achievementsCount0 → 2. Same for the siblinggoogle-docs-skia-proxy-two-column.google-docs-skia-proxy-nonstandardprojects stays 1;student-projects-activities-singlecolunchanged; main's wrapped-bullet-tail handling preserved.Tests
entry-blocks.test.ts(flat-list split; sub-bullet-as-body).main; all others byte-identical. No score changes.npm run typecheck, fullvitest(628 passed), andeslinton changed files all clean.🤖 Generated with Claude Code