Land the work from #18 and #19 on main - #20
Merged
Conversation
Every change here is backed by a check that now passes. Where a check asserted "the API sends what SKILL.md documents" and the doc was the wrong side of that comparison, the check is inverted to assert the corrected behaviour rather than deleted, so a field cannot creep back into the docs without the API sending it. search-charts: - Drop `objectID` from BaseSearchChartHit (returned on 0 of 200 hits) and the now-unused ExplorerType enum along with `explorerType`. - Drop `chartConfigId` from SearchMultiDimViewHit, same reason. - Document `publishedAt` / `updatedAt` (always present) and `containerTitle` (on both non-chart record types). - Add StackedDiscreteBar and Dumbbell to the tab -> URL parameter table, with the values taken from GRAPHER_TAB_CONFIG_OPTIONS in owid-grapher, and add Dumbbell to the GrapherTabName union. fetch-chart-data: - descriptionKey is a markdown bulleted string, not string[]. - Note that useColumnShortNames=true lowercases the first three CSV headers, so the recommended base parameters return `entity,code,year`. owid-catalog: - search() takes no sort_by argument; the documented example raised TypeError. Replace it with ResponseSet.sort_by(), and name the fields it can sort on. - Name each dependent indicator check once, so the skip path and the run path cannot drift apart as checks are added or renamed. The previous hardcoded skip list had already gone stale against the renamed sort_by check. Contract tests: 119 checks, 0 failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-charts
The first real `make triggers` run found owid-catalog at 40% recall: it lost three
of its five positives to search-charts, and the three it lost were precisely its
exclusive differentiators — multi-dimensional garden data, Python workflows, and
semantic indicator search. Two of the three misrouted 3/3, so this was consistent
behaviour rather than sampling noise.
Two causes, both in the descriptions:
- search-charts spent three of its four sentences enumerating topics ("global
population data, energy and electricity, economic data like GDP or poverty,
health data like causes of death..."). A topic catalogue matches any OWID data
question, which made it a general attractor: it fired 35 times across the 40
queries, against owid-catalog's 19.
- owid-catalog described itself as a "Python-native alternative to the HTTP-based
search-charts and fetch-chart-data skills" — subordinate framing — and kept its
actual trigger vocabulary (pandas, notebook, codebook, sex/age dimensions,
semantic search) in the body, which is never read when routing.
So: search-charts now leads with the action, compresses the topic list, and states
what it is not for; owid-catalog names its exclusive territory in concrete terms
and says "prefer it over" rather than "alternative to". Both stay well inside the
spec's 1024-character limit.
Measured, one run per query, both descriptions live at the same time:
owid-catalog recall 40% -> 100%, 3 misroutes -> 0
search-charts recall 100% -> 100%, 0 misroutes -> 0, no regression
All five owid-catalog positives now route correctly, and all three of its
sibling-routing negatives still yield to the right sibling.
Two harness fixes this exposed:
- skills_in_line crashed with "'str' object has no attribute 'get'" on stream
events that carry `message` as a string rather than an object. It was reported
as a runner error rather than a wrong result, so no measurement was corrupted.
- --effort no longer defaults to low. That default was wrong: at low effort the
model answers more queries directly instead of reaching for a skill, which
showed up as five spurious misses and understated real triggering. Effort is a
fidelity knob, not a free cost saving — the help text now says so. Runs inherit
the session's effort unless asked otherwise.
- The default timeout goes 90s -> 180s. A run that fires a skill is killed as soon
as the decision is observed, but a true negative has no decision to observe, so
it only ends when the model finishes answering. At faithful effort two negatives
exceeded 90s and were correctly reported as errors rather than as passes.
Contract tests unchanged at 120 passed, 0 failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Building the trigger evals turned up a failure mode worth writing down for users rather than only for us: lowering an agent's reasoning effort makes it make fewer tool calls, and since skills are invoked through a tool call, a setting turned down for cost can stop skills firing at all. On Claude Code at --effort low, five queries that reliably invoke a skill at normal effort were answered directly with no skill consulted. Nothing about the skills had changed. That is the kind of thing a user experiences as "these skills don't work" and has no way to diagnose, so the FAQ leads with it, alongside the more mundane cause of files sitting in a directory the agent does not read. Deliberately not Claude-centric. The install-path answer covers the 19 agents that share the .agents/skills convention (Codex, Cursor, Gemini CLI among them) and notes that Claude Code is the exception at .claude/skills. On effort, the finding is reported as measured on Claude Code only; Codex's equivalent model_reasoning_effort is named as a plausible first thing to check rather than a known cause, since we have not measured it. Better to be explicit about the limit of the evidence than to generalise a single measurement across 76 agents. Also covers: which skill does what when the wrong one fires, whether installing writes into your repo, per-skill tool prerequisites, and how to tell a skill bug from a data question — including the two API traps the contract tests found (csvType=filtered applying the chart's own entity selection, and a no-match search still returning low-relevance hits). Contributor half answers the questions this repo's own tooling raises: why `make test` can fail with no local change, how to keep `make triggers` cheap without corrupting it, why no SKILL.md references its evals, and why a non-zero `make triggers` is not a failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`—` is a JSON-escaped em dash, and it was my regression: python's json.dumps defaults to ensure_ascii=True, so the round-trip that added a note to owid-catalog/triggers.json rewrote every em dash in that file as an escape. Three occurrences, and it left that file inconsistent with the other three trigger sets, which still had the real character. It matters because these files are hand-authored and hand-reviewed. A trigger query is prose a person has to read to judge whether it is a realistic prompt, and "quick one — what's the URL" is not reviewable prose. Verified the fix is encoding-only: the parsed JSON is identical. Guarded in `make validate` rather than left to memory, since the failure is silent and any future json.dumps without ensure_ascii=False reintroduces it. Confirmed the check fires by rewriting the file the buggy way. Separately, the two rewritten descriptions no longer use em dashes at all. Those strings are routing input and land in every user's context, so plain commas are the better call there. The em dash is pre-existing house style in this repo (README.md had seven before this work, owid-catalog/SKILL.md eight), so prose elsewhere is left alone. Contract tests unchanged at 120 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rewrite two skill descriptions so owid-catalog stops losing to search-charts
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.
Recovery PR. This is #18 and #19's work, already reviewed and approved — nothing new. It just never reached
main.What happened
Both PRs were stacked, and GitHub only auto-retargets a stacked PR when its base branch is deleted:
evaluate-skillsevaluate-skills, which was already on main via #17 — so it added nothingfix-skill-doc-driftfix-skill-doc-drift, a branch no PR targeted at mainevaluate-skillsstill existed when #18 merged, so the retarget never happened and both merges went sideways.mainhas been sitting at #17 the whole time.fix-skill-doc-drifthappens to contain the complete stack, so a single PR recovers everything.What this contains
96d73f7— #18: the seven documentation drifts the contract tests foundsearch-charts: dropobjectID,explorerTypeandchartConfigId(documented as required, returned on 0 of 200+ hits) and the now-unusedExplorerTypeenum; documentpublishedAt/updatedAtandcontainerTitle; addStackedDiscreteBarandDumbbellto the tab→URL table, values taken fromGRAPHER_TAB_CONFIG_OPTIONSin owid-grapherfetch-chart-data:descriptionKeyis a markdown bulleted string, notstring[]; note that the recommendeduseColumnShortNames=truelowercases the first three CSV headersowid-catalog:search()takes nosort_byargument — the documented example raisedTypeError21c5bb9— #19: description surgery so owid-catalog stops losing to search-chartsowid-catalog was at 40% recall, losing three of its five positives — its own exclusive differentiators — to search-charts, two of them 3/3. Measured after: recall 40% → 100%, misroutes 3 → 0, no regression in search-charts.
8d489cc— #19:FAQ.md, led by the reasoning-effort trap (lowering effort makes agents make fewer tool calls, so skills stop firing). Deliberately cross-agent: covers the 19 agents sharing.agents/skills/, and marks the effort finding as measured on Claude Code only.8a3b823— #19: fix escaped unicode in a trigger file (—from ajson.dumpswithoutensure_ascii=False), guarded inmake validate.Verified on this exact commit
After merging
Please delete
evaluate-skills,fix-skill-doc-driftandimprove-skill-triggering. Leaving them is what caused this — a live base branch silently absorbs a stacked merge. Deleting them also means any future stacked PR retargets tomainautomatically.I kept this as one PR rather than re-splitting into two: both halves are already reviewed, and re-stacking is exactly the mechanic that went wrong. Say the word if you'd rather merge the doc fixes and the triggering work separately and I'll split them, both targeting
maindirectly.🤖 Generated with Claude Code