Skip to content

[parser] skills — the final wrap of a comma-separated list never rejoins, shredding the last multi-word skill #834

Description

@s-annam

Context — what a knownWrong entry is

Each corpus fixture under tests/fixtures/pdfs/ may carry a hand-authored ground-truth sidecar (*.truth.json, minted under #654). Its knownWrong block records, per field, a place where the parser disagrees with what the page actually draws, with a status of open (live bug, issue must be open), accepted (written-down tradeoff), or unfiled (measured, never filed — issue: null).

unfiled is capped at UNFILED_TRUTH_CEILING = 10 (src/lib/heuristics/corpus.test.ts:186) and is saturated at 10/10 today. This issue files one of them.

What's wrong

When a comma-separated skills list wraps, the last wrap is never rejoined, so the skill straddling that break is split into two junk tokens.

tests/fixtures/pdfs/unknown/single-word-name-mononym.pdf draws (pdftotext -layout):

SKILLS
Python, Go, TypeScript, PostgreSQL, Kafka, Kubernetes, Terraform, AWS, gRPC, Distributed
Systems

Expected skills (from the fixture's truth sidecar): Python, Go, TypeScript, PostgreSQL, Kafka, Kubernetes, Terraform, AWS, gRPC, Distributed Systems — ten skills.

Observed: eleven, ending …, gRPC, Distributed, Systems. Distributed Systems is destroyed and neither half is a real skill.

Recorded today as a status: "unfiled" knownWrong entry on skills in tests/fixtures/pdfs/unknown/single-word-name-mononym.truth.json.

This is common, not exotic: any comma-delimited skills row long enough to wrap ends its final visual line with a fragment, and a multi-word skill in that position always splits.

Not a duplicate of #301

#301 (closed, completed 2026-07-03) fixed the round-trip direction — a multi-word skill split at the boundary our own exporter's word-wrap introduced, parse → render → re-parse. This is the source-parse direction: the split happens on the user's original PDF, before any export, and #301's fix does not reach it.

Root cause

src/lib/heuristics/extract/skills.ts:388-500, isSoftWrapContinuation. The function decides whether a pending line and the next line are one wrapped list. It has four join conditions:

Condition Fires when Line
A′ next line LEADS with a bare & / + connector ~415
C pending has an unclosed paren AND ends on a comma, and the clarifier closes within the look-ahead (#465) ~460
A pending ENDS with a standalone &, -, , + (#301) ~487
B nextText.includes(",") && pending.includes(",") && !/[,;]\s*$/.test(pending) ~493

Trace this fixture:

  • pending = "Python, Go, …, gRPC, Distributed" — has commas, does not end on one.
  • nextText = "Systems".

Condition A fails (no trailing connector glyph). A′ fails (no leading connector). C fails (no unclosed paren). Condition B fails on its first conjunct: "Systems" contains no comma.

That conjunct is what makes B blind to the last wrap specifically. Condition B's design assumption is that a continuation line is "itself part of a longer comma-separated list that wrapped" — true for every wrap except the final one, whose tail is by definition the end of the list and carries no further comma. So B rejoins every intermediate wrap of a 3+ line list and drops exactly the last.

The pending.includes(",") conjunct is separately load-bearing and correct — its comment explains it keeps a comma-less standalone skill (Machine Learning) from being swallowed by a following independent list. That half must survive any fix.

Implementation plan

  1. src/lib/heuristics/extract/skills.ts — add Condition B′ for the terminal wrap. Keep B untouched; add a narrower sibling that handles the comma-less tail:

    // Condition B′: the FINAL wrap of a comma-separated list. B requires a comma
    // in `nextText` because an intermediate continuation is itself mid-list — but
    // the LAST continuation ends the list and carries no further comma
    // ("…, gRPC, Distributed" ⏎ "Systems"). Without B′ that tail is never
    // rejoined and the straddling skill is shredded into two non-skills.
    //
    // Narrower than B on purpose, because a comma-less next line is also what an
    // independent standalone skill on its own row looks like:
    //  1. `pending` is an unterminated list fragment — same two conjuncts as B.
    //  2. `nextText` is SHORT (a skill tail, not a new row): at most
    //     TERMINAL_WRAP_MAX_WORDS words.
    //  3. `nextText` is the LAST cell of the section — nothing follows it that
    //     could make it a list of its own.

    Conjunct 3 is what keeps B′ from merging a genuine standalone final skill into the row above it. It needs the look-ahead the caller already threads in: isSoftWrapContinuation receives upcoming (built by upcomingContinuationTexts, skills.ts:582) — require upcoming.length === 0.

    Pick TERMINAL_WRAP_MAX_WORDS as a named constant beside CLARIFIER_WRAP_LOOKAHEAD. 2 covers Distributed Systems, Machine Learning, Data Engineering; 3 covers Amazon Web Services. Start at 3 and prove no corpus fixture regresses; if one does, drop to 2 and say so in the constant's docblock.

  2. Do not weaken Condition B. Removing nextText.includes(",") from B would rejoin any comma-less line into any preceding list — exactly the false merge B's own comment warns about. B′ exists so the fix is additive.

  3. Check the interaction with collectSkillCells. Joining happens one hop at a time (skills.ts:522-560); confirm a B′ join is a terminal hop and cannot chain, so a two-line tail cannot accumulate a third line.

  4. Flip the ground-truth entry in tests/fixtures/pdfs/unknown/single-word-name-mononym.truth.jsonskills, to status: "open" with this issue's number. If the fix lands in the same PR, delete the entry instead. Do not edit the truth VALUES — a truth file records what the page draws and is never edited to match the parser (see its provenance).

  5. Lower UNFILED_TRUTH_CEILING (src/lib/heuristics/corpus.test.ts:186) by 1. Sibling issues from the same audit also lower it — on a rebase conflict, take the LOWER number.

Acceptance criteria

  • single-word-name-mononym.pdf parses exactly ten skills, ending with Distributed Systems as one token.
  • A genuine standalone final skill is NOT merged: a section drawn as "Python, Go, Rust""Machine Learning" where Machine Learning is the last cell still yields four skills, not three. This is the exact case conjuncts 2 and 3 have to separate from the bug — if it cannot be separated, say so in the issue rather than shipping the merge. Unit test both directions.
  • A 3+ line wrapped list still rejoins at every intermediate break (Condition B) AND at the final one (B′): "A, B, C,""D, E, Distributed""Systems".
  • The [parser] skills — bulleted single-column labelled skills shred multi-word tokens and ingest category labels #465 clarifier cases (Condition C) and the Skills: multi-word skill token splits at line-wrap boundary on round-trip #301 compact-skill cases (C++, PHP7+ must NOT trigger Condition A) are unchanged — regression tests already exist for both; they must stay green.
  • npx vitest run src/lib/heuristics/corpus.test.ts passes with skills precision AND recall unchanged or improved on all 58 fixtures — a fix that trades a working fixture for this one is a loss, not a win.
  • npx vitest run src/lib/heuristics/corpus-roundtrip.test.ts passes with no new KNOWN_FAILURES baseline rows.
  • npm run check:baselines reports 1 fewer unfiled entry.
  • npm run verify passes.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingux:parsingUX program: parsing accuracy as the user experiences it

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions