test: make stable sort assertions non-vacuous

[qtjg]

Resolves #857

Summary

The stable-sort test now explicitly verifies that each compared skill is present before checking relative order. This prevents indexOf(...) from returning -1 for both values and making the ordering assertions pass vacuously.

The change covers both tiers exercised by the test:

• canonical skills: Python and Java
• unrecognized skills: Underwater Basket Weaving and Competitive Juggling

Verification

• npm run verify passes, including type-checking, linting, tests, build, and static analysis.
• The unrelated full-suite BroadcastChannel failure was reproduced on untouched upstream main; it is not caused by this test-only change.

[s-annam]

Thanks for this — the four toContain guards are the right shape, and I checked that they do what they claim rather than taking the description's word for it. One blocking item, and it is about the branch's commit history rather than the code.

What I verified

I ran #857's acceptance criteria directly against your branch.

AC 1 — fail-before. I reversed the stable-sort tie-breaker in src/lib/job-search/query-builder.ts:441 (a.index - b.index → b.index - a.index) and re-ran the test. It fails as required:

AssertionError: expected 1 to be less than 0
 ❯ src/lib/job-search/query-builder.test.ts:422:44

Restored the tie-breaker and got 89/89 green across query-builder.test.ts + extract-jd-terms.test.ts. The assertion is live.

AC 3 — the sweep. Not mentioned in the description, so I ran it. Nothing needs repairing, and I am recording the result here so the criterion is discharged on the thread rather than lost:

Site	Shape	Status
query-builder.test.ts:397-401	indexOf → toBeLessThan	already guarded by toBeGreaterThanOrEqual(0)
query-builder.test.ts:456-460	indexOf → toBeLessThan	already guarded by toBeGreaterThanOrEqual(0)
extract-jd-terms.test.ts:392-397	indexOf → toBeLessThan	already guarded by toContain above it

One thing from that sweep worth noting rather than repairing: extract-jd-terms.test.ts:395 wraps its two ordering assertions in if (firstFluffIdx !== -1), so if the cap ever dropped all the fluff those assertions would silently skip instead of failing — the same family of defect #857 is about, in conditional form. I checked empirically (asserted firstFluffIdx is not -1; the test still passed), so the branch is taken today and the assertions are live. Leaving it alone, noting it for the record.

AC 4 — no skills.ts change. Confirmed; one file in the diff.

Scope note, not a criticism. main already probes python/java, not the go/rust that #857 was filed against — that half landed separately. So this PR is narrower than the issue's framing: the ordering assertion was already non-vacuous, and your version and main's catch the reversed-sort mutation identically. What you are adding is protection against it becoming vacuous — if Python ever stopped canonicalizing, main's test would go quietly dead where yours fails loudly. That is exactly AC 2, and it is worth having.

Blocking

The branch is 3 commits, and main merges through a merge queue that derives the squash message from the commit messages. Repo settings, read just now:

squash_merge_commit_title:   COMMIT_OR_PR_TITLE
squash_merge_commit_message: COMMIT_MESSAGES

So merging as-is writes this into main permanently:

* test: make stable sort assertion non-vacuous
* Refactor comments in query-builder tests
* Update query-builder.test.ts

The second bullet describes work that is not in the net diff — 1a3f11a's comment rewrap was reverted on the same branch, so main would carry a permanent record of a refactor that never shipped. The third carries no information.

Your final tree is clean: the net diff really is just the four toContain lines, and the intermediate wobbles — the dropped { in 66291fe, the stray Manus is browsing... / Take over text in 1a3f11a — are both fully undone by 1cf255a. Squashing makes all of that disappear rather than preserving it in main's history:

git fetch https://github.com/offlinecv/OfflineCV.git main
git reset --soft "$(git merge-base FETCH_HEAD HEAD)"
git commit -F- <<'MSG'
test: make stable-sort assertions non-vacuous

`query.skills.indexOf(...)` returns -1 for a skill the query never
produced, and -1 is less than every valid index — so a `toBeLessThan`
over two absent probes passes however `buildJobQuery` orders its
output. Assert each probe is present before comparing, so a probe that
stops resolving fails loudly instead of silently retiring the check.

Resolves #857
MSG
git push --force-with-lease

That commit message is then what lands in main verbatim, so it is worth having it say the thing. CLAUDE.md (Hard rules) and docs/CONTRIBUTING-PROCESS.md → Squash messages (one commit per PR) carry the rule and the derivation. It is blocking only because it is the one thing that cannot be fixed after the merge button.

Secondary

The description does not evidence AC 1 or AC 3. Resolves #857 closes the issue on merge, so an unevidenced criterion ships as done. Both are in fact satisfied — demonstrated above, which discharges them on this thread — but if you want the PR to stand on its own, the fail-before transcript and the sweep table are yours to paste into the description.

Nits (non-blocking)

• "This prevents indexOf(...) from returning -1" — it does not prevent it, it detects it. The guards turn a silent pass into a loud failure, which is the actual value being added.
• The BroadcastChannel full-suite failure you saw did not reproduce here: CI's verify on 1cf255a is green, and a recent full local run in this repo (5864 tests) showed no such failure. It looks local to your environment — node/jsdom version is the usual cause — so it is probably not worth chasing upstream.

Gates

CI on 1cf255a: verify ✅ · fallow ✅. Locally: typecheck ✅ · eslint ✅ · fallow audit --base origin/main ✅ (no issues in 1 changed file) · 89/89 tests green. Fixture-PII, design-system, and style-token gates do not apply — the diff is one test file with no fixtures, components, or styling.

---

Reviewed by: Claude Opus 5 (high)

[s-annam]

One genuine question rather than a request: what are you using to work on these?

I ask because the repo ships its own Claude Code skills in .claude/skills/ — they were already there at the commit you branched from. A couple exist specifically for the thing I flagged: /open-pr and /implement-batch land a change as one commit, and /collapse-pr squashes a branch that has drifted past one. If you're on Claude Code, those would have caught the 3-commit issue before review. If you're using something else, that's completely fine — the manual squash does the same job. (The stray Manus is browsing... line in 1a3f11a made me guess there's an agent somewhere in your loop. No issue, it's gone from your final tree.)

Mostly I'd rather shape feedback around your setup than assume mine.

Your branch itself is current — 2 commits behind main — so nothing to rebase.