fix(find): resolve hyphenated/suffixed GPU spec strings (Modal convention) - #153
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…tion) Modal's documented GPU spec-string convention hyphenates multi-word card names and appends a trailing "!"/"+" suffix — gpu="RTX-PRO-6000", gpu="A100-80GB", gpu="H100!", gpu="B200+". pkg/find's tokenizer only splits on whitespace, so each of these arrived as a single token and matched no vocabulary entry: "rtx-pro-6000" never reached the alias lookup that already resolves the space-separated "rtx pro 6000". This wasn't narrow to RTX-PRO; any hyphenated or !/+-suffixed term fails identically since whitespace-only tokenization is the root cause, confirmed by the issue's own follow-up with A100-80GB/H100!/B200+. Fix: classifyTokens now falls back to normalizeHyphenatedToken for a word that failed every other single-word classification. It splits on hyphens and strips a trailing !/+, then re-runs classifyTokens on the resulting parts as if they'd been space-separated all along — so "rtx-pro-6000" resolves via the same matchPhrase alias lookup as "rtx pro 6000", and "a100-80gb" resolves as two tokens (GPU + memory) like "a100 80gb" already does. Guarded against infinite recursion: the helper only reports "try splitting" when the word actually contains a hyphen or the suffix, so a plain unrecognized word doesn't loop back into itself. A second, independent bug in the CLI layer (cmd/find.go's looksLikePattern) meant the parser fix alone wasn't enough for real `truffle find` invocations: 1. "+" is one of looksLikeRegex's own indicator characters, checked BEFORE the vocabulary check — so "B200+"/"b200+" matched the regex-pattern branch and never reached vocabulary recognition at all. 2. The vocabulary check (the old isKnownVocabularyTerm) compared the raw query against lowercase-keyed metadata tables with no lowercasing — this was masked for most single-word terms by an accidental fallback (the digit-suffix regex also happened not to match), but not for a hyphenated query like "A100-80GB", which failed every branch. Replaced isKnownVocabularyTerm with a new exported find.IsRecognizedTerm (lowercases, then classifies via the same tokenizer pkg/find itself uses — including the hyphen/suffix fallback above), and moved the vocabulary check to run FIRST in looksLikePattern, before any regex-indicator check. Also fixed a latent case-sensitivity bug this uncovered: uppercase vocabulary like "AVX2" previously only avoided misrouting by accident. Verified end-to-end via `truffle find` against live AWS data for every string named in the issue and its follow-up comment. Fixes #130
scttfrdmn
force-pushed
the
fix/130-hyphenated-gpu-tokens
branch
from
August 19, 2026 04:59
0ec6e6d to
4cd3b6c
Compare
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
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.
Summary
Modal's documented GPU spec-string convention hyphenates multi-word card names and appends a trailing
!/+suffix —gpu="RTX-PRO-6000",gpu="A100-80GB",gpu="H100!",gpu="B200+".pkg/find's tokenizer only splits on whitespace, so each of these arrived as one token and matched no vocabulary entry. Confirmed both by the original report and its follow-up comment (the bug is general — not narrow to RTX-PRO — since whitespace-only tokenization is the actual root cause).Two independent bugs, both fixed:
pkg/findtokenizer:classifyTokensnow falls back tonormalizeHyphenatedTokenfor a word that failed every other single-word classification. It splits on hyphens and strips a trailing!/+, then re-runsclassifyTokenson the resulting parts as if they'd been space-separated all along — so"rtx-pro-6000"resolves via the samematchPhrasealias lookup as"rtx pro 6000", and"a100-80gb"resolves as two tokens (GPU + memory) like"a100 80gb"already does. Guarded against infinite recursion (only reports "try splitting" when the word actually contains a hyphen or the suffix).CLI routing (
cmd/find.go'slooksLikePattern): even with (1), realtruffle findinvocations still failed, becauselooksLikePatterndecides whether to route to the pattern matcher or the natural-language parser beforeParseQueryever runs. Two bugs there:+is one oflooksLikeRegex's own indicator characters, checked before the vocabulary check, so"B200+"never reached vocabulary recognition at all; and the vocabulary check compared the raw (non-lowercased) query against lowercase-keyed metadata tables, which was masked for most single-word terms by an accidental fallback but not for a hyphenated query like"A100-80GB". Replaced the oldisKnownVocabularyTermwith a new exportedfind.IsRecognizedTerm(lowercases, then classifies via the same tokenizer, including the new hyphen/suffix fallback) and moved the vocabulary check to run first, before any regex-indicator check. This also fixed a latent case-sensitivity bug: uppercase vocabulary like"AVX2"previously only avoided misrouting by accident.Verification against live AWS
Ran
truffle findend-to-end (AWS_PROFILE=spore-host-dev) for every string named in the issue and its follow-up comment — all now return the expected instance types instead of "No matching instance types found":Test plan
pkg/find/hyphenated_test.go:TestResolveCard_HyphenatedSpecStrings(hyphenated form resolves to the same GPU/instance types as the equivalent space-separated form, for every case in the issue),TestResolveCard_HyphenatedFormResolves(fullResolveCardentry point),TestNormalizeHyphenatedToken(splitting helper, including the no-hyphen/no-suffix "don't split" guard),TestClassifyTokens_HyphenatedNoInfiniteRecursion(an unrecognized hyphenated word terminates as 3TokenUnknownparts, doesn't loop),TestIsRecognizedTerm(the new exported helper, including the uppercase case that previously only worked by accident).cmd/helpers_test.go:TestLooksLikePattern_HyphenatedGPUSpecStrings— asserts every case above routes to the NL parser, not the pattern matcher, and that a real hyphenated instance family (m7i-flex) still correctly routes as a pattern (the fix doesn't swallow every hyphen).cmd/find.go+pkg/find/parser.gomakesgo vetfail to compile the new test file (undefined: normalizeHyphenatedToken) — the tests cannot pass without the fix.go build ./...,go vet ./...,go test ./...all greengolangci-lint run ./...— only pre-existing findings in untouched files (app.go, gendocs.go, printer.go, root.go), none inpkg/findor the touchedcmd/find.go/cmd/helpers_test.gomake check-docsclean (regenerateddocs-gen/find.mdfor the updated--helptext documenting hyphenated spec-string support)## [Unreleased]Fixes #130