Generator fixes: which scorer natural-language ranking + store composite resources PK#3476
Generator fixes: which scorer natural-language ranking + store composite resources PK#3476roboulos wants to merge 6 commits into
Conversation
Downstream snappy-os runs a skill-provenance-required lint (Phase E.7) that mandates `created_by: user|agent` frontmatter on every skill it manages. The 9 PP skills are symlinked into snappy-os (state/skills/printing-press*/SKILL.md -> here), so the lint reads these files directly. Phase D.5d (2026-05-10) removed the upstream- managed exemption, expecting the upstream files to carry the field. Adding `created_by: user` to mark these as human-authored (so the snappy curator's auto-archive-stale-agent-skills rule never touches them) and clear the lint at 230/230. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…anguage discovery
The snappy-os SearchSkill index (state/lib/harness/skill-search.ts) reads
this `description:` field as the sole input for token-overlap scoring. The
prior wording leaned on "Generate a ship-ready CLI ... shipcheck loop"
vocabulary that no user types in chat. Live test on 2026-05-11 with the
natural prompt "set up a new integration with httpbin.org" returned zero
points for printing-press across three SearchSkill queries the model tried
("integration setup API connector", "connector configuration webhook",
"httpbin test API"). The model fell through to AskUserQuestion and never
reached InvokeSkill.
Broaden the description with the actual phrases users say. Retest with the
same prompt now ranks printing-press at mvanhorn#2 for "integration setup API
connector" and "wrap api httpbin". Token scoring details in
/Users/robertboulos/projects/snappy-os/state/log/pp-from-chat-live-test-2026-05-11.md.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…l catalog entry) Add internal/googlediscovery package that converts Google Discovery REST descriptions (googleapis.com/discovery/v1/apis) into the internal APISpec the generator consumes. Wire IsDiscovery detection into the generate command's spec-format dispatch alongside the existing OpenAPI and GraphQL branches. Key mapping: Discovery resources.methods -> OpenAPI paths, Discovery parameters (location: path) -> positional Params (sorted first so args[i] indices are stable), Discovery schemas -> TypeDef-compatible body params, Discovery auth.oauth2 -> bearer_token AuthConfig. Handles multi-level nesting via flattenInto: sub-sub-resources are promoted as sibling sub-resources with compound names (settings-delegates) so the generator's single-level SubResource template works without changes. Verified: printing-press generate --spec gmail-discovery.json passes all gates (go mod tidy, govulncheck, go vet, go build, --help, version, doctor). Dry-run produces correct URL: GET .../gmail/v1/users/me/messages. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Every connector generation failed the govulncheck gate with exit 3: the generated module resolved golang.org/x/net at 0.43.0 (GO-2026-5026, reachable via cliutil.ProbeReachable) and go1.26.3 carries two reachable stdlib vulns (GO-2026-5039 net/textproto, GO-2026-5037 crypto/x509), both fixed in go1.26.4. The template now pins x/net v0.55.0 unconditionally (it was gated behind HasHTMLExtraction while cliutil pulls it transitively for every connector) and declares toolchain go1.26.4 so generated modules auto-fetch the patched Go. Probe spec (apis.guru google.com/v3) now generates, validates, and bundles clean.
Five defects fixed in the generated scorer: - hyphenated command leaves were single exact tokens, so 'authenticated' or 'repositories' could never match repos-list-for-authenticated and every command in a group tied on the group token with index order deciding - no prefix normalization (repositories ~ repos, market ~ markets) - description token credit was uncapped, letting token-soup descriptions outrank the precise command path (now saturates at 3) - no possessive aliasing: 'my/mine/me/current' is API-speak for the authenticated caller; authenticated-scoped commands now outrank generic listings for possessive asks - ties broke by index order; now by command specificity (fewest capability sub-tokens), and write-verb commands are penalized for read-shaped asks so a neutral request can never rank a destructive command first Proof (github library rebuilt from this scorer): 'my authenticated user repositories', 'list repositories for authenticated user', and 'repos-list-for-authenticated' all rank user repos-list-for-authenticated first; coingecko 'top coins by market cap' unregressed (coins markets first). Known limit: bare 'my repos' still favors group-token matches - tracked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The v1 single-column PK silently merged rows across resource types on upsert: a markets sync for id 'bitcoin' overwrote the coins row (kept resource_type coins, swapped the data), and a ping health-check row ate a real coin named 'ping'. Fetched rows vanished while sync_summary reported success - the worst kind of data loss, invisible at every layer above the store. - resources DDL: PRIMARY KEY (resource_type, id); upsert conflict target matches - StoreSchemaVersion 2 with an in-place rebuild for v1 databases (create v2 shape, copy with INSERT OR IGNORE, swap, reindex; generic FTS emptied and repopulated on next sync) - generic FTS rowids namespaced by resource type so two types sharing an id cannot cross-delete each other's index entries Found live on the coingecko CLI (markets rows fetched=100 stored=0; fresh-db control proved the write path clean when no collision exists). Generator test suite green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merge Protections🔴 2 of 2 protections blocking · waiting on 👀 reviews, 🤖 CI and 🙋 you
🔴 require-ready-label-and-ciWaiting for
This rule is failing.
🔴 🚦 Auto-queueWaiting for
This rule is failing.When all merge protections are satisfied and these conditions match, this pull request will be queued automatically.
|
Greptile SummaryThis PR fixes generated CLI ranking and store keying while adding Google Discovery support. The main changes are:
Confidence Score: 4/5The Discovery generation path and generic store search can produce incorrect generated CLI behavior.
internal/googlediscovery/parser.go, internal/generator/templates/store.go.tmpl, internal/generator/templates/which.go.tmpl Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| baseURL := doc.RootURL | ||
| if baseURL == "" { | ||
| baseURL = doc.BaseURL | ||
| } | ||
| // Strip trailing slash; generator adds its own separators. | ||
| baseURL = strings.TrimRight(baseURL, "/") |
There was a problem hiding this comment.
Discovery path values are relative to the service basePath, but this converter stores only rootUrl or baseUrl. When a valid method has no flatPath, the generated CLI calls https://www.googleapis.com/{relative-path} instead of the service path such as /gmail/v1/{relative-path}, so requests go to the wrong endpoint and fail.
| var pathParams, queryParams []spec.Param | ||
| for paramName, param := range method.Parameters { | ||
| p := convertParam(paramName, ¶m) | ||
| if param.Location == "path" { | ||
| pathParams = append(pathParams, p) | ||
| } else { | ||
| queryParams = append(queryParams, p) | ||
| } | ||
| } | ||
| sort.Slice(pathParams, func(i, j int) bool { return pathParams[i].Name < pathParams[j].Name }) | ||
| sort.Slice(queryParams, func(i, j int) bool { return queryParams[i].Name < queryParams[j].Name }) |
There was a problem hiding this comment.
Discovery provides parameterOrder for positional path parameters, but this code sorts path params alphabetically. Generated commands bind positional args from this order before replacing {name} placeholders, so any method whose path order differs from alphabetical order sends user arguments to the wrong resource path.
| if len(a) >= 4 && len(b) >= 4 && (strings.HasPrefix(a, b) || strings.HasPrefix(b, a)) { | ||
| return true | ||
| } |
There was a problem hiding this comment.
This treats any two tokens of length four or more as equal when one is a prefix of the other. A query token like user can score an unrelated command token such as usersettings, or list can score listeners, which can rank the wrong command for an agent-facing which lookup.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if score > 0 { | ||
| unmatched := 0 | ||
| for _, ct := range cmdTokens { | ||
| hit := false | ||
| for _, qt := range qTokens { | ||
| if whichTokenMatch(qt, ct) { | ||
| hit = true | ||
| break | ||
| } | ||
| } | ||
| if !hit { | ||
| unmatched++ | ||
| } | ||
| } | ||
| if unmatched > 3 { | ||
| unmatched = 3 | ||
| } | ||
| score -= unmatched | ||
| } |
There was a problem hiding this comment.
The specificity penalty counts every unmatched command sub-token, including parent resource and operation tokens. For a query like tasks, a matching nested command such as projects tasks list-project gains credit for tasks and then loses the same amount to unrelated path tokens, so a valid lookup can be dropped as a zero-score match or lose to a less relevant command.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| var whichWriteVerbs = map[string]bool{ | ||
| "delete": true, "remove": true, "update": true, "create": true, "set": true, | ||
| "add": true, "replace": true, "rename": true, "transfer": true, "merge": true, | ||
| "lock": true, "unlock": true, "star": true, "unstar": true, "follow": true, | ||
| "unfollow": true, "block": true, "unblock": true, "mute": true, "archive": true, | ||
| "unarchive": true, "cancel": true, "send": true, "upload": true, "subscribe": true, | ||
| "unsubscribe": true, "dismiss": true, "approve": true, "decline": true, | ||
| } |
There was a problem hiding this comment.
The read-intent penalty only recognizes exact tokens in this map. Queries such as edit project, post message, or modify label do not set queryWrite, so the intended mutating command is penalized and a read/list command can rank above the action the user asked for.
| var whichWriteVerbs = map[string]bool{ | |
| "delete": true, "remove": true, "update": true, "create": true, "set": true, | |
| "add": true, "replace": true, "rename": true, "transfer": true, "merge": true, | |
| "lock": true, "unlock": true, "star": true, "unstar": true, "follow": true, | |
| "unfollow": true, "block": true, "unblock": true, "mute": true, "archive": true, | |
| "unarchive": true, "cancel": true, "send": true, "upload": true, "subscribe": true, | |
| "unsubscribe": true, "dismiss": true, "approve": true, "decline": true, | |
| } | |
| var whichWriteVerbs = map[string]bool{ | |
| "delete": true, "remove": true, "update": true, "create": true, "set": true, | |
| "add": true, "replace": true, "rename": true, "transfer": true, "merge": true, | |
| "lock": true, "unlock": true, "star": true, "unstar": true, "follow": true, | |
| "unfollow": true, "block": true, "unblock": true, "mute": true, "archive": true, | |
| "unarchive": true, "cancel": true, "send": true, "upload": true, "subscribe": true, | |
| "unsubscribe": true, "dismiss": true, "approve": true, "decline": true, | |
| "post": true, "put": true, "write": true, "edit": true, "modify": true, | |
| "publish": true, "share": true, "comment": true, "grant": true, "revoke": true, | |
| } |
|
Third generator defect found while validating this PR on a live regeneration: syncResourcePath derivation misses nested resources. The coingecko spec declares /coins/markets, but the generated path map only carries top-level entries, so the markets resource falls back to /markets and 404s. Currently hand-fixed in the library copy (markets -> /coins/markets?vs_currency=usd&order=market_cap_desc); the durable fix is deriving sync paths for nested path items (and their required query params) in the profiler. Validation of THIS PR's store v2 on a real db: user_version migrated 1 -> 2 in place, 18456 coins preserved through the rebuild, 100 market rows now persist under the composite PK (they vanished under v1). |
|
@roboulos please rebase to fix merge conflicts and address code review feedback. |
2 participants
Two generator-template defects found live through Snappy OS's connector substrate, both fixed at the template so every generated CLI heals on regen.
1. which scorer cannot rank obvious natural-language capability asks (which.go.tmpl)
github-pp-cli which "my authenticated user repositories"never surfaced the indexeduser repos-list-for-authenticated(not even in the top 30): hyphenated command leaves were matched as single exact tokens, so capability words inside a leaf were unmatchable and every command in a group tied on the group token with source order deciding.Fixes: sub-token splitting (spaces/hyphens/underscores/slashes) for both command paths and queries, one-sided prefix normalization (repositories ~ repos), capped description token credit (uncapped credit let token-soup descriptions outrank the precise command), possessive aliasing (my/mine/me/current boosts authenticated-scoped commands), a write-verb penalty so read-shaped asks never rank destructive commands first on ties, and a specificity tie-break (fewest capability sub-tokens wins).
Proof on the rebuilt github CLI: "my authenticated user repositories", "list repositories for authenticated user", and "repos-list-for-authenticated" all rank
user repos-list-for-authenticatedfirst; coingecko "top coins by market cap" ->coins marketsunregressed. Known limit: bare "my repos" still favors group-token matches.2. store v1 PK silently merges rows across resource types (store.go.tmpl)
resources.idwas PRIMARY KEY alone, so a markets upsert for idbitcoinmerged into the coins row and apinghealth-check row ate a real coin named Ping - rows vanished while sync_summary reported success. Fixed: composite PRIMARY KEY (resource_type, id) + matching conflict target, StoreSchemaVersion 2 with an in-place rebuild for v1 databases, and type-namespaced generic FTS rowids.Both verified against live CLIs (github, coingecko) and the generator test suite is green.
🤖 Generated with Claude Code