Fix JSONL parsing warnings and add missing model pricing#127
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMade Claude Code deserialization tolerant of null/unknown fields and optional tool result content. Expanded the in-code model registry with many new models, aliases, and free-tier handling via a refactored lookup. Minor pattern/guard refactors in several analyzers and the TUI; behavior preserved. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/analyzers/claude_code.rs (1)
313-315: Consider keeping one low-noise signal for skipped schema types.These catch-alls solve the warning storm, but they also make future Claude schema drift invisible. A per-run counter or a single aggregated
warn_oncewhenOtheris hit would preserve observability without bringing back the old log spam.Also applies to: 428-430
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/analyzers/claude_code.rs`:
- Around line 331-350: The current deserialize_u64_or_null function turns
explicit JSON nulls into 0 on deserialization, which causes Usage fields in
struct Usage to be treated as real zeroes during fingerprinting/merge; change
the model to preserve "unknown" vs zero by making these Usage fields Option<u64>
(keep deserialize_u64_or_null or rename to deserialize_option_u64 to return
Option<u64>), update deserialize logic to return None for null/missing and
Some(n) for numbers, and then adjust the dedup fingerprint and merge path
(functions like merge_message_into and any fingerprint computation) to use
presence/absence (None vs Some(0)) rather than raw numeric 0 when deciding
deduplication and summation so unknown counts are not treated as zeroes during
merge.
In `@src/models.rs`:
- Around line 997-1007: Add missing alias entries in MODEL_ALIASES to mirror the
newly added models in MODEL_INDEX: add self-referencing mappings for
"glm-5-code", "step-3.5-flash", "solar-pro-3", and "aurora-alpha" and also add
date-suffixed variants (e.g., "glm-5-code-YYYYMMDD" => "glm-5-code",
"step-3.5-flash-YYYYMMDD" => "step-3.5-flash", etc.) following the existing
alias patterns used for entries like "glm-5-20260211" => "glm-5"; update
MODEL_ALIASES in src/models.rs so lookups for both canonical and date-tagged
names resolve to the canonical model keys.
- Around line 1065-1068: The suffix branch that handles
base_name.strip_suffix("-free") currently returns
lookup_model(base).unwrap_or(&FREE_MODEL_INFO), which incorrectly uses the base
model's pricing when the lookup succeeds; change it so that whenever a "-free"
suffix is detected the function returns FREE_MODEL_INFO unconditionally (i.e.,
always return Some(&FREE_MODEL_INFO) from the strip_suffix("-free") branch
instead of calling lookup_model).
- Around line 1049-1053: The current branch handling
after_slash.strip_suffix(":free") calls
lookup_model(base).unwrap_or(&FREE_MODEL_INFO), which returns the base model's
real pricing when found; change it so :free always yields $0 pricing by either
(A) returning FREE_MODEL_INFO unconditionally instead of
lookup_model(...).unwrap_or(...), or (B) if you need to preserve other metadata
from lookup_model(base), create/return a copy of the found model with its
pricing fields zeroed (e.g., lookup_model(base).map(|m|
m.with_zero_pricing()).unwrap_or(&FREE_MODEL_INFO)); update the code around
after_slash.strip_suffix(":free"), lookup_model, and FREE_MODEL_INFO
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7b5554dd-6217-4ddc-929f-4ded08c39bd3
📒 Files selected for processing (2)
src/analyzers/claude_code.rssrc/models.rs
mike1858
force-pushed
the
fix/jsonl-parsing-and-model-pricing
branch
from
557fa7a to
bff9595
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/models.rs (1)
1011-1017: Consider adding date-suffixed aliases for consistency.The new models
step-3.5-flash,solar-pro-3, andaurora-alphaonly have self-referencing aliases. Other models in the codebase include date-suffixed variants (e.g.,glm-5-20260211→glm-5). Consider adding similar aliases for consistency, though this is optional if these models don't use date-based versioning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models.rs` around lines 1011 - 1017, The three new model alias entries ("step-3.5-flash", "solar-pro-3", "aurora-alpha") currently only map to themselves; add date-suffixed aliases consistent with the rest of models so date-versioned names resolve to the base name. In src/models.rs, next to the existing entries for step-3.5-flash, solar-pro-3, and aurora-alpha, add entries like "step-3.5-flash-YYYYMMDD" => "step-3.5-flash", "solar-pro-3-YYYYMMDD" => "solar-pro-3", and "aurora-alpha-YYYYMMDD" => "aurora-alpha" (use the actual date patterns used elsewhere), following the same map structure and ordering as surrounding alias entries so date-suffixed lookups behave consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/models.rs`:
- Around line 347-356: The "gpt-5.4" entry currently uses PricingStructure::Flat
and a single CachingSupport::OpenAI rate but the spec requires tiered pricing
and caching; update the "gpt-5.4" ModelInfo to use PricingStructure::Tiered (or
equivalent) with two tiers: ≤272k context -> input_per_1m: 2.50, output_per_1m:
15.00; >272k context -> input_per_1m: 5.00, output_per_1m: 22.50, and set
CachingSupport::OpenAI with two corresponding cached rates: 0.25 for ≤272k and
0.50 for >272k, preserving is_estimated: false and keeping the same "gpt-5.4"
key and ModelInfo structure.
---
Nitpick comments:
In `@src/models.rs`:
- Around line 1011-1017: The three new model alias entries ("step-3.5-flash",
"solar-pro-3", "aurora-alpha") currently only map to themselves; add
date-suffixed aliases consistent with the rest of models so date-versioned names
resolve to the base name. In src/models.rs, next to the existing entries for
step-3.5-flash, solar-pro-3, and aurora-alpha, add entries like
"step-3.5-flash-YYYYMMDD" => "step-3.5-flash", "solar-pro-3-YYYYMMDD" =>
"solar-pro-3", and "aurora-alpha-YYYYMMDD" => "aurora-alpha" (use the actual
date patterns used elsewhere), following the same map structure and ordering as
surrounding alias entries so date-suffixed lookups behave consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a3b5c5a-1287-4835-9a04-ca00e4817215
📒 Files selected for processing (5)
src/analyzers/claude_code.rssrc/analyzers/codex_cli.rssrc/analyzers/copilot.rssrc/models.rssrc/tui.rs
Claude Code analyzer: - Add catch-all #[serde(other)] variant to ClaudeCodeEntry enum to silently skip unknown entry types like 'last-prompt' - Add catch-all #[serde(other)] variant to ContentBlock enum to handle new block types like 'tool_reference' and 'redacted_thinking' - Make ToolResult.content optional to handle tool results with no content field - Add deserialize_u64_or_null helper to handle null token counts in Usage struct (OpenRouter sends null instead of omitting fields) Model pricing: - Add model name normalization to get_model_info() that strips provider prefixes (minimax/, z-ai/, openrouter/, etc.) and handles :free and -free suffixes with $0 pricing - Add pricing for GPT-5.4, GLM-5, GLM-5-Code, GLM-4.5-Air, MiniMax M2.5, Step 3.5 Flash, Solar Pro 3, Aurora Alpha - Add date-suffixed aliases for new models Reduces startup warnings from 130+ to 2 (unrelated to Claude Code).
mike1858
force-pushed
the
fix/jsonl-parsing-and-model-pricing
branch
from
bff9595 to
c4f606b
Compare
Member
Author
|
@coderabbitai Review. |
|
✅ Actions performedReview triggered.
|
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
1 participant
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.
Claude Code analyzer:
Model pricing:
Reduces startup warnings from 130+ to 2 (unrelated to Claude Code).
Closes #126.
Summary by CodeRabbit