feat(mcp): stop telling the agent a country is a valid location filter - #181
Open
ArtyETH06 wants to merge 31 commits into
Open
feat(mcp): stop telling the agent a country is a valid location filter#181ArtyETH06 wants to merge 31 commits into
ArtyETH06 wants to merge 31 commits into
Conversation
Whitespace only — 1109 chars of markdown column-alignment padding in an internal decision table. Renders identically; every cell's content is byte-for-byte unchanged. leadbay_pull_followups drops 16948 -> 15643 against the 17000 audit cap, buying the headroom the single-country rule snippet needs at the site where the failure was actually observed. Co-Authored-By: Claude <noreply@anthropic.com>
Each backend serves exactly ONE country, so a country name is never a
location criterion — whole-country intent means omitting the filter. It
does not fail loudly either: the admin-area index excludes country nodes
(product#3885), so the value trigram-matches a same-named town ("France"
to the commune of Francs, "United States" to Statesboro) and silently
fences the search to one village. 3/3 agent sessions in the 2026-08-02
acceptance eval passed one anyway; an FR session burned six variants
inside that fence and gave the user a confident wrong diagnosis.
New _country-names.ts (data only, no imports, no new dependency) carries
the full ISO 3166-1 list with English + French names, and _country-guard.ts
owns the code/message/hint once. Each tool delivers it in its own idiom:
the composites return status "country_level_location" and write nothing,
update_lens_filter throws as it already does, list_locations returns its
empty envelope. Every check is the first statement of execute, so a bad
value costs zero HTTP — including /geo/search and /users/me.
Three exemptions keep it from breaking real prospecting: a sovereign
field so Guadeloupe/Martinique/Reunion/Guyane stay valid on FR and
Puerto Rico/Guam on US; region homonyms (Georgia the state, Jersey); and
no foreign alpha-2 rejection on US, where 26 ISO codes double as state
postal codes (CA, IN, LA, PA, ME...). Sweeps over all 50 states, their
postal codes, and all 13 regions + 101 departements hold that line.
Also fixes two bugs carried by the earlier US/FR-only guard: apostrophes
were deleted before the article strip, so l'Allemagne folded to
lallemagne and the elided French article never matched; and the
combining-diacritical class was written with raw marks, which a reformat
would silently destroy.
Country values passed as resolved numeric ids remain invisible here — that
needs a backend lookup this client lacks, and is tracked in product#3939.
A test records the gap rather than leaving it assumed.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
The 2026-08-02 acceptance eval blamed 3/3 sessions for passing a country
label, but the descriptions were instructing it. followups-map.md.tmpl
said to pass `countries ("France", "United States")` and advertised
`level 2 (country)` as a searchable index level; pull-followups,
adjust-audience, new-lens, tour-plan and the followup_check_in prompt
each legitimized country-level place names. The agents were following
guidance, so this is a contradiction sweep, not a missing rule.
Every one of those clauses is rewritten to enumerate the levels the
argument actually accepts (state / region / departement / county / city)
rather than describing them relative to a level that must never be used
— naming country beside the accepted range is what invited the attempt.
Their "Bavaria" examples went with them: a German region has no meaning
on a US/FR-only product.
New snippet heuristics/single-country-universe.md carries the rule once
into 8 tool descriptions and 3 prompts: what nationwide means, the
measured failure (Francs, Statesboro), the city-and-country tiebreak,
and the recovery step keyed to COUNTRY_LEVEL_LOCATION — re-issue the
same call without the location argument, do not re-spell it.
The new audit has two halves and the second is the load-bearing one: the
rule must be PRESENT in all 11 surfaces, and none of them may still say
a country is a valid geo value. It imports COUNTRY_LEVEL_LOCATION from
core so a rename cannot leave the prose teaching a recovery for an error
that no longer exists. It caught three contradictions left by my own
first rewrite.
Budget: the snippet is ~1.1k x 8 descriptions and pull_followups had 52
chars of headroom. Paid for by the preceding whitespace commit plus two
trims of text the auto-emitted routing block already states; it now sits
at 16581 with 419 spare, and CLAUDE.md documents the measurement recipe
(a naive wc over-reports by 70-360 chars/tool) so the next author checks
before drafting.
Also: new_lens and adjust_audience gain the "companies anywhere in the
US" routing negative + an anti-trigger; WORKFLOWS.md row 39 gains the
country-is-not-a-territory rule and a criterion, plus new row 52 with
its contract; two eval scenarios (over- and under-deliver) under
test/eval/scenarios/country-scope/; CLAUDE.md snippet table and mermaid
node updated, and its stale "16,000 for composites" budget corrected to
the 17,000 the audit actually enforces.
The list_locations level legend (5=region...) is left as-is on purpose:
it contradicts both types.ts:332 and _geo-helpers.ts:115-124, and
picking a winner needs a live /geo/search probe I had no credentials
for. What matters there is documented instead — country nodes are absent
from the index, so a country query can only return a same-named town.
Closes leadbay/product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
product#3951 lists five prompts; I wired three. leadbay_refine_audience and leadbay_setup_team_prospecting were dropped because neither declares a geo ARGUMENT, so following the location params found neither — but absence of a geo param is not absence of geo intent. "Scope my lens to the whole US" lands in refine_audience, and setup_team_prospecting's free-text audience and rep_split can each carry a country. refine_audience gets a PHASE 0: a refine prompt shapes the KIND of company, never WHERE it is, so a place routes to adjust_audience and a country sets no geography at all. setup_team_prospecting gets the rule after its refine step, and its rep_split description now says splitting by country is not a split. This also removes an inconsistency I shipped: the under-deliver eval scenario drives through refine_audience, which had no country rule to test. Both names are added to PROMPTS_WITH_GEO_INTENT so the audit fails if either loses the include again — the audit not covering them is what let the omission through. Refs product#3951 Co-Authored-By: Claude <noreply@anthropic.com>
All four review findings were real. The first two mattered most because each produced a confident wrong answer rather than an error. P1 set_filter bypass. The guard inspected city/city_id only, but both Monitor composites also accept geography as a raw location_ids criterion inside set_filter — a documented path that touches neither argument. That criterion reached POST /monitor/filter, and both composites deliberately catch a failed POST and carry on reading with whatever filter was previously persisted. So a country in set_filter returned a plausible cohort drawn from a STALE filter, presented as the requested one; on the success path it persisted a village-fenced criterion instead. Added detectCountryLocationsInSetFilter and wired it into pull_followups (which covers followups_map) and scan_portfolio_signals, alongside the existing argument checks rather than instead of them. The criteria walk is now defined once and shared with the FilterPayload path — enforcing the rule on one envelope and not the other is what caused this in the first place. P1 prompt fall-through. refine_audience PHASE 0 said "set no geography" and then fell straight into an unconditional refine_prompt call, which would overwrite the qualitative audience prompt and trigger an intelligence recompute — contradicting the no-edit contract in WORKFLOWS.md row 52. PHASE 0 is now a classifying gate that TERMINATES for a country-only or sub-country ask. Same ordering bug fixed in plan_tour_in_city and setup_team_prospecting, where the shared rule sat after an unconditional country-bearing call: the rule now precedes the call and each carries an explicit pre-call gate. P2 input schemas. The tool descriptions were swept but the inputSchema property descriptions were not, and hosts surface those alongside the generated description — so locations still advertised "all admin levels including country" and still used out-of-universe Bavaria. Fixed on locations / location_ids / exclude_locations / city / set_filter.criteria / q across all six geo-accepting tools. P2 output schemas. scan_portfolio_signals and tour_plan returned the new status at runtime but declared neither it nor country_locations, leaving schema-driven hosts with no declared payload for a terminal mode. Both now declare it, matching the four tools already updated. WORKFLOWS row 52 and the under-deliver eval scenario now also forbid refine_prompt and name the set_filter ingress, so the normative contract matches the terminating branch. New test file set-filter-country-guard.test.ts (12 tests): the detector, both composites, the followups_map free ride, the leadIds divergence, and a legitimate sub-country set_filter still applying. Verified red before the fix (5 of 12 failed) and green after. Refs product#3951 Co-Authored-By: Claude <noreply@anthropic.com>
P1, and the sharp irony is that my own guidance reintroduced the failure
class this PR exists to remove. The snippet grouped "country only, or a
supra-national scope" with the home country and said "pass no geo
argument", and hintFor() told a foreign hit to "drop the param". So on a
US workspace, "show leads in France" dropped the filter, re-ran, and
returned ALL US leads as the answer to a question about France — a
confident wrong result, exactly what the guard was written to stop.
Only the HOME country is equivalent to no filter. The recovery is now
three-way, in both the runtime hint and the prose:
home_country -> omit the argument and say it covers the workspace
foreign_country -> unsupported: do NOT re-run unfiltered, report that
the workspace holds its own country only
supranational -> name what the workspace covers, then offer the
whole-workspace view as an explicit choice
Fixing that exposed a second conflation of mine: SUPRANATIONAL_LABELS
held "nationwide", "partout en France", "everywhere" — which mean the
whole of THIS country, i.e. home intent — next to genuine multi-country
scopes like EMEA and APAC. They now live in WHOLE_WORKSPACE_LABELS and
classify as home_country, so the commonest phrasing of all gets the
omit-and-answer recovery. That also removes a contradiction inside the
snippet, whose first paragraph already said "nationwide" means omit. On a
custom backend there is no home country to claim, so those labels fall
back to the conservative report-the-scope treatment.
P2: the us-wide-followups eval accepted "pull_followups (or
followups_map)" in a success criterion while required_calls demanded
pull_followups exactly. preCheckExpectedCalls compares tool names as a
set with no notion of alternatives, so a valid followups_map run would
have failed before the judge ever saw the accepted behaviour. The
criterion now names only the canonical pull, with a comment recording
that followups_map is not expected for this intent (its triggers are
travel/itinerary, absent from a weekly follow-up ask).
Budget: the three-way split cost real chars and pushed pull_followups 30
over the 17000 cap. Paid for by tightening the snippet (it lands in 8
descriptions, so trimming there helps every site) plus the lossless
de-pad of the criterion table I skipped earlier — 443 chars of column
alignment, no content change. Now 16500 with 500 spare.
The audit gained a test pinning the three-way distinction, so a future
edit cannot quietly collapse foreign back into home.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
…nown
Both findings real.
P2 custom backends. LEADBAY_BASE_URL is documented config for staging and
dev, and a custom base URL with no explicit region yields region
"custom" — where homeCountryIso2() is undefined, so every recognized
country fell through to an unconditional foreign_country verdict. On a
custom FR staging backend that told the user "this workspace holds no
France leads", which is simply false. My own test asserted the wrong
behaviour, so the suite agreed with the bug.
The value is still refused — the trigram fall-through is a property of the
admin-area index, not of the region — but the CLAIM is withheld. New kind
country_indeterminate says why the value is unusable and that the
workspace's country is unknown, and its hint offers omission as a
CONDITION ("if you meant this entire workspace") rather than an
instruction, with an explicit don't-present-this-as-an-answer-about-X.
The foreign branch can now assume a known home country, since an unknown
one routes here instead. Fanned out to the kind union, classify(),
messageFor(), hintFor(), the shared snippet and the audit's kind list.
P2 eval entry path. scenario.prompt is injected as the SYSTEM prompt
(scenarios.eval.ts buildSystemPrompt), so seeding leadbay_refine_audience
handed the agent that prompt's PHASE 0 country gate — the very rule under
test. The scenario could pass on an injected prompt while the ordinary
unprompted ask still wrote a country filter, and WORKFLOWS row 52
declares prompt_name: ~ precisely because that story is unprompted. Now
seeded with leadbay_prospecting_overview, which mentions no geography at
all (zero occurrences of "country", does not include the snippet), so the
rule has to arrive through the TOOL DESCRIPTIONS.
The sibling over-deliver scenario keeps leadbay_followup_check_in — that
IS the real entry point for a follow-up ask — but it now carries a
comment stating plainly that a pass there does not prove the tool
descriptions alone hold the line, since that prompt does carry the rule.
Budget after the fourth case: pull_followups 16796, 204 spare.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
All three findings real, all three mine.
P1 had two halves pulling opposite ways. Matching keys EXACTLY meant the
canonical phrasings a user actually types — "whole US", "the whole US",
"all of France", "across the United States" — matched nothing and sailed
through to /geo/search and the same-named-town fence. And in the other
direction, "partout en France" and "toute la France" sat in
WHOLE_WORKSPACE_LABELS, so on a US workspace they classified as the HOME
country and the guidance recommended answering with US leads — the same
accuracy bug the previous round fixed, reintroduced by my own label list.
Now a NAMED country decides the verdict, even inside a scope phrase.
embeddedCountryKey() peels generic wrappers (whole / all of / across /
entire / throughout / partout en / toute la / -wide) and re-strips the
article, and classification uses the country it finds. Generic phrasings
that leave no country behind ("nationwide", "partout", "everywhere") still
mean this workspace. The peel only fires when a real country remains, so
"Whole Foods" and "across the Bay" are untouched, and exemptions plus the
alpha-2 kill switch now key off the bare label so "all of Georgia" stays
the US state.
P2 custom backends: my country_indeterminate branch was unreachable in
the very configuration it was written for. createClient defaulted region
to "us" and passed it explicitly, and bin.ts sets LEADBAY_BASE_URL with no
LEADBAY_REGION on the documented staging path — so a French staging
backend reported region "us", classified France as foreign, and told the
user it holds no French leads. A supplied baseUrl no longer inherits the
default; the constructor derives it, exactly as setBaseUrl already did.
Known regional URLs still map to us/fr, an explicit pin still wins, and
the no-baseUrl default is unchanged.
P2 tour recovery: for city "France", tour_plan forwarded the shared
home-country hint — omit the geo argument and treat the result as
workspace-wide. tour_plan accepts no city and then returns arbitrary
nationwide leads, which is not an itinerary, and the prompt contract
requires asking which city the user is visiting. tour_plan now overrides
the hint with a tour-specific continuation while keeping the shared code,
message and country_locations, and its outputSchema and prompt say the
same thing.
New country-scope-phrases.test.ts (36 tests) covers the wrapper peel, both
directions of the naming rule, the false-positive guards, createClient's
region derivation including the indeterminate reachability, and the tour
override.
Two of my assertions in this round were initially too crude — forbidding
the substring "omit" and the substring "serves" where the real invariants
are "must not INSTRUCT omission" and "must not assert a specific country".
Worth noting because the same over-crude shape is what let earlier bugs
pass: assertions must pin the invariant, not the wording.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
All three findings real, and all three the same omission: I split
home-vs-foreign in the runtime guard and in the shared snippet, then left
the three prompt gates I had written myself still treating any country as
the home country. Each gate decides whether to stop, so each needs the
distinction in its own words.
setup_team_prospecting told the agent to drop a whole-country clause
whatever country it named, so "plumbers across France" on a US workspace
would create and promote a US lens plus campaigns and present them as the
answer. It also still interpolated the raw {{arg:audience}} into the
refine_prompt call, so the country label reached the lens even when the
agent had "dropped" it in prose. Now: home country -> drop that clause and
continue; different country or supra-national -> stop, create nothing; and
the call passes the SANITIZED text, not the raw argument.
top_accounts_to_activate said any country territory means "scope NOTHING,
the plan covers the whole workspace" — so a France territory produced a
plan of US accounts under my heading. Only the home country maps to an
unfiltered plan now; a foreign or supra-national territory is reported as
unfillable from this workspace.
refine_audience had a combined "whole-country or supra-national" branch
that told the user there is nothing to set. Avoiding the write was right,
but the claim is false for a foreign country: the ask is UNSUPPORTED, not
already-satisfied. Three branches now, with the foreign one explicitly
forbidden from offering an unfiltered view as the answer.
Because this is the fourth round of the same class — a rule split in one
surface and left conflated in a sibling — the audit now enforces it
mechanically rather than trusting a sweep. PROMPTS_WITH_COUNTRY_GATE
asserts each gating prompt names a DIFFERENT country separately, and
rejects the three conflating constructions that actually shipped. Verified
by reverting top_accounts to its old wording: the audit fails, and it
fails on that prompt specifically.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
…or pull Four findings, all real. P1 exclusion polarity. Every recovery in this guard was written for an INCLUDE, and each one inverts on an exclude axis. Excluding the home country was answered with "omit the argument and the result covers the whole workspace" — the exact opposite of the request, since omitting the exclusion returns every company the user asked to remove. And excluding a FOREIGN country was reported as unsupported when it is simply a no-op: nothing here is in that country. CountryHit now carries an axis, location_ids criteria read is_excluded, exclude_locations passes axis:"exclude", and hintFor answers the exclude case first with per-kind wording: home -> would empty the workspace AND dropping it inverts the ask; foreign -> harmless no-op, drop it and say the result is unaffected; supranational -> dropping it would instead include everything; custom -> may cut everything or nothing, do not guess. P1 tour description. The shared snippet's home-country line says omit the geo argument, which is right for a Monitor pull and wrong here: tour_plan accepts a missing city and returns arbitrary nationwide leads, not an itinerary. The runtime hint and the prompt already overrode it, but the TOOL DESCRIPTION did not — and direct routing reads that first, so an agent could omit the argument before the guard ever ran. The template now carries a tour-specific override next to the include, and the inputSchema city and outputSchema status descriptions say ask-for-a-city instead of repeating the omit rule. P2 region normalization, a regression from my own last change. Now that a supplied baseUrl no longer inherits "us", the constructor derives the region — but it compared the UNNORMALIZED url, so LEADBAY_BASE_URL=https://api-fr.leadbay.app/ (an ordinary spelling) became "custom" and downgraded a correct home/foreign verdict to country_indeterminate. Extracted regionFromBaseUrl, which strips trailing slashes on both sides; setBaseUrl had the same raw comparison and now shares it. P2 schema validity. The tour rejection returned discover_filter_note: null against a declared type "string", so a client validating structuredContent could reject the whole response and hide the recovery hint it exists to deliver. The happy path always returns a string, so the branch now returns one too rather than loosening the schema. New country-exclude-polarity.test.ts (21 tests). It also corrects an assertion in lens-country-guard.test.ts that had DEMANDED the reversed hint ("OMIT exclude_locations") — my test had encoded the polarity bug, so the suite endorsed it. That is the third time a test of mine locked in the defect it covered; the assertions that hold are the ones stating an invariant, not the ones pinning current wording. Refs product#3951 Co-Authored-By: Claude <noreply@anthropic.com>
…den it Four exclusion/territory paths still gave reversed or contradictory recovery guidance. single-country-universe.md — the canonical heuristic said "omit the geo argument" unconditionally. On `exclude_locations: ["France"]` over an FR workspace that instructs the agent to return every company the user asked to remove. The snippet now splits on `country_locations[].axis`: the omit recovery is scoped to `axis: "include"`, and `axis: "exclude"` gets its own paragraph — excluding the home country empties the workspace, excluding any other country is a no-op, and neither is fixed by dropping the argument. _country-guard.ts, echoed rows — a round-tripped `update_lens_filter` payload carries an excluded country as a numeric criterion id, with the name visible only in `locations.results[].name`. Those rows defaulted to `axis: "include"` and lost the criterion's `is_excluded: true`, so the hint told the caller to omit it. Criterion polarity is now harvested into an id→axis map (exclude wins a conflict) and echoed rows inherit it. _country-guard.ts, message — a direct home-country exclusion said filtering by France "removes nothing" while the hint said the exclusion would empty the workspace. Both fields ship together, so the response contradicted itself. `messageFor` now branches on `hit.axis` the way the hint already did, for home and foreign alike. _country-guard.ts, dependent territories — on a custom endpoint `home` is undefined, so the strict `entry.sovereign === home` test exempted nothing and rejected Martinique on an FR staging backend and Puerto Rico on a US one as `country_indeterminate`. With no known home country the guard now takes the permissive branch, matching the choice `exemptKeysFor()` already makes for custom. Refs product#3951 Co-Authored-By: Claude <noreply@anthropic.com>
The tour is the one geo tool whose country recovery is NOT "omit the
argument": tour_plan accepts a missing `city` and answers with unfiltered
Monitor leads plus arbitrary Discover leads — a nationwide list presented
as an itinerary. The body said so, in a "tour-specific override" paragraph
mid-description. Its two EARLIEST-read surfaces said the opposite.
tour-plan.md.tmpl `prefer_when` ended "a whole-country ask means NO geo
filter". Routing frontmatter lands in the first ~500 chars, the chunk every
truncating host loads, so the contradicting instruction was strictly more
visible than the correction. It now says: do NOT omit `city`, a city-less
tour is arbitrary nationwide leads, ask which city or region (220/240).
leadbay_plan_tour_in_city.md.tmpl's `city` argument repeated it verbatim
("a whole-country ask needs no geo filter at all"). Argument descriptions
are surfaced on their own in prompts/list, with none of the body around
them. Same correction.
The runtime was already right — tour-plan.ts's guard, its inputSchema and
its outputSchema all say the fix is to ask, not to drop the argument. Only
the prose disagreed, so nothing behavioural changes.
Audit: "the tour's EARLY surfaces send the agent to ask, not to omit"
parses the WHEN TO USE block out of the generated description and the
`city` entry out of PROMPT_META, and holds each to the same three bans and
two requirements. Verified it goes red on the exact prefer_when that
shipped. Not a whole-description sweep — the shared snippet's include-axis
"omit the geo argument" is correct everywhere else in this body.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
… of France" Two ways the guard still handed back a wrong-shaped world. MIXED ARRAYS. `locations: ["Paris", "France"]` on FR flagged only France, and the recovery said "OMIT locations entirely". Every guarded tool returns BEFORE resolving anything, so an agent following that hint re-ran with no geo filter at all — losing the city the user asked for and widening the request it was correcting. The rule's own opening tiebreak has always been "keep the city, drop the country"; the runtime contradicted it. CountryHit now carries `kept`: the other values on the same argument that are not country-level, collected in the same pass (non-strings included — a resolved numeric id is not classifiable here but is still a value the caller wants). `hintFor` answers the mixed case before anything else, because every other branch ends in some form of "drop the argument": it names the single value to strip and the ones to keep, and each kind keeps its own sentence about what to TELL the user — the home country must not be described as workspace-wide, the foreign one still has no data here, the custom one still claims nothing, and an exclusion says the surviving exclusions still apply. The snippet's include bullet is now conditional to match. "THE WHOLE OF <COUNTRY>". countryKey renders it "whole of france", and SCOPE_WRAPPERS takes the FIRST wrapper that matches — the bare /^whole\s+/ stripped to "of france", a key matching no country. The guard found nothing, the caller went on to /geo/search, and the same-named-town fence closed exactly as if the guard were not there. /^whole\s+of\s+/ now precedes it, with a comment stating the ordering rule, since which regex wins is not visible at review time. country-mixed-array-recovery.test.ts (25) pins both: the wrapper cases by behaviour rather than by list order, the mixed cases on the hint STRING — that string is the whole product, nothing downstream enforces it. Verified each half goes red when reverted. Refs product#3951 Co-Authored-By: Claude <noreply@anthropic.com>
WRAPPED SUPRA-NATIONAL SCOPES. The scope-wrapper strip existed, but it was
only ever applied while looking for a COUNTRY. So "EU-wide", "all of
Europe" and "across EMEA" — what a rep actually types — matched no exact
supra-national key, produced no hit, reached /geo/search and got fenced to
a same-named town. That is worse than having no guard: the rule promises a
supra-national scope gets its own recovery, and these got none.
embeddedCountryKey is now a thin wrapper over a shared embeddedKey(key,
known), with embeddedSupranationalKey as its sibling. classify consults it
LAST of the three, so a named country inside a wrapper still wins ("all of
France" is France — home on FR, FOREIGN on US) and the generic
whole-workspace phrasings ("the whole country", "everywhere") keep meaning
this workspace. "Whole Foods" and "across the Bay" stay invisible, because
the strip only ever fires when what remains is a known label.
CONTRADICTORY RECOVERIES. The per-kind hints are each correct alone and
mutually exclusive together — only the home country licenses an unfiltered
re-run. `locations: ["France", "Canada"]` on FR concatenated "OMIT
locations entirely" with "Do NOT simply drop locations and re-run", leaving
the agent no safe move on an argument it has to fix in one turn.
Hints are now built per ARGUMENT+AXIS rather than per value. One kind in a
group keeps the existing per-kind text, and every distinct country is still
named. Several kinds go through reconciledHint, which handles the argument
ONCE (the country-level values come off; `kept` decides trim vs drop) and
then lets each kind contribute only what it alone knows: the home half is
answerable and the hint says exactly how far that goes ("answers the France
part of the ask and nothing else"), the foreign half is not, a
supra-national value is not a place, and a custom backend claims nothing.
On the exclude axis it says why each exclusion fails and forbids reporting
a carve-out that never happened. Two DIFFERENT arguments still get two
instructions — that is not a contradiction, and collapsing them would lose
a fix.
country-mixed-kind-recovery.test.ts (25) covers both, asserting on the
joined hint STRING since a contradiction is a property of the whole string.
Reverted each half and watched 17 go red.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
MUTATION RETRIES. The shared recovery was written for the read tools, where it is exactly right: pull_followups re-called without `city` returns every follow-up, which is what a whole-country ask meant. On a lens-writing tool the same sentence inverts. new_lens re-called without `locations` CREATES a lens with no geography; adjust_audience rewrites the active lens's criteria; update_lens_filter replaces the whole filter. All three persist a change carrying no scope at all, to express something the workspace already is — and WORKFLOWS.md's "Country-wide scope" row names those exact three tools in forbidden_calls and requires that NOTHING be written. The guard now takes a GuardIntent, threaded as "write" from those three call sites and defaulted to "read" everywhere else. The split is not read-vs-write in general, it is "would the re-call leave the argument EMPTY": a country beside a real place is a lens worth writing, so `kept.length > 0` keeps the surgical remove-and-re-call recovery untouched. When nothing survives, the hint stops — and still DELIVERS, as the workflow requires: it says the audience already covers the country and offers the axes that do narrow it (sector, size, a city/département/région), rather than stalling. SAME-KIND OFFENDERS. Reconciliation triggered on differing kinds only, so ["Paris", "Canada", "Germany"] emitted `Remove ONLY "Canada"` and `Remove ONLY "Germany"`, each a complete instruction that silently drops the other country — and "ONLY" made that read as deliberate. Grouping now reconciles any argument with more than one offender: one instruction, both values named in it, and the reason phrased for a single kind rather than "mixing kinds". The hint-dedupe went with it: groups are keyed by param+axis and every hint names its param, so no two could ever collide — it was dead code guarded by a test that mis-stated why it passed. Budget: the snippet's new write-stop line costs 156 chars in 8 descriptions and left pull_followups with 11. Paid for by de-padding the criterion-chip table in rendering/pull-followups-table.md — 209 chars of markdown column alignment, renders identically. Headroom back to 220. Four assertions in this PR's own test files encoded the old read-shaped recovery on write tools and were updated to the new invariant. country-write-stop.test.ts (7) exercises the real tools and pairs every case with an empty getHttpRequests(), so "wrote nothing" is a fact and not a claim about wording. Reverted each fix: 3 and 9 red respectively. Refs product#3951 Co-Authored-By: Claude <noreply@anthropic.com>
The write-stop was too wide, in the runtime and in the prompt, and both
ways it discarded the part of the request the user actually cared about.
RUNTIME. `hit.kept` holds siblings from the SAME argument, so
`newLens({sectors: ["Healthcare"], locations: ["France"]})` on FR had
`kept: []` and hit the stop: the guard declared the country the only scope
passed and told the agent to write nothing, throwing away the Healthcare
criterion. Same for adjust_audience, and worse for update_lens_filter,
which REPLACES the whole filter — a sector criterion beside the country was
simply lost.
The three write call sites now compute `otherScope` across the complete
request (sectors / exclude_sectors / sizes / base for new_lens; sectors /
sector_ids / exclude_sectors / sizes for adjust_audience;
filterCarriesOtherScope() over the criteria for update_lens_filter), and
the stop fires only when nothing else survives. When something does, the
recovery drops the geo argument and re-calls ONCE with the rest intact —
and still says the country half cannot be served, so a foreign value is not
quietly written off as applied. A display `name` is deliberately NOT scope:
a lens called "All France" with nothing in it is the write WORKFLOWS.md
forbids.
PROMPT. leadbay_refine_audience's PHASE 0 matched "names this workspace's
own country" and ended the run. "Focus on hospitals running their own IT
nationwide" hit that branch and the hospitals half — the whole point of the
instruction — went with it; so did the Paris in "hospitals in Paris,
France", which the shared rule says to keep. The gate is now strip-then-
classify: remove the country phrase, classify what REMAINS, and stop only
when nothing does. A sub-country place plus a qualitative part now produces
both actions instead of one, and PHASE 1 is explicitly handed the stripped
text so the country cannot reach refine_prompt by the back door.
country-other-scope.test.ts (14) drives the real tools for both halves,
including the ones that must NOT change: a bare `name`, and a country
beside a real place (which keeps the surgical trim, not the re-call). The
audit gains a strip-then-classify pin. Reverted each fix: 5 and 1 red.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
…ckend
EXCLUSION INVERSION. Last round's `otherScope` carve-out reached a case it
should not have. `newLens({sectors: ["Healthcare"], exclude_locations:
["France"]})` on FR asks for an audience with nothing in it; the guard saw a
surviving sector and said "drop the geo argument and re-call with the rest
intact", which persists a lens of French healthcare companies — every
company the user asked to remove, written to the lens and reported as done.
Dropping an INCLUDE widens the result and for the home country is exactly
right. Dropping an EXCLUDE inverts it, and a surviving criterion does not
make the inversion less wrong — it only decides how much of it gets written.
So `excludeBlocksWrite` stops the write for any exclusion that is not
provably a no-op: the home country (excluding it asks for an empty
audience), a supra-national scope (which may well cover this workspace), and
an unknown country on a custom backend. Only a FOREIGN exclusion is safe to
drop and carry on with — there is nothing here to remove. Ordered first in
both hintFor and reconciledHint, ahead of every branch that ends in a
re-call. Reads are untouched: nothing is being written there.
BLIND REGION BRANCHING. leadbay_refine_audience and
leadbay_setup_team_prospecting both branch on "this workspace's own country"
vs "a different country" before any tool call, and neither had the fact.
"French hospitals across France" is a redundant clause on an FR backend and
an unsupported ask on a US one; the language of the request says nothing
about which is connected. team-setup then creates a lens plus per-rep
campaigns off that guess. Both now resolve it first: `_meta.region` rides on
every Leadbay tool result, and `leadbay_account_status` returns it directly
when nothing this session has — read-only, writes nothing. Both spell out
that it cannot be inferred from the country named or the language used, and
what `custom` means: the country is unknown, so claim nothing about it.
country-exclusion-blocks-write.test.ts (9) covers both sides, including the
foreign no-op that must still go through and an INCLUDE beside a sector that
must still re-call. The audit gains a region-resolution pin over both
prompts. Two assertions in this PR's own files matched the older wording and
were updated. Reverted each fix: 6 and 1 red.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
GEOGRAPHY ON ANOTHER ARGUMENT. `CountryHit.kept` only ever sees the argument
its own value arrived on, and the otherScope calculation looked at sectors,
sizes and base — never at the geo fields. So `newLens({locations:
["France"], exclude_locations: ["Paris"]})` on FR reported the country as
the only scope passed and wrote nothing, discarding a perfectly good Paris
exclusion.
`geoScopeSurvives(params, region)` now counts usable values across ALL the
geo arguments, and both new_lens and adjust_audience feed it the same array
they already hand to the detector. A non-string member counts: a resolved
numeric admin-area id is not classifiable here but is unmistakably a place
the caller asked for. `filterCarriesOtherScope` gets the same correction
inside a lens-filter payload — a location criterion naming a real place
beside the country is scope, and that filter is replaced wholesale, so
stopping loses it. The carry wording drops "non-geo", which is no longer
true of what survives.
REP_SPLIT. leadbay_setup_team_prospecting has TWO free-text arguments that
reach the workspace by different routes — `audience` becomes the lens,
`rep_split` becomes the campaigns — and the gate classified only the first.
"Split France to Alice and Germany to Bob" went straight through to PHASE 3,
which partitioned a single-country cohort along an axis that does not exist
here and persisted a campaign per rep. The gate now names both, and adds the
verdict for a SPLIT AXIS specifically: the home country is not a split (one
rep gets everything, the rest nothing), and a foreign or supra-national one
has no cohort to give that rep at all — so stop rather than hand them an
empty campaign, or a slice of home-country leads labelled with another
country's name. PHASE 3 partitions by the sanitized split.
country-geo-scope-survives.test.ts (12) covers the helper directly
(including the region-dependent Georgia case) and through all three write
tools, plus the case that must still stop: a country on every geo argument.
Audit gains a rep_split pin, with wrap-tolerant regexes — the prompt is
hard-wrapped, so a pinned phrase can straddle a newline. Reverted the fix: 1
red.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
A round-tripped lens filter echoes `locations.results` and `locations.parents` as denormalized lookup data — names for the ids the criteria reference, plus the ancestor chain the UI renders as "Limoges ‹ Haute-Vienne ‹ Nouvelle-Aquitaine ‹ France". The country is in that chain almost always, because every French admin area has France as an ancestor. The echoed blocks exist here for one narrow job: a country selected as a bare numeric id is invisible in the criteria, and only the echoed row puts a name on it. Scanning every row rather than the selected ones inverted the guard — a filter legitimately scoped to Île-de-France was rejected because its breadcrumb mentions France, so update_lens_filter refused a valid write outright. Rejecting real geography is worse than the bug this guard was added for. A row now participates only when a location_ids criterion actually selects its id. That is the same correlation the polarity map already needed, so it costs nothing: `polarityById.get(id)` returning undefined now means "not selected" instead of defaulting to include. Which block the name lives in is a backend detail and stays irrelevant — a selected id found only in `parents` is still caught. A country passed by NAME never depended on this path at all; criteriaHits catches it directly. country-echoed-parents.test.ts (9) pins both directions, including the id-only ingress the blocks were added for and the numeric/string id correlation. Two fixtures in this PR's own files encoded the old behaviour — one echoed a row under an id no criterion referenced, the other asserted the include default outright — and were corrected to the real round-trip shape. Reverted the fix: 4 red. Refs product#3951 Co-Authored-By: Claude <noreply@anthropic.com>
…struction
The stop was right; its packaging left live instructions to do the thing it
forbids.
TWO INSTRUCTIONS, ONE REQUEST. Hints are built per argument, so
`{locations: ["France"], exclude_locations: ["France"], sectors: [...]}`
emitted "drop `locations` and re-call ONCE with the rest of the request
intact" and then "write nothing" — and an agent acting on the first has
already persisted the inversion. The same contradiction fitted inside one
argument, where the surgical "remove these and re-call" was prepended to the
STOP: on `exclude_locations: ["France", "EU", "Paris"]` that first half
licenses a Paris-only exclusion that keeps every French company the user
asked to remove.
The envelope now reconciles the write globally, before any per-argument hint
exists, and the text it returns carries no re-call directive at all — the
group-level blocked branches in hintFor and reconciledHint are gone rather
than fixed, because a second instruction beside a STOP is the defect. It
names every blocker and the distinct reason for each, says the rest of the
request cannot be written either (the point an agent is most likely to talk
itself out of — the sectors are fine, so why not write those?), and notes
any remaining country-level values as something to strip whenever a
corrected call is finally made, not as an alternative action.
THE SNIPPET SAID LESS. Its write rule was "if the country was the only
scope: write nothing", which reads as permission the moment anything else is
in the request — and that path never reaches the runtime guard, because the
offending argument is gone before the call is made. Prose is the only thing
in front of it. It now covers ANY non-`foreign_country` exclude hit however
much else came with it, and forbids the re-call rather than the argument.
Costs 134 chars in 8 descriptions; pull_followups keeps 86.
country-blocked-write-single-instruction.test.ts (9) sweeps every re-call
phrasing in the module against the blocked text, and pins the three that
must still carry one: a foreign exclusion, an include-only write, and every
read. Reverted each fix: 5 and 1 red.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
…t scope FOREIGN ROUTING. new_lens and adjust_audience both carried the anti-trigger "companies anywhere in the <country> / nationwide → leadbay_pull_leads", unconditionally. On an FR workspace "companies anywhere in the US" therefore came back as French leads presented as the answer to a US question — the confidently-wrong-result failure this whole rule exists to prevent, produced by the routing hint itself, and in the first ~500 chars every truncating host reads. The shared contract is explicit that a foreign country is unsupported, not unfiltered. The phrase now scopes the route to "this workspace's OWN country / nationwide" and carries the negation inline: a foreign country is unsupported, not unfiltered — call nothing. It is also moved to the head of each anti_triggers list so it lands at char 477 / 517 rather than ~800. A first draft spelled the whole recovery out at 205 chars and pushed the negative-example block past the routing audit's 1500-char window — the audit caught it, and the shorter form is what fits: early visibility is a fixed budget, not free. NAMED IDS ARE NOT SCOPE. An opaque id counts as surviving scope everywhere else in this module, because nothing can tell "416102" from a country (product#3939). That limit does not apply when the same payload echoes a name for it: detectCountryLocationsInFilter already identifies the country that way, but filterCarriesOtherScope still counted the id, so a filter whose ONLY criterion selects a country by id got "remove the country and re-call with the remainder" — where the remainder is nothing, and the corrected call replaces the lens with an empty filter. WORKFLOWS.md requires writing nothing. echoedCountryIds() now discounts exactly those ids, in results and parents alike; an id nothing names still counts, unchanged. Five tests added to country-echoed-parents.test.ts (14 total) and a routing pin per tool, including that the correction lands inside the first 600 chars. Reverted each fix: 3 and 1 red. Refs product#3951 Co-Authored-By: Claude <noreply@anthropic.com>
First live eval of this PR, on an FR staging tenant. It found something 16
review rounds and a clean Codex pass did not, because it is not visible in
the code: the guard only fires on a bad ARGUMENT, and this was a bad BELIEF.
Run 1 passed every assertion 5/5 — no country value sent anywhere, no
forbidden write — and still answered "Your workspace is a US workspace:
every lead in it is a US company" on an FR backend. It inferred the country
from my own phrase "the whole US". Nothing corrected it, because nothing
wrong was ever sent. My success criteria were written region-blind, so the
judge scored it perfect: the agent DID say the workspace serves one country.
It just named the wrong one.
Run 1 also captured that belief to agent memory as `preferred_region`,
`user_stated`, confidence 9. Run 2 read it back — alongside `region:"fr"` in
the same `_meta` payload — and believed the memory over the fact. One
session's hallucination becomes the next session's recalled truth, and a
recalled claim outranks a present one. That is why no_fabrication fell 5 → 4
on the second run: the fabrication was born in run 1 and caught in run 2.
Three fixes, at three different costs:
- agent-memory-capture: never capture which country the workspace serves. It
is a backend fact, not a taste signal, and a wrong one does not fade — it
is replayed as remembered fact. Sub-country territory preferences stay
fine. This is where the false datum is BORN, and that description has
~15.9k chars of headroom, so the long explanation lives there.
- The shared snippet takes 218 chars: never infer the country from the
user's wording, read `_meta.region`, and it outranks any recalled memory.
Paid for by two restatement trims in pull-followups (the sub-country token
enumeration the geo paragraph above already covers, and a sentence the
auto-emitted trigger list repeats). 24 chars of headroom left.
- Both eval scenarios. `us-wide-followups` was mis-specified — the same
region-blindness I was fixing elsewhere. "Across the United States" is the
HOME country on a US tenant and FOREIGN on FR, and its criteria assumed
home; on FR the agent correctly refused to answer a US question with
French leads, and the scenario marked it wrong for not "still delivering
candidates" — which would have meant doing exactly what the rule forbids.
The intent is now region-relative. `country-scope-writes-nothing` gains
two criteria a region-blind judge cannot pass: do not state which country
the workspace serves without having read it, and do not persist one.
Also: `leadbay_followup_check_in` is not a fetchable prompt at all — it and
`leadbay_extend_my_lens` are generated and shipped as SKILL.md, never
registered in the server CATALOG, so prompts/get throws "Unknown prompt".
The scenario now targets a registered prompt, and the audit says plainly
that it proves the text exists, not that prompts/get serves it.
Eval infrastructure: LEADBAY_BASE_URL is forwarded to the spawned server so
a staging tenant can be targeted, with the region still passed explicitly —
without that pin the client derives "custom" and every country classifies as
country_indeterminate, so a staging run would silently exercise a different
branch than the one under test.
Run 3, from a clean memory state: both scenarios pass. The rule-free
scenario now reads the region first ("I'll check your workspace first — the
country scope matters here"), names France, writes nothing, and captures no
memory.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
First run on a US staging tenant. The agent behaved correctly — called
account_status, received `_meta.region: "us"`, answered "your workspace
already covers the entire US", passed no geo argument and wrote nothing —
and the scenario failed it anyway. The criterion was mine and it was
unverifiable: the judge's ledger carries tool INPUTS plus `ok=` and `len=`,
never the output body, so it could only observe that the country named in
the answer matched the country named in my own request.
The asymmetry is the point. On FR the same criterion passed, because
"France" cannot have come from a question that said "the whole US". On US,
a grounded answer and a straight echo of the prompt are word for word the
same sentence. That is exactly the case where "right" and "right for the
right reason" come apart — and it is the failure mode the FR run 1 had, so
a region-blind agent would pass here by luck.
Fixed by making the fact visible rather than by softening the criterion:
`_meta.region` is parsed out of the tool result alongside the existing
envelope check, carried on output_summary, and rendered into the ledger line
the judge reads (`… → ok=true len=970 _meta.region=us`). The existing
`sample` could not serve — it keeps the first 240 chars and `_meta.region`
sits at the end of a large payload. The criterion now names what the ledger
shows, with the reason written into it.
Result on US staging, verified from the raw payloads rather than the scores:
no country value on any geo argument, `pull_followups({count: 20})` with no
geo at all, no lens write, no memory capture, and the answer citing
`_meta.region: "us"` as its source. The mirror of FR, from the same prompts.
Honest tally: four US runs, two fully green. One failure was this criterion;
one was no_fabrication scoring 4 then 5 on identical code, which is judge
and session nondeterminism. The deterministic facts above held in every run.
Refs product#3951
Co-Authored-By: Claude <noreply@anthropic.com>
Seven live runs on staging — three FR, four US. `us-wide-followups-omits-geo` scored no_fabrication 5 five times and 4 twice, on identical code, and neither 4 reproduced when the scenario was re-run alone. So the reason was never captured, and none is claimed: an earlier note blaming an extra leadbay_pull_leads call was a hypothesis the re-run did not confirm, and it is not recorded as fact. What the runs do show is that both tenants have an EMPTY Monitor, so the agent has no follow-ups to report and writes prose around a zero result. That is where the variance lives, and it is a property of the fixture rather than of the rule under test. Seeding the tenant is the real fix; lowering NO_FABRICATION_FLOOR is not, since that floor is shared by every scenario. The scenario now carries the tally and the five facts that did NOT vary in any of the seven runs, read from the raw payloads rather than from the judge: no country value on any geo argument, pull_followups called with no geo at all, no lens write, no country captured to memory, and the country named in the answer traceable to `_meta.region`. Those are the ticket. A single red run here is flake until one of them breaks — which is what the next person to see this red needs to know, and could not have known from the score alone. Refs product#3951 Co-Authored-By: Claude <noreply@anthropic.com>
Both entries were written with the first commit and never revisited across ten rounds of review. The engineering one still said the recovery was "on COUNTRY_LEVEL_LOCATION, re-issue the same call without the location argument" — which is now the single most dangerous thing an agent can do here: on the EXCLUDE axis it persists the inverse of the request, and on a lens-writing tool it performs the mutation WORKFLOWS.md forbids. It also said 3 prompts where there are 5. Rewritten to the behaviour that shipped: the recovery branches on `country_locations[].axis` and `[].kind`, only the home country on the include axis means "omit the argument", writes stop rather than retry when the country was the only scope, a non-foreign exclusion blocks the write whatever else survives, and the country is read from `_meta.region` rather than inferred — never captured to memory. Added the live-eval result on FR and US staging, stated as the facts that held rather than as scores. The product changelog gained the three things a user actually notices and which were missing: nothing is saved to express a scope the workspace already has, a foreign country is told so instead of quietly answering with home leads, and the rest of the request survives when a country rides along. Dates moved 08-17 → 08-19; the work ran two days past the first commit. Refs product#3951 Co-Authored-By: Claude <noreply@anthropic.com>
ArtyETH06
marked this pull request as ready for review
August 19, 2026 12:44
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Contributor
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91a3f92fe5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codex flagged leadbay_pull_followups at 16,976 chars — 24 below the hard cap, down from 52 on main. The PR had spent half the buffer on the exact tool where the production failure was observed, so the next shared-snippet edit anywhere would have turned the audit red. The chars came back from prose that restated what promptforge already auto-emits above it. The body carried "WHEN TO USE: ... what should I follow up on" and "WHEN NOT TO USE: for NEW leads — that's leadbay_pull_leads" while the frontmatter routing block emits both, in the first 600 chars that every truncating host reads. The opener said the same about NEW leads a third time, and the keywords rule is now stated by the shared country snippet. "stale leads" was a real trigger phrase that only existed in that body prose, so it moves into frontmatter triggers rather than disappearing — it now lands in the truncation window instead of below it. 16,976 -> 16,668 (headroom 24 -> 332, vs 52 on main). No rule is stated in fewer places than before; only in fewer copies. CLAUDE.md claimed ~419 headroom for this tool. That was a mid-PR snapshot of mine that was stale before it landed, and it would have told the next author they had 17x the room they actually had. Replaced with measured values plus a note to re-measure rather than trust them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 832bd47ef1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Seven review findings, all in the same shape: the guard STOPS the right calls, but what it tells the agent to do next was wrong in cases the first pass never separated. Wrapped whole-country phrases reached /geo/search. The whole-workspace labels were the only one of three vocabularies matched by exact key — embeddedCountryKey and embeddedSupranationalKey both peel scope wrappers first, this one did not. So "country-wide", "across the country" and "across the whole country" matched nothing and got the same-named-town fence this module exists to prevent. Adds embeddedWholeWorkspaceKey and the bare noun the strip reduces those phrasings to. "nationwide" on a custom backend was called supra-national, whose hint FORBIDS the unfiltered read. But it names no country and every backend covers exactly one, so the unfiltered read is the exact answer — users on the documented LEADBAY_BASE_URL path were told their request had none. It is indeterminate with no country named: omit and answer, name nothing. Four outputSchema blocks told the agent to re-call without the geo argument, full stop. That is right for a home-country include and wrong for the other three cases the runtime hint already distinguishes: an exclude returns the companies the user asked to remove, a foreign country answers a different question, and a write persists a scope that inverts the ask. They now defer to `hint` and say why not to assume. A country selected by ID and revealed only by its echoed name got a recovery aimed at the name. Removing the name leaves the id in the criterion, and a bare id is not classifiable (product#3939) — the "corrected" re-call persisted the country filter past a guard that could no longer see it. Hits now carry selectedId and the recovery names it. refine_audience ordered the strip before the region lookup, so "strip this workspace's OWN country" had to be executed before knowing which country that was. On "French hospitals across France" the agent guesses, and a wrong guess strips a foreign scope and applies the rest to the wrong country. Region first, then strip, then classify. top_accounts_to_activate said only an identity mismatch may stop delivery, then twenty lines later said to stop on a foreign territory. Both applied to the same request and the first is the more emphatic, so an agent could ship a whole-workspace plan under a France heading. The exception is now named in the deliver-first rule itself. prompts/list served hand-copied argument descriptions. Seven had drifted from their templates, including all four that carry the country warning. The single-country audit asserted the GENERATED text and passed while the server shipped the stale copy — an audit proving a guard exists where it is never delivered is worse than none. The catalog now reads PROMPT_META, and a new audit pins the two together for every prompt and argument. It immediately caught setup_team_prospecting's `audience`, which had no country warning on any surface. Three tests changed rather than being added: all three are this PR's own unlanded work, and each asserted a defect above verbatim — the supra- national fallback and the echoed-name param. Their comments were wrong, not just their expectations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b111a52eb1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Six review findings, and they rhyme: the guard treated a country as a
value to strip, when in each of these cases it was the load-bearing half
and the "rest" only made sense as a qualifier on it.
new_lens({sectors:["Healthcare"], locations:["Canada"]}) on FR was told
to drop Canada and write the rest — persisting a real, saved FRENCH
healthcare lens for a Canadian request, which reads in the product
exactly like the ask being honoured. "Healthcare" was an adjective on
"Canada", not a second request. A foreign or supra-national INCLUDE now
blocks the whole write, the way a non-foreign exclusion already did.
exclude: ["France","Paris"] on FR got "Remove ONLY France and re-call
with the rest". Following that excludes Paris only and returns most of
France — a far narrower question than the one asked, with nothing in the
output to show the substitution. Mixed non-foreign exclusions now fail
closed and ask. A foreign exclusion is still a provable no-op and keeps
its surgical recovery.
A set_filter country criterion beside a last_action_date criterion looked
identical to a country on its own, so the hint said "omit it and the
result covers everything" while the date filter still applied — and
"remove the locations" would have left a location_ids criterion holding
nothing, which is invalid rather than neutral. Hits now carry
siblingCriteria and the recovery names them.
pull_followups defaults `filtered` to true, so omitting `city` does not
widen anything: the Monitor view is still read through the filter
persisted earlier, and an old Paris filter would come back described as
the whole workspace — the guard's own advice producing the exact failure
it exists to prevent. The omit recovery now carries `filtered:false`.
Threaded as an opt-in caveat conditioned on the assembled hint actually
saying OMIT, so it never attaches to a foreign or exclude recovery.
scan_portfolio_signals was checked and is not affected: it only sends
filtered=true when it stored the filter itself.
forbidden_calls were checked only after runSessionLive returned. On a
live tenant with write tools enabled that is an assertion about a
mutation it failed to prevent. Armed now at the tool boundary before the
session, the same move the no-spend switch already made for the same
reason. The tools stay LISTED and throw on execute — hiding them would
make every forbidden_calls assertion vacuous. An unknown name in the list
is a hard error, since a typo would otherwise protect nothing and read as
a pass.
top_accounts_to_activate claimed whole-workspace coverage for a
home-country territory while pull_leads keeps applying an active lens the
prompt itself says may be Paris-only. Same confidently-wrong deliverable,
this time in the header rather than the filter. It must read the lens
geography before making the claim.
Five existing test files changed. All five are this PR's own unlanded
work and each asserted a finding above verbatim — one was literally named
"a foreign country beside a real criterion still writes the criterion".
Also fixed a verb regression the tests caught: the blocked-write ask said
"targeted" for an exclusion, which asked the wrong question.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c78f924c7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Five findings, and three are the previous fixes colliding with each
other rather than new ground.
Siblings were attached by criteriaHits only. A country arriving as a bare
ID is found on the other path — through its echoed name — and that path
built its hit without them, so the recovery said "remove id 27925 from
the criterion" and never "and remove the criterion itself". The retry it
authorized carries a location_ids criterion holding nothing, which is
invalid rather than neutral. Siblings are now harvested per-id alongside
polarity and travel with the echoed hit.
pull_followups appended "pass filtered:false" to every omit recovery. Set
beside the sibling note added last round, the two say opposite things:
the sibling note promises the last_action_date criterion survives, and
filtered:false bypasses it while the offered alternative,
set_filter:{criteria:[]}, deletes it — turning a requested date-scoped
read into an all-dates org-wide one. The caveat now branches: with
nothing else requested it still names filtered:false, and with surviving
criteria it says to re-send the corrected filter, which overwrites the
stored one so the stale-filter problem cannot arise in the first place.
new_lens counted a bare `base` id as surviving scope. It is not scope: it
names a lens whose geography nobody has read, and EVERY new lens is a
clone, so the authorized retry inherits it. {name:"Nationwide",
base:<Paris lens>, locations:["France"]} was told to drop the country and
retry, and the retry writes Paris under the name Nationwide. A base alone
no longer licenses the drop, and where a retry IS authorized the hint now
warns the clone inherits the base's geography and points at
lens://<id>/definition. That is broader than the report: base defaults to
the ACTIVE lens, so the trap applies with sectors present too.
top_accounts_to_activate: last round I sent the agent to read the lens
geography from `response.lens` or `active_filters`. Neither can answer
it — pull_leads returns only lens:{id} and active_filters is the Monitor
filter — and the same prompt already says so 30 lines further down. Now
points at lens://<id>/definition, and says to call the scope unverified
rather than national when it cannot be read.
The eval harness defaulted region to "us" and passed it explicitly, so a
staging tenant nobody had identified was asserted to hold United States
companies only and the run reported _meta.region:"us" for it. The pin
stays load-bearing when supplied; unset now means undefined, and the
client derives — known URLs to us/fr, anything else to custom. The judge
ledger carries _meta.region, so the guess would have been read as
evidence the agent got the region right.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 168096a407
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ull_leads
"Omit the geo argument and the result covers the whole workspace" is only
true of a tool that reads leads and whose unfiltered read really is the
whole workspace. Four tools are not that, and each got the sentence
anyway.
adjust_audience MERGES into the lens's existing filter rather than
replacing it, so "the lens then carries no geo criterion" is a claim
about a filter nobody read. On a Paris-scoped lens, {sectors:
["Healthcare"], locations:["France"]} drops the country, merges Healthcare
into the Paris criterion, and returns Paris healthcare described as
nationwide. Where a re-call is authorized the hint now says location
criteria merge, points at lens://<id>/definition, and names the two
sources that look like they should answer it and cannot.
scan_portfolio_signals got the shared omit recovery beside the sibling
note, which contradict each other, and acting on the omission sends a
filter that drops or breaks the caller's other criteria. That is not a
no-op here: a failed POST makes this tool scan UNFILTERED, so a requested
date-scoped scan silently becomes an all-dates one. It gets the
surviving-criteria recovery, but NOT pull_followups' filtered:false half
— this tool sends filtered only when it stored the filter itself, so
dropping the geo argument genuinely does scan unfiltered.
list_locations is a taxonomy lookup with a REQUIRED q. Omitting it fails
validation, and the empty-q branch returns no results rather than
workspace-wide data — so an agent following the shared advice reports
"covers everything" over a lookup that found nothing. Overridden the way
tour_plan overrides it: there is no country id to hand out and nothing to
retry.
setup_team_prospecting said to drop the home-country clause and continue.
With audience:"France" the remainder is empty, and continuing means
refine_prompt({user_prompt:""}) overwriting the refinement prompt, then
create_lens + promote_lens persisting AND ACTIVATING a scopeless lens.
Now stops, names those three calls, and asks for a real criterion. Same
for an emptied rep_split.
And the echoed-ID path still rebuilt its hit from the country name alone,
so `kept` was empty even when the criterion selected a real place:
locations:["27925","99"] said "omit the whole locations property" beside
an id note saying to remove only 27925 — one discards Paris, the other
leaves an invalid criterion. Sibling ids now travel with the hit, labelled
with their echoed names, and a SECOND country is never listed as a
survivor to keep.
That last fix exposed one of mine: the sibling note asserted the criterion
always empties, which would have discarded a surviving place. It is now
conditional — remove the whole criterion only when nothing else selects
through it, otherwise keep it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cc497f031
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Five of six findings share one root, and it is the important one: every
fix to the COUNTRY_LEVEL_LOCATION envelope is reachable ONLY on the
rejection path, while the descriptions tell the agent to recognise a
whole-country ask and omit the geo argument BEFORE calling. That is the
normal path, and the one a well-behaved agent takes. On it the guard
never fires and none of the recovery exists.
"make this healthcare nationwide" -> adjust_audience({sectors:[...]})
carries no country at all, merges into a Paris-scoped lens, and returns
Paris healthcare with nothing anywhere having said otherwise. So the
rules now live on the surface the agent reads before it calls:
- adjust_audience: location criteria MERGE, so omitting `locations` does
not clear what the lens already carries; read lens://<id>/definition.
- new_lens: a lens is a CLONE and `base` defaults to the ACTIVE lens, so
it inherits geography even with sectors present.
- pull_followups: a whole-workspace read also needs `filtered:false` —
or, when other criteria were requested, re-sending them in `set_filter`
instead, which overwrites the stored filter. Same rule added to the
leadbay_followup_check_in prompt, which is the other way in.
- list_locations: the omit-and-claim-coverage recovery does not apply at
all — `q` is required and an empty lookup is not workspace-wide data.
The sixth was a real bug in the predicate I added last round.
`survivingCriteria` rejected every criterion of type `location_ids`, so
{city:"France", set_filter:{criteria:[{location_ids:["99"]}]}} — offender
on `city`, Paris in the filter — reported "nothing else was requested"
and advised `filtered:false`, discarding the Paris scope. Replaced with
setFilterCarriesOtherScope, which decides on VALUES, shared with
scan_portfolio_signals so the two cannot drift.
Also: the eval runner still synthesised region "us" one layer out from
last round's fix, so a custom base URL was still paired with an invented
pin. Region is optional end-to-end now and LEADBAY_REGION is omitted when
unset. And the forbidden-call denylist armed against the FULL catalog
while the server runs includeAdvanced:false — so leadbay_update_lens_filter
counted as armed, satisfied the unmatched check, and left its
forbidden_calls assertion vacuous. It arms from the exposed catalog now,
and an unexposed name is its own named error. Dropped that tool from the
scenario; its guard is covered by a unit test instead.
pull_followups paid for its new rule: a restated resolver sentence and a
next-steps preamble that repeated the table's own mandate came out.
16,636 — 364 headroom, better than the 332 it started the round with.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each Leadbay backend serves exactly ONE country, so a country name is never a location criterion — whole-country intent means omitting the filter. It never failed loudly: the admin-area index excludes country nodes (product#3885), so the value trigram-matched a same-named town (
France→ the commune of Francs,United States→ Statesboro) and silently fenced the search to one village.The root cause was a contradiction, not a missing rule. The 2026-08-02 acceptance eval recorded 3/3 sessions passing a country label — but
followups-map.md.tmpl:47told them to passcountries ("France", "United States")and advertisedlevel 2 (country)as searchable.pull-followups,adjust-audience,new-lens,tour-planand thefollowup_check_inprompt each did something similar. The agents were following the guidance.What changed
"Bavaria"examples went too: a German region on a US/FR-only product.heuristics/single-country-universe.md→ 8 tool descriptions + 5 prompts (every location-accepting surface product#3951 names that exists onmain). Carries the rule, the measured failure, the city-and-country tiebreak, and the recovery step (onCOUNTRY_LEVEL_LOCATION, re-issue the same call without the location argument — do not re-spell it)._country-names.ts(full ISO 3166-1, EN + FR, no new dependency) +_country-guard.ts, which owns the code/message/hint once. Delivered in each tool's own idiom: composites returnstatus: "country_level_location"and write nothing,update_lens_filterthrows (including ondry_run, so a preview can't bless a bad body),list_locationsreturns its empty envelope. Always the first statement ofexecute→ zero HTTP for a bad value, asserted in every test.single-country-rule.test.ts— the rule is present in all 13 surfaces AND none still says a country is valid, so the pre-fix state is unmergeable. ImportsCOUNTRY_LEVEL_LOCATIONfrom core so a rename can't leave the prose teaching a dead recovery. It caught three contradictions my own first rewrite left behind.WORKFLOWS.mdrow 39 + new row 52 with its contract; routing negatives onnew_lens/adjust_audience;CLAUDE.mdsnippet table, mermaid node, and its stale "16,000 for composites" corrected to the 17,000 actually enforced.Two of those prompts —
refine_audienceandsetup_team_prospecting— declare no geo argument, so a sweep that follows location params misses them. They still receive geo intent as free text ("scope my lens to the whole US" lands inrefine_audience), so both are in the snippet list and in the audit.Not breaking real prospecting
A naive country list would refuse legitimate in-universe areas. Three exemptions, each test-enforced:
sovereignfieldGeorgiais a US state a rep names with the bare word;Jerseyis colloquial New JerseyCA,IN,LA,PA,ME,DE…), and_geo-helpers.tsalready mapsla→ Los AngelesSweeps over all 50 state names, all 51 postal codes, and all 13 régions + 101 départements hold that line.
Île-de-Francestays distinct fromFrance; a qualified"China, ME"passes through, which is also the documented user override.Also fixes two bugs in the earlier US/FR-only guard: apostrophes were deleted before the article strip, so
l'Allemagnefolded tolallemagneand the elided French article never matched (harmless with a two-country list, fatal with a full one); and the combining-diacritical class was written with raw marks that a reformat would silently destroy.Budget
The snippet is ~1.1k chars × 8 descriptions, and
pull_followupshad 52 chars of headroom. Paid for by a separate whitespace-only commit (1109 chars of markdown column alignment in the shared NEXT STEPS table) plus two trims of text the auto-emitted routing block already states. Now 16,581 / 17,000 — 419 spare. The cap was not raised.CLAUDE.mdnow documents the measurement recipe, since a naivewcover-reports by 70–360 chars/tool.Review follow-up (all four findings fixed)
set_filterbypass. The guard checkedcity/city_idonly, but both Monitor composites also accept geography as a rawlocation_idscriterion inset_filter. That criterion reachedPOST /monitor/filter, and both composites catch a failed POST and keep reading with the previously persisted filter — so a country returned a plausible cohort from a stale filter rather than an error. Now guarded via a shared criteria walk (one definition for both theset_filterandFilterPayloadenvelopes).refine_audiencePHASE 0 said "set no geography" then fell into an unconditionalrefine_promptcall, which rewrites the audience prompt and triggers a recompute — contradicting WORKFLOWS row 52. PHASE 0 now terminates. Same ordering fixed inplan_tour_in_cityandsetup_team_prospecting, where the rule sat after the call.Bavaria. Swept acrosslocations,location_ids,exclude_locations,city,set_filter.criteria,q.scan_portfolio_signalsandtour_planreturned the new status without declaring it orcountry_locations; both now do.New
set-filter-country-guard.test.ts(12 tests), verified red before the fix (5/12 failing) and green after. Gates re-run: 1,579 tests / 210 files pass, typecheck clean,prompts:checkin sync,pull_followupsstill 16,581/17,000.Gates
pnpm prompts:build→pnpm -r build→pnpm -r test→pnpm -r typecheck→pnpm prompts:check, all green, observed locally: 1,565 tests / 209 files pass, generated files in sync. Core gained 174 tests across 5 new files; mcp gained the audit.Eval coverage
Two scenarios added under
test/eval/scenarios/country-scope/— over-deliver ("leads across the United States" must send no geo argument) and under-deliver ("scope my lens to the whole US" must write nothing,forbidden_calls-enforced). I have not run them live: they are gated behindEVAL=1+ aLEADBAY_TOKEN, and this environment has no credentials. CI protection for this change is the new audit, which does run inpnpm -r test.Not covered, stated plainly: the FR tenant, where the failure was actually observed (needs an FR token); truncating hosts, since the snippet sits mid-body — which is why the
prefer_whenclause was worth its 64 chars in the first-600-char window; and the backend trigram behaviour itself (product#3885 / product#3939).Deliberately left alone
list_locationslevel legend (5=region…) contradicts bothtypes.ts:332and_geo-helpers.ts:115-124. Picking a winner needs a live/geo/searchprobe I couldn't run, and a confidently wrong legend is how this bug class started. What matters is documented instead: country nodes are absent from the index, so a country query can only return a same-named town.1234is a country needs a lookup this client doesn't have. Real fix is server-side (product#3939); a passing test records the gap rather than leaving it assumed.leadbay_find_new_leadsis not onmain— it and its original guard live only on the unmerged feat(mcp): MCP-first lead delivery — find_new_leads, qualify_leads, lead_job_status + seed-craft prompt #168. When that lands it should pick up this shared helper and snippet; its inlinerejectCountryLocationsis then duplicate logic worth deleting.routing-fixtures.tsuntouched: nothing importsROUTING_FIXTURES, so a 91st intent there would be zero signal.Closes https://github.com/leadbay/product/issues/3951