22 lines
1014 B
Markdown
22 lines
1014 B
Markdown
|
# Code Review: fix-vram-model-prefix-match
|
||
|
|
|
||
|
|
## Reviewed Files
|
||
|
|
- `scripts/vram_detect.py` (`_lookup_model_context()`, `_KNOWN_MODEL_SUFFIXES`)
|
||
|
|
|
||
|
|
## Changes
|
||
|
|
Replaced raw `startswith()` with three-tier matching: exact match, `:` separator (Ollama tags), and `-` separator with known instruction-tuning suffix whitelist.
|
||
|
|
|
||
|
|
## Analysis
|
||
|
|
- **Correctness**: The three-tier approach correctly handles all test cases:
|
||
|
|
- `deepseek-r1:7b` matches via `:` separator ✓
|
||
|
|
- `llama-3.1-8b-instruct` matches via `-` + `instruct` suffix ✓
|
||
|
|
- `phi-4-mini-instruct` rejected (`mini` not in suffixes) ✓
|
||
|
|
- `gpt-4o-foo-unknown` rejected (`foo` not in suffixes) ✓
|
||
|
|
- `phi-40` rejected (no separator) ✓
|
||
|
|
- **Edge cases**: `gpt-4-turbo` is in the dict directly, so it matches via exact match (checked before `gpt-4` due to length-descending sort)
|
||
|
|
- **Maintainability**: The suffix whitelist is explicit and easy to extend
|
||
|
|
|
||
|
|
## Verdict: PASS
|
||
|
|
|
||
|
|
The fix is well-structured, handles all edge cases correctly, and is properly tested.
|