feat(job-search): a local-only filter and narrowing controls beside the results (#809) - #905
feat(job-search): a local-only filter and narrowing controls beside the results (#809)#905rohithgollapalli wants to merge 2 commits into
Conversation
Deploying offlinecv with
|
| Latest commit: |
8b52a38
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c86e0976.offlinecv.pages.dev |
| Branch Preview URL: | https://feat-809-local-only-refine-s.offlinecv.pages.dev |
…he results (#809) Three respondents in the Aug 2026 round reported the same thing: the job search returns postings they did not ask for, and nothing they can reach makes it stop. Both halves of that were true. Reachability. Every narrowing lever the lane has — role chips, exclude terms, target level — lives inside the four-step query form, and `FindJobsPanel` folds that whole form to a one-line summary the moment Search is clicked. At the exact moment a user finally has a result set to react to, all of it is behind an "Edit search" button, in a walk, on a step they have to pick. `JobResultRefineStrip` now renders the three highest-value levers with the results, outside the fold. It is not a second query surface: it edits the same `JobQuery` through the same setter and reuses the same controls, so a chip removed there is gone from the form too. Locality. `location` was a bounded soft axis and nothing else, so a stated city returned the whole feed, reordered. `JobQuery.locationOnly` adds the hard arm: `refineSearchResult` drops postings `locationMatches` rejects, but only because the user armed a visible toggle. This does not reopen #570/#716 — the default behaviour of every axis is unchanged, and the answer to "returns everything" is an explicit lever, never a re-inflated implicit boost. Remote postings always pass, so it narrows to "near me OR anywhere", never to "on-site only". The predicate moved to `location-match.ts` and both readers import it, so the toggle cannot hide a posting whose own card shows a location match. There is no radius and no geocoding: a distance model needs a geocoder, which is a network call this app does not make. Freshers. `SENIORITY_PATTERNS` derives a level from title text, so a candidate with no prior title derives nothing — and the form only reveals `LevelSelect` once a level WAS derived. The gate's condition is exactly the person it excludes. The strip's copy is ungated. Both never-fail-closed, the floor `excludeTerms` and the role filter already apply: a filter that would empty a non-empty set is skipped and flagged for a notice, because a blank panel the user cannot diagnose is worse than an unfiltered one. `locationFilteredOut` states what the filter did remove, and unticking the toggle is the recovery. No egress change: the filter is a local set operation over already-fetched postings, it re-ranks through `refineSearchResult` with no new fetch, and `providers/keywords.ts` stays the sole resume-derived egress helper. `withExcludeTerm`/`withoutExcludeTerm` are extracted to `query-builder.ts` now that two editors write that field — the `undefined`-means-`[]` contract should not be half-remembered in one of them. `check:core` asserts the `.` entry's value-edge closure exactly, so this moves that constant from 27 to 28: `rank.ts` takes a value edge on `location-match.ts` and `rank.ts` is on the barrel. The network-free claim is unaffected — `location-match.ts` is zero-dep and pure, the closure still names no network primitive, `./job-search` is unchanged at 11, and the two closures still share zero modules. The surrounding prose numbers were already stale by more than this change makes them (the emit is 65 modules with 26 unreachable, not 62/24, and 15 of the unreachable sit behind a value edge, not 13); they are corrected to the measured values rather than carried forward with a +1 on a wrong base.
b6588d7 to
65a7bd3
Compare
Samhit21
left a comment
Samhit21
left a comment
There was a problem hiding this comment.
PR Review: a local-only filter and narrowing controls beside the results (#809)
Summary
Strong PR, and unusually well-reasoned about what it deliberately does not do — keeping level and comp floor as soft axes rather than re-inflating #570/#716 is the right call, and saying so in the lane's CLAUDE.md is better than leaving it as tribal knowledge. The reachability diagnosis is the valuable part: "the levers exist but fold away at the exact moment the user has results to react to" explains the feedback better than the feature request did. location-match.ts correctly collapses two private copies into one predicate so the soft axis and the hard filter cannot disagree on screen, the never-fail-closed floor matches the existing excludeTerms/role precedent, and locationFilteredOut vs locationSuppressed is a genuinely useful distinction — "declined to run" and "ran and removed N" are different things a user needs told differently.
I checked two things I expected to be problems and both were fine, so noting them so nobody re-checks: editing location in the strip cannot desync from the fetched snapshot (no provider reads query.location — it is a purely local knob, and it was already in the live re-rank deps), and the . entry's closure growth is machine-enforced safe (check-core-package.mjs asserts disjointness and networkBearingModules: 0, and check:core is green in CI).
Findings
location-match.ts:55— the predicate keeps the wrong city under a hard filter.Portland, ORmatchesPortland, ME;San Jose, CAmatchesSan Jose, Costa Rica; a bareINmatchesAustin, TX. Verified by running it on this branch, not inferred. Tolerable when location only nudged a rating; now it is the filter's whole job. The one finding I would want addressed.JobSearchResults.tsx:214— both notices quote "only jobs near me" while the checkbox they point at reads "Only jobs near {location}" in exactly the situations the notices fire.- Description accuracy (nit). The Files table lists 10 of the 18 changed files, omitting
packages/core/src/index.ts,packages/core/tsconfig.build.json,scripts/check-core-package.mjsandJobQueryEditor.tsx. The first three are where the core package's network-free claim is written down, and this PR moves the.entry's closure from 27 to 28 modules. The claim still holds —networkBearingModulesstays 0, the closures stay disjoint, CI enforces both — but a reviewer reading "## Egress — Unchanged" has no signal to go look. Worth a line in the body given how much care the rest of the description takes.
Two verification notes from the body worth a second pair of eyes, though neither is a code issue: the en-IN locale failures in schema-org*.test.ts reproduce on clean main (so genuinely environmental), and npm run verify never ran end-to-end locally. CI's verify is green on 65a7bd3, which covers it.
Verdict
Action: COMMENT — nothing merge-blocking. Finding 1 is the one I would want resolved before this ships, since a location filter that returns the wrong Portland is the same class of complaint #809 was filed about.
s-annam
left a comment
s-annam
left a comment
There was a problem hiding this comment.
Reviewed against #809's acceptance criteria and the diff first, description last. I also re-ran @Samhit21's two open findings on this branch rather than taking them on trust — both reproduce, and the first one is larger than it looked.
The design is right and worth saying so plainly: soft axes rank, three user-armed hard filters remove, never-fail-closed on all three, one predicate shared by the soft axis and the hard arm so the toggle can't contradict the card. Writing that rule into the lane's CLAUDE.md is worth more than the feature. The reachability diagnosis — "the levers exist but fold away at the exact moment the user has results to react to" — explains the feedback better than the issue did.
The blockers are all one root cause: locationMatches was written as a rating hint and is now the sole predicate behind a remover, and it was promoted without being re-examined for that job. Every finding below is downstream of that.
Blocking
1. location-match.ts:55 — the predicate keeps the wrong place, and drops the right one
Reproduced by calling the real function on 65a7bd3:
| query | posting | result | |
|---|---|---|---|
Portland, OR |
Portland, ME |
true |
wrong keep |
San Jose, CA |
San Jose, Costa Rica |
true |
wrong keep |
Springfield, IL |
Springfield, MA |
true |
wrong keep |
Columbus, OH |
Columbus, GA |
true |
wrong keep |
Kansas City, MO |
Kansas City, KS |
true |
wrong keep |
Austin, TX |
IN |
true |
wrong keep |
Norwich, UK |
OR |
true |
wrong keep |
Boston |
Boston Consulting Group, London |
true |
wrong keep |
New York, NY |
New York City, NY |
false |
wrong drop |
SF Bay Area |
San Francisco, CA |
false |
wrong drop |
Austin, TX |
Austin, TX, USA |
true |
correct |
Boston, MA |
Chicago, IL |
false |
correct |
Three causes:
postingCity === queryCitythrows away the qualifier. Portland OR/ME, Springfield, Columbus, Kansas City MO/KS, San Jose CA/Costa Rica are exactly the pairs a location filter exists to separate.- The bidirectional
includesmatches non-token substrings."austin, tx".includes("in")is why a bareINposting survives an Austin filter;"norwich, uk".includes("or")is whyORdoes. A feed emitting a bare state or country code is enough to trigger it. - The two arms disagree about what they compare. The equality arm uses the pre-comma token; the
includesarms use the whole string including the suffix. That asymmetry is what makesNew York, NYvsNew York City, NYfail — the cities differ by one token and the suffixes are identical, so neither arm can see it. The wrong-drop direction is the quieter half and gets no notice at all: never-fail-closed only fires when the set reaches zero, so a partial wrong-drop is silent.
Why Blocking rather than Secondary. As a bounded soft axis this was tolerable — a wrong location nudged a star rating and nothing vanished. Here it is the sole predicate behind a hard remover, driving a control that renders as "Only jobs near Portland, OR". A user who ticks that and gets Portland, Maine — or ticks "Only jobs near New York, NY" and loses every New York City posting, labelled "too far away" — is filing #809 again verbatim: "it is giving the jobs which I didn't mention". The PR ticks AC 1 ("can reach a result set that excludes non-matching postings") under Closes #809, so merging as-is closes the issue against a filter that fails in both directions, and nothing will reopen it.
Not asking for geocoding — the no-radius reasoning in the docblock is right and should stay. String work covers it: compare the qualifier when both sides have one, and make the fallback token-aware and consistent about which token it compares.
const [postingCity, ...postingRest] = posting.split(",").map((s) => s.trim());
const [queryCity, ...queryRest] = query.split(",").map((s) => s.trim());
if (postingCity === queryCity) {
// Same city name: require the qualifiers not to CONFLICT.
return postingRest.length === 0 || queryRest.length === 0
|| postingRest[0] === queryRest[0];
}
// Fallback: whole-token containment on the CITY tokens, not raw substring
// on the full strings — the asymmetry above is what breaks New York City.
const tokens = (s: string) => new Set(s.split(/\s+/).filter(Boolean));
const postingTokens = tokens(postingCity);
const queryTokens = tokens(queryCity);
return [...queryTokens].every((t) => postingTokens.has(t))
|| [...postingTokens].every((t) => queryTokens.has(t));Shape is yours. The cases I would want pinned in location-match.test.ts: Portland, OR vs Portland, ME, Austin, TX vs IN, and New York, NY vs New York City, NY — none need unusual feed data and all three are what a user would report.
One consequence worth stating because it sizes the fix: the soft axis inherits whatever you do here, since rank.ts now reads the same predicate. That is the shared-predicate design working as intended, and it makes RatingInput.locationMatch more honest too — but rank.ts's existing expectations are the thing to re-run.
2. location-match.ts:78 — a posting whose feed omitted location is hard-dropped and reported as "too far away"
locationMatches returns false for "", which the docblock defends as "no evidence, no credit". That is correct for a rating axis and wrong for a remover, and it was carried over without re-examination. JobPosting.location is documented "" when the feed omits it, and this PR's own search.ts docblock says the keyless feeds "are inconsistent about populating a posting's location at all" — so this is the ordinary case, not an edge.
Reproduced against filterPostingsByLocation directly — one Austin posting plus three with location: "", query Austin, TX:
kept = 1 suppressed = false → locationFilteredOut = 3
which JobSearchResults renders as "3 postings hidden as too far away". The app has no idea where those three are; it says it knows they are far. The never-fail-closed floor cannot catch this — it only triggers on kept.length === 0, and the mixed set is the likely one.
Decide the semantics and state them: either an unstated location passes the hard filter (unknown ≠ far, consistent with remote always passing), or it is dropped but counted and worded separately from "too far away". The first is the smaller change and matches the floor's own stated reasoning; whichever you pick, locationFilteredOut must not silently merge "elsewhere" with "didn't say".
Secondary
3. JobSearchResults.tsx:218 — the suppressed-filter notice states a cause that is usually false. locationSuppressed is set whenever the filter would empty a non-empty set, for any reason, but the copy is "None of these postings say where they are". An Austin candidate whose feed returned only Seattle postings — every one of which states its location — is told the postings didn't say. The PR's own refine.test.ts case (far: "Seattle, WA" + unstated: "") is exactly that mixture. Either widen the copy to cover "nothing matched" as well as "nothing stated", or split the flag. Note this one needs the assertion in JobSearchResults.test.tsx updated with it, which is why I have not attached a suggestion block for it.
4. JobSearchResults.tsx:214 — both notices quote a label that is never on screen. They say untick / turn off "only jobs near me", but localOnlyLabel returns that string only when no location is set — and the filter cannot run without a location, so whenever either notice appears the visible checkbox reads "Only jobs near {city}". The strip's own comment on that helper argues the label must name the place; the copy pointing at the label has to stay in step. Suggestion block attached for the first notice (it keeps the existing test assertions green); the second is folded into finding 3.
5. JobResultRefineStrip.tsx:115 — two LevelSelect instances can be in the DOM at once, and it hardcodes id="level-select-label". StepPanel keeps inactive panels mounted, and the strip renders on phase.kind === "loaded" independently of open. So: pick a level in the strip (which sets query.seniority, un-gating JobQueryEditor's AddPill), then click "Edit search" — two elements now share id="level-select-label", and the second radiogroup's aria-labelledby resolves to the first one's node. Duplicate ids also break getElementById and any future test keyed on it. LevelSelect should mint its id with useId() — a one-line change in a file this PR does not otherwise touch, which is why it is here rather than in the strip.
6. FindJobsPanel.tsx:256 — the mount comment does not match the mount condition. It says the strip is withheld "over the loading skeleton or the error state", but total provider failure is not an error phase: searchJobs never rejects, so degradedProviders.length === providerCount arrives as kind: "loaded" and JobSearchResults renders HardError (:153) beneath a fully interactive "Narrow these results" card. Same for the zero-match ErrorState (:158), whose copy tells the user to "Open Edit search to broaden the query" while narrowing controls sit directly above it. Gate on the hard-error condition or on result.jobs.length > 0 to match the stated intent, or correct the comment.
7. ## Verification claims fallow audit --base origin/main: … complexity 0; it is 1 on this branch. Run here on 65a7bd3:
● High complexity functions (1)
src/components/features/JobSearchResults.tsx
:102 Loaded HIGH
14 cyclomatic 13 cognitive 160 lines 56.3 CRAP
✓ complexity: 1 finding · duplication: 6 clone groups (warn) · 18 changed files
audit gate excluded 4 inherited findings
Diff-attributed, not inherited (the 4 inherited are excluded separately) — the two new notices add 20 lines and two branches to what was already the largest function in the file. Per CLAUDE.md a fallow complexity finding is report-only inside verify and never Blocking on its own, so this is not a merge gate; the finding is that the body states a gate result that does not reproduce. Extracting the notice stack into a sibling alongside the existing roleSuppressed/excludeSuppressed paragraphs would fix both the metric and findings 3–4 in one place.
8. The Files table lists 10 of 18 changed files. Omitted: packages/core/src/index.ts, packages/core/tsconfig.build.json, scripts/check-core-package.mjs, JobQueryEditor.tsx. The first three are where the core package's network-free claim is written down, and this PR moves the . entry's value-edge closure from 27 to 28 modules — under an ## Egress — Unchanged heading, with nothing pointing a reader at it.
The claim still holds — I walked the . entry's value-edge closure independently and got 28, location-match.ts being the addition (it enters via rank.ts, already on that closure), reaching no fetch/WebSocket/XMLHttpRequest/EventSource; ENTRY_CLOSURES still asserts networkBearingModules: 0, the two closures stay disjoint, and check:core is green in CI and in my local verify. A description gap, not a defect — but a reviewer should not have to discover a closure change by diffing the file list.
Nits
9. location-match.ts:23 — the docblock's consequence doesn't round-trip. "so it stays out of the dynamic-import tiers: refine.ts can filter before it has paid for rank.ts" — but refineSearchResult's first statement is await import("./rank.ts") (refine.ts:51), before any filter runs. Zero-dep and pure is true and worth saying; that particular consequence is not what the code does.
10. JobResultRefineStrip.tsx:123 — only the handlers were deduped. withExcludeTerm/withoutExcludeTerm were correctly extracted, but the Location EditableField block and the Exclude ChipListEditor block — including its hint string — are now byte-duplicated against JobQueryEditor.tsx. That hint is the exact class of shared rule the withExcludeTerm docblock argues should have one definition.
11. ## Provenance is retired — docs/CONTRIBUTING-PROCESS.md → Model provenance — retired: "no longer written, updated, or expected, and existing ones need not be removed." Nothing to do on this PR; flagging so it isn't copied into the next body.
Checked and clean
Stated so nobody re-checks:
- AC 2, 3, 4, 5 are met. Level is ungated in the strip and offers
Intern/Junior/Mid— the rungsSENIORITY_PATTERNScannot derive — with a test pinning it.locationFilteredOutcorrectly counts only its own removals (excludeFiltered.length - filtered.length), so the exclude filter's drops aren't double-counted, and it is 0 on every suppressed/inert path.query.locationOnlyjoins the live re-rank dep array; that effect never touchesrawFetchRefand keepsphase.kind === "loaded", so no fetch and the strip never unmounts mid-edit. No new egress: providers never receivequery.location,locationOnlyreaches no adapter, andkeywords.tsis untouched. - Filter ordering is right, and the comment gives the real reason (the never-fail-closed check must read the set the user sees, or the notice names the wrong control).
refineSearchResultis the only constructor ofJobSearchResult—searchJobsdelegates to it — so the two new required fields cannot be half-populated.- House gates. No raw
<button>/modal/dropzone in feature code; no hex, raw palette class, or manualdark:variant;Checkboxis the existing primitive used as designed; the strip is 137 LOC, inside the ~200 rule; the reuse analysis is in the docblock and it genuinely is not a second query surface. No fixtures touched, so the PII gate does not apply.npm run verifygreen locally end-to-end on65a7bd3. - @Samhit21's two "expected to be problems, both fine" notes hold. No provider reads
query.location, and the closure growth is machine-enforced safe.
Verdict
REQUEST_CHANGES — 2 Blocking, 6 Secondary, 3 Nits. Rule applied: ≥1 Blocking → REQUEST_CHANGES; nits never gate.
Findings 1 and 2 are one change to one file plus tests. Nothing was pushed from this review: the blockers are behavioural, and this is a named contributor's branch, so nothing here rewrites it.
Reviewed by: Claude Opus 5 (high)
| if (!posting || !query) return false; | ||
| const postingCity = posting.split(",")[0].trim(); | ||
| const queryCity = query.split(",")[0].trim(); | ||
| return postingCity === queryCity || posting.includes(query) || query.includes(posting); |
There was a problem hiding this comment.
[Blocking] The predicate is unfit as a hard filter, in both directions. Verified by calling it on this branch:
| query | posting | result |
|---|---|---|
Portland, OR |
Portland, ME |
true — wrong keep |
Columbus, OH |
Columbus, GA |
true — wrong keep |
Austin, TX |
IN |
true — wrong keep |
New York, NY |
New York City, NY |
false — wrong drop |
SF Bay Area |
San Francisco, CA |
false — wrong drop |
The equality arm compares the pre-comma token; the two includes arms compare the whole string including the suffix. That asymmetry is why New York City, NY fails — one extra city token, identical suffixes, so neither arm sees a match. And "austin, tx".includes("in") is why a bare IN posting survives an Austin filter.
Tolerable while location only nudged a star rating; this PR makes it the sole predicate behind a remover, driving a control labelled "Only jobs near Portland, OR". Full reasoning and a suggested shape are in the review body — the cases I'd want pinned in location-match.test.ts are Portland, OR vs Portland, ME, Austin, TX vs IN, and New York, NY vs New York City, NY.
There was a problem hiding this comment.
Fixed in 8b52a38, and adopted the shape you sketched with one addition — the qualifier check runs on both arms, so New York, NY vs New York City, CA is false too.
if (!qualifiersAgree(posting[1], query[1])) return false;
if (posting[0] === query[0]) return true;
return containsAllWords(postingWords, queryWords) || containsAllWords(queryWords, postingWords);Every row of your table except two now behaves:
| query | posting | before | now |
|---|---|---|---|
Portland, OR |
Portland, ME |
true | false |
Columbus, OH |
Columbus, GA |
true | false |
Kansas City, MO |
Kansas City, KS |
true | false |
San Jose, CA |
San Jose, Costa Rica |
true | false |
Austin, TX |
IN |
true | false |
Norwich, UK |
OR |
true | false |
New York, NY |
New York City, NY |
false | true |
Austin, TX |
Austin, TX, USA |
true | true |
Boston, MA |
Chicago, IL |
false | false |
All three cases you asked to pin are in location-match.test.ts, plus the other wrong-keep pairs and a both-directions "only one side names a state" case.
The two I did not fix, and why they are in the docblock as known limits rather than in the tests as green:
SF Bay AreavsSan Francisco, CA— needs an alias table. There is no string relation between the two; any rule that matched them would have to encode that those name the same place.BostonvsBoston Consulting Group, London— needs a gazetteer.Bostonis a whole-word prefix ofBoston Consulting Groupin exactly the wayNew Yorkis ofNew York City, so no purely lexical rule separates the city refinement from the company name. I tried two: an allowlist of locality words (city/metro/area) as the only permitted extra tokens, which breaks the existingBerlinvsBerlin Officecase; and "reject when the posting names a qualifier and the query does not", which breaksAustinvsAustin Metro, TX. Both trade a wrong keep for a wrong drop, and the wrong drop is the silent direction — so I stopped rather than ship a rule I could not defend.
Leaving this thread open for your call on whether either is worth an alias table in this PR or a follow-up issue. Both currently fail toward the soft axis, and the never-fail-closed floor stops either from emptying the panel.
Worth flagging the consequence you named: rank.ts inherits all of this, so the soft axis moved with it. rank.test.ts and the full lane suite are green (643 passed / 0 failed) — the rating change is in the honest direction, since RatingInput.locationMatch no longer credits Portland, Maine.
|
@s-annam @Samhit21 — review round addressed in The three findings that had no inline thread: 7 — 8 — Files table. Rewritten: all 18 files, plus the four new ones from this round. The three 11 — Gates on Not collapsed to one commit this round — the fixup is left as a separate commit so the delta is diffable while a thread is still open. It gets collapsed before the queue. |
s-annam
left a comment
s-annam
left a comment
There was a problem hiding this comment.
Requesting changes on two findings in one module — location-match.ts. Everything else here is strong work, and I want to be precise about how narrow the blockers are: the architecture is right, the extraction is right, the never-fail-closed floor is right, the "unknown is not far" split between the two readers of a blank location is genuinely subtle and correctly argued. verify is green end-to-end on macOS, including the check:core closure assertion. The two blockers are both in the matching vocabulary, not in the design.
I measured rather than read, because this PR makes a falsifiable claim about default behaviour. I extracted origin/main's private locationMatches alongside HEAD's and ran both over 481 query×posting pairs of real feed shapes: 465 identical, 13 lost, 3 gained. Three of the 13 are the intended Portland, OR/Portland, ME and bare-IN/OR fixes. Four are same-city false negatives, below.
Blocking
1. qualifiersAgree reads a different qualifier vocabulary as a contradiction, so the same city stops matching itself. Detail and the measured table inline at location-match.ts:55. The four regressions vs main:
| query | posting | main |
HEAD |
|---|---|---|---|
Austin, TX |
Austin, Texas |
match | no |
Austin, TX |
Austin, United States |
match | no |
Hyderabad, India |
Hyderabad, Telangana, India |
match | no |
United States |
Austin, United States |
match | no |
This lands twice. With the toggle on it hides the user's own city and the notice calls it "hidden as too far away". With the toggle off it changes RatingInput.locationMatch, so the star rating and the ranking move for every user who has a location set — which makes "The default behaviour of every axis is byte-identical" false, in the PR body and in the commit message that becomes the squash. That is the one sentence load-bearing for "does not reopen #570 / #716", so it needs either a fix or a restatement.
2. Two of the three always-on keyless providers are remote-only boards whose location is an eligibility region, and the filter reads it as a physical place. Detail inline at location-match.ts:27. KEYLESS_PROVIDERS = [remotive, arbeitnow, jobicy]; Remotive's own adapter docblock says "Remote-only feed", and its location is candidate_required_location — "USA", "USA, Canada", "United States", "Europe". REMOTE_PATTERN matches only remote|worldwide|anywhere|wfh, so those are dropped as too far away.
Simulating a plausible default result set for an Austin candidate: 7 of 11 postings dropped, and 6 of those 7 drops are wrong — 4 remote-eligible postings plus the two Austin, Texas/Austin, United States ones. Only the Berlin posting was a correct drop. The never-fail-closed floor does not rescue it, because the literal-Anywhere postings keep the set non-empty. It also falsifies the control's own on-screen hint, LOCAL_ONLY_HINT: "Remote postings always stay — this hides the ones tied to somewhere else."
I'm deliberately not prescribing the fix — a state/country alias table is real data this module says it doesn't want to carry, and source naming a remote-only board is available but is a different kind of signal. Your call which way it goes; the constraint is only that the code and the on-screen promise agree.
Secondary
3. The fresher rationale is inverted — inline at JobResultRefineStrip.tsx:31, with a suggestion. The form renders {query.seniority || seniorityExpanded ? <LevelSelect/> : <AddPill label="Target level"/>} (JobQueryEditor.tsx:215), so the AddPill appears only when no level was derived — it is shown to the fresher, not hidden from them. "An AddPill that only appears once a level WAS derived, i.e. never for them" and "The gate's condition is exactly the person it excludes" state the opposite of the code. This appears in three shipped docblocks, the commit message, and the PR body.
The strip is still justified — post-search the whole form including that pill is behind "Edit search" and a step pick — so this is a restatement, not a design change.
4. The docblock promises region matching that doesn't exist — inline at location-match.ts:18, with a suggestion. "The posting names my city, my region, or is remote", but only the city segment is ever compared: California vs San Jose, CA → no match, Telangana vs Hyderabad, Telangana, India → no match. A region-only query matches nothing and survives only via the floor, and then only when nothing matched.
Nits
<JobSearchNotices {...result} />spreads all 11JobSearchResultfields into a component typed as a 5-fieldPick. Typechecks (JSX spreads skip excess-property checks) and harmless — flagging only because thePickstates an intent the call site doesn't keep. Inline atJobSearchResults.tsx:184.fallowreports 3 clone groups / 23 lines inrefine.test.ts(plus oneJobSearchResults.test.tsxpair). Report-only insideverify, so non-blocking by repo rule — noting it because it's your file and a helper would erase it.- Double blank line left at
JobQueryEditor.tsx:137-138where the two exclude handlers were removed. Context lines, so no anchor to hang a suggestion on. ## Verificationsaysfallowexcluded "1 inherited finding"; my run reports 3. Base drift, not a defect — worth a glance in case it's pointing at something else.
Gates
OFFLINECV_FULL_TESTS=1 npm run verify → exit 0 on macOS: tsc -b --noEmit, eslint ., check:nul, check:fixtures, check:baselines, check:core, full suite, vite build, fallow. So your two Windows caveats are resolved rather than outstanding — verify does run end-to-end here, and the 4 en-IN locale failures do not reproduce. check:core passes and prints the closure it asserts: . 28 modules/0 fetching, ./job-search 11 modules/7 fetching, disjoint — which does make the Egress — Unchanged claim machine-enforced, as the Files table says. I also confirmed no locationOnly reference anywhere under providers/.
fallow: dead code 0, complexity 0. Extracting the notice stack really did retire the Loaded CRAP finding.
Gates skipped as inapplicable: 3a fixture PII (no fixture binaries touched), 3b design-system (no raw interactive elements; the four new files are all extractions, all well under 200 LOC, and this PR reduces JobQueryEditor by 27 lines and JobSearchResults by 21). 3c's grep hits are all #809/#905 issue refs matching the hex pattern — no real hex or palette classes. 3e applies only to scripts/check-core-package.mjs, where the change is two constants plus prose the gate itself verifies. One commit per PR is not met — the branch is at 2 commits — but that is yours to collapse and I did not touch the branch.
Acceptance criteria (#809)
| AC | Verdict |
|---|---|
| Reach a result set excluding non-matching postings, ≤1 interaction from results | Met — but Blocking 1/2 mean it also excludes matching ones |
| A candidate with no prior title can express their level and see it change the ranking | Met — strip's LevelSelect is ungated and in the live re-rank deps |
| Whatever is hidden is stated as a count and recoverable | Met mechanically; the notice's reason ("too far away") is wrong for the Blocking-2 postings |
Control edits still re-rank through refineSearchResult with no new fetch |
Met — verified query.locationOnly in the dep array, hand-audited both directions |
| No change to what leaves the browser | Met — machine-enforced by check:core, egress tests green |
No AC is unimplemented, so Closes #809 is not itself a problem. Your two corrections to the issue thread both check out: the relevance floor did ship in #569, and refineSearchResult really did hard-filter on only role families and exclude terms before this PR.
Worth saying plainly: the reason I could measure any of this in an afternoon is that location-match.ts is zero-dep and pure, and every non-obvious decision in it already had a docblock explaining why. The blockers are a vocabulary problem in one predicate, not a hole in the thinking.
Reviewed by: Claude Opus 5 (high)
| * Either side may omit it (a feed's "Austin" against a query's "Austin, TX"), | ||
| * but "Portland, OR" and "Portland, ME" are exactly the pair a location filter | ||
| * exists to separate, so two stated qualifiers that differ are a mismatch. */ | ||
| function qualifiersAgree(a: string | undefined, b: string | undefined): boolean { |
There was a problem hiding this comment.
Blocking. qualifiersAgree compares the second segment by exact string equality, but feeds don't share one qualifier vocabulary — a state abbreviation, a spelled-out state, a country abbreviation and a country name all land in segments()[1]. So two spellings of the same place read as a contradiction and the city match is discarded, even when posting[0] === query[0] matched exactly.
Measured against origin/main's predicate:
| query | posting | main |
HEAD |
|---|---|---|---|
Austin, TX |
Austin, Texas |
match | no |
Austin, TX |
Austin, United States |
match | no |
Hyderabad, India |
Hyderabad, Telangana, India |
match | no |
United States |
Austin, United States |
match | no |
The third one is the sharpest: the feed supplies more precision (Telangana) than the query (India) and that extra precision is what loses the match. Greenhouse's location.name is employer-authored free text, so Austin, TX and Austin, Texas are both everyday values.
Two consequences, and the second is the one I'd weigh most:
- Toggle on — the posting is dropped and
JobSearchNoticessays "hidden as too far away" about a job in the user's own city. - Toggle off — this predicate still feeds
RatingInput.locationMatchviarank.ts, so the star rating and the ranking change for every user with a location set. That contradicts "The default behaviour of every axis is byte-identical" in both the PR body and the commit message, which is the sentence carrying "does not reopen [job-search v3] Location boost dominates fit — a flat +10 outranks the entire fit signal on real data #570 / Fitness rating should be absolute, not set-relative #716".
Note the tension the fix has to resolve: you can't simply skip the qualifier check when the cities match, because Portland, OR vs Portland, ME is exactly that shape and is the case the check was added for. Distinguishing "TX ≡ Texas" from "OR ≠ ME" needs an equivalence notion the module deliberately doesn't carry — so this may be a documented-and-tested limit rather than a code fix. Either resolution works for me; what can't stand is the byte-identical claim alongside a 13-pair change to the soft axis.
| * already loading. | ||
| */ | ||
|
|
||
| const REMOTE_PATTERN = /\b(remote|worldwide|anywhere|wfh)\b/i; |
There was a problem hiding this comment.
Blocking. REMOTE_PATTERN recognises four literal words, but the two remote-only boards in KEYLESS_PROVIDERS don't use them — they put an eligibility region in location:
remotive.ts— docblock says "Remote-only feed";location: (job.candidate_required_location ?? "").trim()→"USA","USA, Canada","Europe","Worldwide"jobicy.ts—location: (job.jobGeo ?? "").trim()→"USA","United States","Anywhere"
"USA" is a remote job an Austin candidate is fully eligible for. isRemotePosting("USA") is false, locationMatches("Austin, TX", "USA") is false, so it is dropped — and reported as "hidden as too far away".
Simulated on a plausible default set (the three keyless providers plus a company board), query Austin, TX, toggle on:
DROP remotive:1 "USA" <- remote, eligible
KEPT remotive:2 "Worldwide"
DROP remotive:3 "USA, Canada" <- remote, eligible
KEPT remotive:4 "Anywhere"
DROP jobicy:1 "USA" <- remote, eligible
KEPT jobicy:2 "Anywhere"
DROP jobicy:3 "United States" <- remote, eligible
DROP arbeitnow:1 "Berlin" <- correct
KEPT gh:1 "Austin, TX"
DROP gh:2 "Austin, Texas" <- finding 1
DROP gh:3 "Austin, United States" <- finding 1
kept 4/11, suppressed=false
panel renders: "7 postings hidden as too far away"
6 of the 7 drops are wrong. The floor can't catch it because the literal-Anywhere postings keep the set non-empty, so there is no suppression notice — just a count that misattributes the reason.
It also directly falsifies this file's sibling copy, LOCAL_ONLY_HINT: "Remote postings always stay — this hides the ones tied to somewhere else." A "USA" posting is not tied to somewhere else. Same sentence appears in the PR body as "Remote postings always pass, so the toggle narrows to 'near me OR anywhere', never to 'on-site only'."
JobPosting carries no separate remote flag — its location comment even says "Often 'Remote' / 'Worldwide'" — so location is the only signal, and source (which names the board) is the other thing available. Which lever you reach for is your call.
| * with no prior title has nothing for `SENIORITY_PATTERNS` to derive from, and | ||
| * in the form the level control is hidden behind an `AddPill` that only appears | ||
| * once a level WAS derived, i.e. never for them. Here it is always visible. |
There was a problem hiding this comment.
Secondary. This has the form's gate backwards, and the same inversion is in the commit message (so it lands in main) and the PR body.
JobQueryEditor.tsx:215:
{query.seniority || seniorityExpanded ? (
<LevelSelect value={query.seniority} onChange={setLevel} />
) : (
<AddPill label="Target level" onClick={() => setSeniorityExpanded(true)} />
)}The AddPill is the else branch — it appears precisely when no level was derived, i.e. it is the affordance shown to the fresher, and clicking it reveals the same LevelSelect. So "an AddPill that only appears once a level WAS derived, i.e. never for them" is inverted, and "The gate's condition is exactly the person it excludes" (PR body + commit) describes a gate that excludes nobody.
The strip still earns its place — after Search the entire form including that pill is behind "Edit search" plus a step pick, which is a real discoverability failure and is what your Reachability paragraph already argues. It's the "the form structurally excludes freshers" version of the claim that doesn't hold. Suggestion restates it as the cost it actually is:
| * with no prior title has nothing for `SENIORITY_PATTERNS` to derive from, and | |
| * in the form the level control is hidden behind an `AddPill` that only appears | |
| * once a level WAS derived, i.e. never for them. Here it is always visible. | |
| * with no prior title has nothing for `SENIORITY_PATTERNS` to derive from, so | |
| * the form offers a `+ Target level` pill rather than the control itself — | |
| * reachable, but only after unfolding the form and picking a step. Here it is | |
| * always visible. |
| * geocoding, no distance. "Near me" in the #809 feedback is served by "the | ||
| * posting names my city, my region, or is remote" — which is what a feed's |
There was a problem hiding this comment.
Secondary. "My region" isn't implemented — only segments()[0], the city, is ever compared. The qualifier segments participate solely as a non-contradiction check, never as a match source, so a region-only query matches nothing:
CaliforniavsSan Jose, CA→ no matchCAvsSan Jose, CA→ no matchTelanganavsHyderabad, Telangana, India→ no matchIndiavsBangalore, India→ no match
With the toggle on, such a query is rescued only by the never-fail-closed floor, and only while nothing matches — add one Remote posting to the set and the floor stops firing, leaving the user with the remote posting alone and every in-region job counted as "too far away".
Worth keeping the sentence honest since this file is the lane's stated single source of truth for the predicate:
| * geocoding, no distance. "Near me" in the #809 feedback is served by "the | |
| * posting names my city, my region, or is remote" — which is what a feed's | |
| * geocoding, no distance. "Near me" in the #809 feedback is served by "the | |
| * posting names my city, or is remote" — only the CITY segment is compared, so | |
| * a region-only query ("California", "Telangana") matches nothing — which is | |
| * what a feed's |
| and apply role filtering again. | ||
| </p> | ||
| )} | ||
| <JobSearchNotices {...result} /> |
There was a problem hiding this comment.
Nit. {...result} hands over all 11 fields of JobSearchResult; NoticeFlags is a Pick of 5. It typechecks — JSX spreads skip excess-property checking — and it's harmless at runtime, so this is purely about the Pick stating an intent the call site doesn't keep, and about the next reader of NoticeFlags not being able to trust it as the prop list.
| <JobSearchNotices {...result} /> | |
| <JobSearchNotices | |
| degradedProviders={degradedProviders} | |
| excludeSuppressed={result.excludeSuppressed} | |
| roleSuppressed={result.roleSuppressed} | |
| locationSuppressed={result.locationSuppressed} | |
| locationFilteredOut={result.locationFilteredOut} | |
| /> |
(degradedProviders is already destructured at :112; the other four aren't, hence the result. prefixes.)
3 participants
Closes #809.
What the feedback actually hit
Three respondents in the Aug 2026 round reported the same thing: the search returns postings they did not ask for, and nothing they can reach makes it stop. Both halves were true, for different reasons.
Reachability. Every narrowing lever the lane has — role chips, exclude terms, target level — lives inside the four-step query form, and
FindJobsPanelfolds that whole form to a one-line summary the moment Search is clicked. At the exact moment a user finally has a result set to react to, all of it sits behind an "Edit search" button, in a walk, on a step they have to pick. Nobody found them.Locality.
locationwas a bounded soft axis and nothing else, so a stated city returned the whole feed, reordered. There was no hard location filter anywhere in the tree.Freshers.
SENIORITY_PATTERNSderives a level from title text, so a candidate with no prior title derives nothing — and the form only revealsLevelSelectonce a level was derived. The gate's condition is exactly the person it excludes.What this does
JobResultRefineStriprenders the three highest-value levers with the results, outside the fold. It is not a second query surface: it edits the sameJobQuerythrough the same setter and reuses the same controls (LevelSelect,ChipListEditor,EditableField,Card,Checkbox), so a chip removed there is gone from the form's Narrow step too. No new primitives.JobQuery.locationOnlyadds the hard arm of the location axis.refineSearchResultdrops postingslocationMatchesrejects — but only because the user armed a visible toggle.The strip's
LevelSelectis ungated, which is the fresher's way in.What this deliberately does not do
It does not reopen #570 / #716. The default behaviour of every axis is byte-identical. Location, level and comp floor still rank and drop nothing unless a user explicitly says otherwise. The answer to "the search returns everything" is an explicit lever, not a re-inflated implicit boost.
No radius, no geocoding. The predicate is a string comparison over a feed's free-text
locationfield. A distance model needs a geocoder, which is a network call this app does not get to make. Remote postings always pass, so the toggle narrows to "near me OR anywhere", never to "on-site only".No new remover without a control.
refine.tsnow has three hard filters — role families, exclude terms, local-only — and all three are armed by something the user can see and clear. That rule is now written down in the lane'sCLAUDE.md.One correction to the issue thread
Option 2 already shipped. The relevance floor is not a resumption —
weakMatchThreshold.ts(2.5★) +WeakMatchesSection.tsxlanded in #569 and are live: cutoff, count, and a "Show weak matches (N)" disclosure. The comment on #809 saying "there is no fold, threshold, or show-all inJobSearchResults.tsx" is out of date. Nothing about the floor is declined here; there was nothing left to decide.Separately, the issue body says
refineSearchResulthard-filters on "role families, exclude terms, target level, comp floor, and location". Before this PR it applied only role families and exclude terms; level, comp floor and location were passed torankPostingsas soft axes and removed nothing. That is why acceptance criterion 1 was unreachable and why this PR needed a filter rather than only a surfacing change.Egress
Unchanged. The filter is a local set operation over already-fetched postings; every strip edit re-ranks through
refineSearchResultwith no new fetch (query.locationOnlyjoins the live re-rank dep array inuseJobSearch).providers/keywords.tsstays the sole resume-derived egress helper — untouched.work-authorization-egress.test.tsand the keyword egress tests are green.Never-fail-closed
Both new signals follow the floor
excludeTermsand the #566 role filter already apply. A filter that would reduce a non-empty set to empty is skipped, the input is kept, andlocationSuppressedgoes back for a notice — a blank panel the user cannot diagnose is worse than an unfiltered one.locationFilteredOutstates what the filter did remove, and unticking the toggle is the recovery.Unknown is not far. A posting whose feed omitted
location(the keyless feeds are inconsistent about populating it) passes the hard filter, the same way a remote posting does.locationMatchesreads that blank as a non-match because it is scoring a rating with no evidence to credit; a remover cannot borrow that read without telling the user it hid a posting "as too far away" when the app never saw a location at all. The two readers of the blank differ on purpose, andlocationFilteredOutcounts only postings that stated a location somewhere else.Acceptance criteria
refineSearchResultwith no new fetchFiles
src/lib/job-search/location-match.ts(new)filterPostingsByLocation. Qualifier-aware equality + whole-word containment; an unstated location passes the hard filterrank.tsPortland, ORvsPortland, MEquery-builder.tslocationOnly;withExcludeTerm/withoutExcludeTermextracted now that two editors write that fieldrefine.tssearch.tslocationSuppressed+locationFilteredOutonJobSearchResultJobResultRefineStrip.tsx(new, 118 LOC)QueryFilterFields.tsx(new)LocationField+ExcludeTermsEditor+EXCLUDE_TERMS_HINT— the two fields both editors render, one definition eachJobQueryEditor.tsxLevelSelect.tsxuseId()for the radiogroup label — two instances can be mounted at onceFindJobsPanel.tsxloadedand a non-empty ranked setJobSearchResults.tsxJobSearchNotices.tsx(new)LoadeduseJobSearch.tsquery.locationOnlyin the live re-rank depssrc/lib/job-search/CLAUDE.mdpackages/core/src/index.ts,packages/core/tsconfig.build.json,scripts/check-core-package.mjslocation-match.tsjoins the.entry's value-edge closure (27 → 28 modules) viarank.ts, which was already on it. It reaches nofetch/WebSocket/XMLHttpRequest/EventSource;ENTRY_CLOSURESstill assertsnetworkBearingModules: 0and the two closures stay disjoint, so the Egress — Unchanged claim above is machine-enforced, not merely assertedVerification
tsc -b --noEmit,eslint .,check:nul,check:fixtures,check:baselines— cleanen-INlocale failures noted below. Lane suite 643 passed / 0 failedvite buildgreenfallow audit --base origin/main: dead code 0, complexity 0, duplication 6 clone groups (warn), 1 inherited finding excluded. An earlier revision of this branch did reportcomplexity: 1onJobSearchResults.tsx:102 Loaded(14 cyclomatic, 56.3 CRAP) — that function's notice stack now lives inJobSearchNotices.tsxTwo caveats on the local run, both verified environmental rather than introduced here:
src/lib/jd-extract/schema-org*.test.tsfails 4 tests on this machine (USD 1,50,000vsUSD 150,000) — the box's locale isen-IN, sotoLocaleStringgroups in lakhs. Confirmed identical on a cleanmaincheckout. Untouched by this PR; CI's locale should not hit it.npm run verifycannot run end-to-end here (cmd.exe cannot parse the|| echosubshell) andcheck:coredies ontar -tzf C:\…under git-bash. Each gate was run individually instead, andnpm run build -w @offlinecv/corebuilds green.Provenance
Written with Claude Code (Opus 5).