fix(education): strip middot/bullet before in/of connective (#839)

[shubhransh-gupta]

Summary

• Widen parseDegreeAndField's separator strip to include · and •, matching cleanField
• Fixes "B.S. · in Computer Science" leaking the connective into the field
• Adds regression tests; hyphen/em-dash and interior middot cases unchanged

Fixes #839

Test plan

• npx vitest run src/lib/heuristics/extract/education.test.ts (67 tests)
• Pre-push npm run verify (scoped tests + build passed locally)

[shubhransh-gupta]

Follow-up to #840 — fixes #839 with the one-line separator widening from the issue's acceptance criteria (·/• added to parseDegreeAndField's strip class) plus regression tests.

Local npm run verify passed (67 education tests + scoped suite). Ready for review whenever you have time — thanks!

[s-annam]

Approving. The fix is the minimal, correct widening #839 asked for, it is pinned by tests that genuinely fail without it, and it moves no fixture numbers. Two non-blocking items below; the one that was safe to fix without changing behaviour is already pushed.

Verdict rule: 0 Blocking → APPROVE. Secondary/Nits do not gate.

What I verified (not taken from the description)

• Fail-before proof. Swapped education.ts back to origin/main and re-ran only the new #839 block: strips middot then in connective fails with "in Computer Science", strips bullet then of connective fails with "of Computer Science"; both pass with the change. The other two cases pass before and after — correct, because AC 3 and AC 4 asked for them as pins on the widening, not as evidence of it.
• Regex. [-–—,:·•] is well-formed: the - is first in the class so it is literal, no accidental range. Bytes checked (c2 b7 middot, e2 80 a2 bullet).
• Blast radius. parseDegreeAndField has three call sites, all inside education.ts. cleanField's edge strip already accepted ·/• before this PR (#835), so no field that legitimately starts with one of those glyphs changes behaviour — they were being stripped downstream already. This PR only moves the strip early enough to also expose the in/of connective behind it.
• Shared-constant check (issue step 2). No exported separator class exists that this could consume — line-primitives.ts's BULLET_CLASS is module-local and semantically different (list-bullet detection: includes *, ●; excludes ·, ,, :). So the local widening is the right scope, exactly as the issue scoped it, and #653 keeps the unification.
• Gates. npm run verify → exit 0 (typecheck, eslint, check:fixtures 58 PDFs + 15 sidecars, check:baselines, check:core, scoped tests, vite build). npx vitest run src/lib/heuristics/corpus.test.ts → 62 passed / 1 skipped, no recall movement in any dimension, as #839 predicted. fallow's 3 complexity findings on extractEducation / educationFromChunk / stripInstitutionLocation are all pre-existing and report-only — not charged to this PR.

Acceptance criteria (#839) — 7/7 met

AC	Status
"B.S. · in Computer Science" → field Computer Science	met (test, fail-before confirmed)
"B.S. • of Computer Science" → field Computer Science	met (test, fail-before confirmed)
hyphen + em dash unchanged, asserted	met
#835 interior-separator guard still holds	met
tests in education.test.ts, sibling describe	met
corpus.test.ts green, no fixture regressions	met (verified locally)
npm run verify passes	met (verified locally)

Secondary

The sibling classes still disagree — one glyph over. See the inline note on education.ts. Not blocking: it is outside #839's ACs, the issue explicitly hands vocabulary unification to #653, and the shape is rare. But it does make the description's "matching cleanField" a near-match rather than a match, and it is the same defect #839 was.

Nits

• Two of the four new tests are regression guards, not pins — see the inline note on education.test.ts. Correct to have; just worth a reader not mistaking them for proof of this fix.

Fixed in dfca1b3

• education.ts:536 — the comment claimed to name the glyphs the class drops but omitted – (en dash, which was already in the class), and carried no issue citation, which this file otherwise does on nearly every behaviour-driving comment (#835, #297, #371, …). Rewrote it to name every glyph, say why the middot/bullet have to live in this strip rather than only in cleanField, and point the residual ; gap at #653. Comment-only — no behaviour change, and npm run verify is green on the pushed head.

Note for whoever merges: this is now two commits on the branch. Squash it (the repo's one-commit-per-PR rule) — the second is mine, not the author's.

Gates run

check:fixtures (green, no fixture touched by this PR), design-system / style-token / skill-script gates N/A — the diff is two files under src/lib/heuristics/extract/, no UI, no scripts, no fixtures. fallow: green-for-this-diff (all findings inherited). Description audit: accurate apart from the "matching cleanField" overstatement noted above; both test-plan boxes independently re-verified green here.

Reviewed at dfca1b3fe8b9c14a6a0cf496adfa95d7cf713ac3.

---

Reviewed by: Claude Opus 5 (high)

[s-annam]

Secondary — the two classes still disagree, one glyph over. cleanField's edge strip is [\s,;:·•–\-—]; this one is [-–—,:·•]. The ; is in the first and not the second, and the ordering that caused #839 is unchanged — this strip runs before the two connective strips, cleanField runs after. Verified on the pushed head by probing extractEducation directly:

"B.S. ; in Computer Science"  → degree "B.S.", field "in Computer Science"    ← still leaks
"B.S. · in Computer Science"  → degree "B.S.", field "Computer Science"       ← fixed by this PR
"B.S. ‣ in Computer Science"  → degree "B.S.", field "‣ in Computer Science"  ← in neither class

Not blocking: ; is outside #839's acceptance criteria, the issue explicitly hands separator-vocabulary unification to #653, and a Degree ; in Field header is rarer still than the middot one. Flagging it because it is exactly the class of finding #839 was, and the next reader of these two strips will again assume they agree. If you want to close it here rather than leave it to #653, the issue's own step 3 is the shape: one module-level const EDGE_SEPARATOR_CLASS consumed by both strips, which makes the drift impossible instead of merely unlikely. Either way this PR is approved as-is.

[s-annam]

Nit — this test and the interior-middot one below it pass identically before and after the change. I confirmed by reverting education.ts to origin/main and running just this describe block: the two middot/bullet cases fail, these two pass. That is correct — AC 3 and AC 4 of #839 asked for them precisely so the fix is pinned as a widening rather than a rewrite — but a later reader scanning a #839 describe block will reasonably read all four as evidence the fix works. One line of comment saying these two are regression guards ("unchanged by #839; here so a future narrowing of the class is caught") would carry that.

Second, smaller: expect(hyphen[0].field) and expect(emDash[0].field) index into the array without an expect(...).toHaveLength(1) first, unlike the two tests above. If a future change split this header into two entries, [0] would still satisfy the assertion and the test would stay green on a real regression.

[s-annam]

Approved — nice work, @shubhransh-gupta. All seven acceptance criteria on #839 are met, the fail-before was proven for both load-bearing cases, and the corpus run showed no recall movement. This is a clean first contribution.

I pushed a commit to your branch — here's exactly what and why

dfca1b3 — comment-only, no behaviour change. Your code does what it did before; not a character of logic moved. It rewrites the comment above the separator strip to name the en dash (which the old comment omitted) and to say out loud that this class is still narrower than cleanField's — cleanField also takes ; — with a pointer to #653 for unifying the separator vocabulary repo-wide.

This is the policy I described over on #840 actually firing: when a review has zero blocking findings, we fix the small stuff ourselves and approve in the same pass rather than bouncing the PR back for a comment tweak. It only works because you left "Allow edits by maintainers" on. If you'd rather review such changes yourself instead of finding them already pushed, say so — I'll switch to leaving them as review notes on your PRs, no friction either way.

One small correction

The PR description says the widened class now matches cleanField's. It doesn't quite — cleanField still accepts ;, so "B.S. ; in Computer Science" parses the field as "in Computer Science". Harmless here and out of scope for #839, which is why it's a note rather than a change request; the comment I pushed records it so the next reader isn't misled.

Non-blocking leftovers

One suggestion and one nit remain in the review (inline at education.ts:543 and education.test.ts:976). Both are genuinely optional — take them or leave a reply saying why not, and either resolves the thread.

Two mechanical notes, neither of which is your homework

• The branch now carries two commits (yours + my comment fix). This repo squashes to one commit per PR, and that happens at merge time — @s-annam handles it. Nothing for you to do; force-pushing to a fork wasn't an option on my side, which is why it's landing this way.
• CI hadn't reported on this branch when I approved. An approval here means the diff is right, not that the pipeline is green — the merge check is still the gate.

Questions, or disagreement with any of the above — tag me. Thanks again.