Keep one-letter skills without corrupting others by Disesfgewu · Pull Request #50 · sysprog21/codetrial

Disesfgewu · 2026-09-16T11:38:45Z

What

unique() dropped any candidate shorter than two characters, meant to drop
stray punctuation left over from a bad split. It also silently dropped
one-letter skills like C and R before a candidate could choose them.

Removing that floor exposed a second bug: clean()'s leading-strip regex
swallowed a bare digit run, so 5G lost its 5 and survived as the
fabricated skill G.

Why

unique() now keeps a token only if it has a letter. Punctuation-only
fragments and bare numbers (24/7, 100%, 2015, 27001, 3.14) are
dropped, whether typed alone or split from a prefix like ISO.
ISO 9001 and 802.11ac are kept.
Bare part numbers such as 6502 and 8051 are dropped too. This is an
accepted loss: 8051 and 2025 have the same shape, so a filter that goes
by shape cannot keep one and drop the other. A named part (MOS 6502,
Z80) is kept.
clean() now strips only a real list marker: a bullet, a run of dashes or
asterisks followed by whitespace, or digits followed by . or ) and then
whitespace or two letters. Digits are matched with \p{Nd}, so a
full-width １. counts like 1..
As a result 5G, 3D, 4K and 9.x Java stay whole, while 1. Must,
1.Must, ** Must and -- Must lose thei

Known limits

The two-letter rule after . is a heuristiits
marker and 3.js experience loses its 3..
A dash or asterisk glued to the text --Mu is,
since stripping it would eat into real content such as -Java.

Testing

node --test tests/browser/document-grounding.test.js: 35 tests pass, with
new coverage for one-letter skills, digit-leded
markers with and without a space, runs of asterisks, version wildcards,
full-width digits, and bare part numbers. `es

Summary by cubic

Fixes grounding extraction so one-letter skills like C and R survive, and 5G is no longer corrupted into G. The old length floor dropped stray punctuation but also erased one-letter skills; removing it exposed clean() treating bare digit runs as list markers.

Changes

clean() now strips only real list markers: bullet glyphs, dash/asterisk runs before whitespace, and digits followed by ./) and then whitespace or two letters. Requirement lines still lose 1.Must and ** Must, while 3D and 9.x Java stay whole.
unique() now keeps a token only if it has a letter, so pure punctuation and bare numbers are dropped — 24/7, 100%, 27001, 3.14, and part numbers like 6502 — while named forms like ISO 9001, 802.11ac, and MOS 6502 survive.

^{Written for commit 0119193. Summary will update on new commits.}

jserv

Indent via make indent and then squash commits without Claude markers.

Disesfgewu · 2026-09-16T15:48:34Z

Indent via make indent and then squash commits without Claude markers.

Sorry about that! I've run make indent, squashed the commits, and pushed the updates. Could you please take another look when you have a moment? Thanks!

jserv · 2026-09-16T15:56:07Z

I've run make indent, squashed the commits, and pushed the updates. Could you please take another look when you have a moment?

You didn't. Check git manual carefully for git rebase.
Don't be overly polite by including "sorry" in replies; it is not particularly useful during collaboration.

Disesfgewu · 2026-09-16T16:05:20Z

Oops. I check that again.

jserv · 2026-09-16T16:09:46Z

Check Git Squash Commits: How to Combine Commits Into One by using git rebase -i carefully.

ColtenOuO

It's recommended to make the commit message title relevant to the actual changes, as this helps others quickly understand the updates when reviewing.

Disesfgewu · 2026-09-16T16:16:14Z

It's recommended to make the commit message title relevant to the actual changes, as this helps others quickly understand the updates when reviewing.

That's very helpful advice. I'll apply this practice to my future commits.

ColtenOuO

As a reminder, once the issue pointed out is fixed, we should add a regression test for it to ensure the bug doesn't come back in future updates.

jserv

Rebase latest main branch and refine commit messages properly.

ColtenOuO · 2026-09-16T19:36:04Z

Also, I don't think it's a good idea to describe the collaboration process in the commit body. It should focus on what was actually changed.

This issue is still present in the current commit message.

ColtenOuO · 2026-09-17T03:11:43Z

By the way, the PR description might need to be updated to reflect the latest changes, as some parts appear to be outdated.

jserv · 2026-09-17T03:22:47Z

I think some of the newly added tests seem unrelated to this PR (e.g., single isolated skill) and might not be necessary.
[...]
Adding unrelated tests without a clear purpose might just introduce unnecessary review overhead.

For small projects like this, it is acceptable to combine diverse changes into a single pull request, provided that each change has a clear purpose and the overall changes are not extensive enough to make the review difficult, allowing reviewers to still walk through them effectively.

jserv

Rebase the current branch onto the upstream default branch and rework the series into functionally minimal commits, folding similar ones and enforcing the project's commit message rules.

ColtenOuO · 2026-09-17T10:25:02Z

Squash the commits touching the same files into one and reword the commit message to reflect the updated state.

jserv

Read https://chris.beams.io/git-commit carefully and enforce the rules.

ColtenOuO

Please remember to resolve the open comments and add regression tests.

ColtenOuO

There are several strange typos in the PR description. Please clean them up.

ColtenOuO

The commit message is a bit contradictory, please fix it.

By the way, the previously mentioned issue with the PR description has not been resolved.

Once the commit message and PR description are fixed, this will be good to go from my side.

ColtenOuO · 2026-09-18T18:33:26Z

 function clean(line) {
-  return line.replace(/^[-*•\d.)\s]+/, "").slice(0, textLimit).trim();
+  return line.trimStart()
+    .replace(/^(?:(?:-+|\*)\s+|\d+[.)]+(?=\s|\p{L})\s*|•\s*)+/u, "")


The numbered-marker regex still uses ASCII-only \d, even though the surrounding filtering is Unicode-aware. JD requirements such as １. Must know Rust retain their markers as well.

It may be worth tracking in a follow-up PR using \p{Nd}+ with regression tests for both resumes and JDs.

So, do I need to check this or just keep this in mind?
I've updated the latest PR description here and the commit message.

jserv · 2026-09-18T23:39:35Z

+  // but not the marker itself for these common copy-paste shapes, so the
+  // digit case relies on "." or ")" following the digits instead: that still
+  // protects "5G" and "3D", which no "." or ")" ever follows.
+  const jd = await parseGroundingFile(txt("1.Must know Rust\n1.) Should know Go\n-- Must know Python"), "jd");


The dashed fixture carries a space, so the "dashed marker with no space" half of the name is never exercised: --Must know Python is left intact, because the dash branch still requires \s+. Use that input and make the branch handle it, or narrow the name to the numbered case this actually covers.

I would narrow the name to the numbered case this actually covers.

jserv · 2026-09-18T23:39:59Z

+// would have to guess at.
 function unique(values, max) {
-  return [...new Set(values.map(clean).filter((value) => value.length >= 2))].slice(0, max);
+  return [...new Set(values.map(clean).filter((value) => /\p{L}/u.test(value)))].slice(0, max);


Every letterless token goes, and on a systems resume that is the part numbers: 6502, 8051, 68000, 802.3 and 99.999% all cleared the old floor and clear nothing here. The rationale above covers years, percentages and split standards but not bare part numbers, which is the one class the audience named in the first test of this change lists without a letter anywhere near them. Worth saying whether that loss is accepted, since the title's bug (a two-character floor hiding C and R) does not require it.

However, tokens like 8051 or 6502 are format-wise indistinguishable from years (e.g., 2015) or arbitrary numbers without deeper contextual analysis.

Since we currently lack the context to tell them apart, treating letterless tokens uniformly is a trade-off we accept for now. I would update the PR rationale to explicitly call out this limitation.

A length floor meant to drop stray punctuation also dropped one-letter skills like C and R. Replace it: a token is kept only if it has a letter, so punctuation, "24/7", "100%" and bare numbers like 2015 or 27001 are dropped, whether typed alone or split from a prefix like ISO. Bare part numbers such as 6502 go too, since they look like a year; a named one like MOS 6502 is kept. Without the floor, the marker strip in clean() turned 5G into G. It now strips only a bullet, a run of dashes or asterisks before whitespace, or digits (full-width included) followed by "." or ")" and then whitespace or two letters. So 5G and 9.x Java survive, while 1.Must and ** Must lose their markers.

This comment was marked as resolved.

Sign in to view

jserv reviewed Sep 16, 2026

View reviewed changes

Comment thread web/document-grounding.js Outdated

jserv reviewed Sep 16, 2026

View reviewed changes

Comment thread web/document-grounding.js Outdated

Disesfgewu requested a review from jserv September 16, 2026 15:31

jserv requested changes Sep 16, 2026

View reviewed changes

Disesfgewu force-pushed the fix-grounding-short-skills branch 2 times, most recently from 1496214 to 2ad5ddb Compare September 16, 2026 15:43

Disesfgewu requested a review from jserv September 16, 2026 15:45

Disesfgewu force-pushed the fix-grounding-short-skills branch from 2ad5ddb to 618f25b Compare September 16, 2026 16:07

This comment was marked as resolved.

Sign in to view

ColtenOuO reviewed Sep 16, 2026

View reviewed changes

Disesfgewu force-pushed the fix-grounding-short-skills branch from 618f25b to 5af2df8 Compare September 16, 2026 16:33

jserv requested changes Sep 16, 2026

View reviewed changes

Disesfgewu force-pushed the fix-grounding-short-skills branch 2 times, most recently from 1f9eec9 to a1f91ad Compare September 16, 2026 16:44

jserv requested a review from ColtenOuO September 16, 2026 16:46

This comment was marked as resolved.

Sign in to view

Disesfgewu force-pushed the fix-grounding-short-skills branch from a1f91ad to c932cb1 Compare September 16, 2026 16:50

ColtenOuO reviewed Sep 16, 2026

View reviewed changes

Comment thread web/document-grounding.js Outdated

Comment thread tests/browser/document-grounding.test.js

This comment was marked as resolved.

Sign in to view

jserv reviewed Sep 17, 2026

View reviewed changes

Comment thread web/document-grounding.js Outdated

jserv reviewed Sep 17, 2026

View reviewed changes

Comment thread web/document-grounding.js Outdated

jserv reviewed Sep 17, 2026

View reviewed changes

Comment thread web/document-grounding.js Outdated

jserv requested changes Sep 17, 2026

View reviewed changes

Disesfgewu force-pushed the fix-grounding-short-skills branch from c272822 to bf7ab09 Compare September 17, 2026 09:59

Disesfgewu force-pushed the fix-grounding-short-skills branch 4 times, most recently from e63a31d to c8f549a Compare September 17, 2026 11:14

jserv requested changes Sep 17, 2026

View reviewed changes

Disesfgewu force-pushed the fix-grounding-short-skills branch from c8f549a to 1149e07 Compare September 17, 2026 12:42

sysprog21 deleted a comment from cubic-dev-ai Bot Sep 17, 2026

This comment was marked as resolved.

Sign in to view

jserv requested a review from ColtenOuO September 17, 2026 13:50

ColtenOuO reviewed Sep 17, 2026

View reviewed changes

Comment thread web/document-grounding.js Outdated

Disesfgewu force-pushed the fix-grounding-short-skills branch from 1149e07 to 500155f Compare September 17, 2026 23:03

Disesfgewu requested a review from ColtenOuO September 18, 2026 00:18

ColtenOuO suggested changes Sep 18, 2026

View reviewed changes

Comment thread web/document-grounding.js Outdated

Comment thread web/document-grounding.js Outdated

Disesfgewu force-pushed the fix-grounding-short-skills branch from 500155f to b9d5fa7 Compare September 18, 2026 15:57

Disesfgewu requested a review from ColtenOuO September 18, 2026 16:14

Disesfgewu force-pushed the fix-grounding-short-skills branch from b9d5fa7 to e5dc8d3 Compare September 18, 2026 16:20

ColtenOuO reviewed Sep 18, 2026

View reviewed changes

jserv reviewed Sep 18, 2026

View reviewed changes

Comment thread web/document-grounding.js Outdated

jserv reviewed Sep 18, 2026

View reviewed changes

Comment thread web/document-grounding.js Outdated

jserv reviewed Sep 18, 2026

View reviewed changes

Disesfgewu force-pushed the fix-grounding-short-skills branch from e5dc8d3 to 0119193 Compare September 19, 2026 02:46

Disesfgewu requested review from ColtenOuO and jserv September 19, 2026 02:52

Conversation

Disesfgewu commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Known limits

Testing

Summary by cubic

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Uh oh!

Disesfgewu commented Sep 16, 2026

Uh oh!

jserv commented Sep 16, 2026

Uh oh!

Disesfgewu commented Sep 16, 2026

Uh oh!

jserv commented Sep 16, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

ColtenOuO left a comment

Choose a reason for hiding this comment

Uh oh!

Disesfgewu commented Sep 16, 2026

Uh oh!

ColtenOuO left a comment

Choose a reason for hiding this comment

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

ColtenOuO commented Sep 16, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ColtenOuO commented Sep 17, 2026

Uh oh!

jserv commented Sep 17, 2026

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Uh oh!

ColtenOuO commented Sep 17, 2026

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

ColtenOuO left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ColtenOuO left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ColtenOuO left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ColtenOuO Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

Disesfgewu Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Disesfgewu commented Sep 16, 2026 •

edited

Loading

ColtenOuO left a comment •

edited

Loading

Disesfgewu Sep 19, 2026 •

edited

Loading