refactor: P0 hardening for model attribution in codex/kimi parsers#9
Conversation
There was a problem hiding this comment.
Pull request overview
P0 hardening of provider-side model attribution to reduce false positives and improve robustness when model names must be inferred from Codex JSON payloads or Kimi log fallback parsing.
Changes:
- Codex: replace broad recursive model extraction with known-path lookup + placeholder rejection.
- Kimi: replace strict
model='...'regex with flexible key=value field parsing and “latest model wins” per session. - Add focused regression tests and a design/acceptance doc for the P0 scope.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| provider/codex/parser.go | Introduces explicit model-path whitelist and candidate filtering for Codex model extraction. |
| provider/codex/parser_test.go | Adds regression tests for known-path preference and placeholder rejection. |
| provider/kimi/parser.go | Updates Kimi log fallback to parse key=value model fields and keep latest per session. |
| provider/kimi/parser_test.go | Adds tests for log-line variants and latest-model behavior per session. |
| docs/model-attribution-p0.md | Documents P0 scope, non-goals, acceptance criteria, and test coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if info.ModelName == "" { | ||
| info.ModelName = strings.TrimSpace(msg.Model) | ||
| if info.ModelName == "" { | ||
| info.ModelName = extractModelFromRawJSON(msg.Info) |
There was a problem hiding this comment.
parseCodexSession assigns info.ModelName directly from msg.Model without running it through the new placeholder/candidate filter. If msg.Model can be values like default/auto/unknown, the parser will lock in a placeholder and skip extracting the real model from msg.Info/other payloads. Apply isLikelyCodexModelName (or equivalent) to msg.Model before accepting it, and fall back to the JSON-path extraction when it’s not a valid model name.
There was a problem hiding this comment.
Fixed in 2dcce7b: msg.Model is now gated by isLikelyCodexModelName before assignment. If it is a placeholder (e.g. default), parser falls back to msg.Info/payload path extraction. Added regression test TestParseCodexSession_ModelExtractionSkipsPlaceholderMsgModel.
Summary
P0 Plan (Implemented)
docs/model-attribution-p0.md.Acceptance Criteria
Using LLM modellines.Use Cases Covered
provider/kimi/parser_test.go: log line variants, latest-model mapping, legacy fallback path.provider/codex/parser_test.go: known-path extraction with noisy metadata, placeholder rejection.Changes
provider/codex/parser.go:default/auto/unknown/rate-limitrejection).provider/kimi/parser.go:model='...'regex with key-value field parsing.model/model_name/model_id/modelidkeys.provider/codex/parser_test.go:provider/kimi/parser_test.go:docs/model-attribution-p0.md:Validation
go test ./provider/codex ./provider/kimigo test ./stats ./cmd ./e2emake testmake lintmake buildmake vetrun_staged_acceptance.shrun_staged_acceptance.sh --run-testsRisks / Notes
stats/aggregator.go) is out of scope for this P0 and remains a follow-up item.