review: document the suffix-match tradeoff in modelComponentsMatch

9e92e76→main·

dipree·1mo ago·2 files·+21 added/-3 removed

A review note flagged that an end-aligned (suffix) span like 'sonnet-4' vs 'claude-sonnet-4' is rejected. This is intentional, not a regression (the prior <= bound also rejected it via the end<len(long) body check): a suffix has no following component, so there's no version boundary to distinguish a real less-specific id from a bare fragment. Allowing suffixes would re-introduce the 'mini' -> 'gpt-4o-mini' and '4-5' -> 'claude-sonnet-4-5' false positives. Realistic configured models (aliases/families/full names) still match because the recorded model carries a trailing version. Expanded the comment with the tradeoff and added tests pinning both the rejected suffix cases and the working alias/family matches.

Sessions

75dabf75ec5dView transcript

?\ Checkout the hand off doc that I just added.Pi·Opus 4.8·3 steps

Changes

2

• cmd/entire/cli/review

• Mmanifest.go+14/-3

• Mmanifest_test.go+7

492 unmodified lines

493
494
495
496
497
498
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512

492 unmodified lines

if len(short) == 0 || len(short) >= len(long) {
        return false
    }
    // Stop before the end-aligned window (i+len(short) < len(long)): a legitimate
    // match needs a numeric component immediately AFTER the matched span (a
    // version/date), so the span is always a strict, non-suffix subspan of long.
    // Visit every start offset whose matched span still has a following
    // component. i+len(short) < len(long) keeps long[i+len(short)] in bounds, so
    // even at the largest offset the span is followed by long's LAST element —
    // the span is never a suffix. A match requires that following component to be
    // purely numeric (a version/date), the boundary that confirms the same model.
    //
    // Suffix windows (i+len(short) == len(long)) are intentionally excluded: with
    // no following component there's no boundary to tell a real less-specific id
    // from a bare fragment, so allowing them would let "mini" match "gpt-4o-mini"
    // or "4-5" match "claude-sonnet-4-5". The cost is that a rare family+version
    // tail like "sonnet-4" won't match "claude-sonnet-4"; realistic configured
    // models (aliases like "sonnet", families like "claude-sonnet", or full names)
    // still match because the recorded model carries a trailing version (e.g.
    // "sonnet" matches "claude-sonnet-4-5").
    for i := 0; i+len(short) < len(long); i++ {
        if componentsEqualAt(long, short, i) && isNumericComponent(long[i+len(short)]) {
            return true
    }

Mcmd/entire/cli/review/manifest.go+14/-3

779 unmodified lines

780
781
782
783
784
785
786
787
788
789
790
791
792

779 unmodified lines

{"slash provider prefix stripped then identical", "anthropic/claude-sonnet", "claude-sonnet", true},
        {"family matches across a provider component at offset", "claude-sonnet", "anthropic-claude-sonnet-4-5", true},
        {"match where the next component is the last element", "sonnet-4", "claude-sonnet-4-5", true},
        // Suffix-only spans are intentionally rejected (no version boundary to
        // confirm the same model); this is what also keeps bare fragments and
        // variant words from matching. Realistic configured models still match via
        // the trailing version (see the alias/family cases above).
        {"family+version suffix is intentionally not matched", "sonnet-4", "claude-sonnet-4", false},
        {"variant-word suffix must not match", "mini", "gpt-4o-mini", false},
        {"bare version suffix must not match", "4-5", "claude-sonnet-4", false},
        {"thinking-suffix-only difference matches", "claude-sonnet:high", "claude-sonnet:low", true},
        {"equal-length different family does not match", "claude-sonnet", "claude-opus", false},
        {"equal-length different version does not match", "claude-sonnet-4", "claude-sonnet-5", false},

Mcmd/entire/cli/review/manifest_test.go+7