model-usage: parse numeric-string costs and skip non-finite values by shuofengzhang · Pull Request #37877 · openclaw/openclaw

shuofengzhang · 2026-03-06T13:56:39Z

What changed

Added a parse_cost() helper in skills/model-usage/scripts/model_usage.py.
aggregate_costs(), pick_current_model(), and latest_day_cost() now use parse_cost() so they:
- accept numeric string costs (for example "1.75"),
- ignore boolean values,
- ignore non-finite values (NaN, Infinity).
Extended skills/model-usage/scripts/test_model_usage.py with coverage for:
- numeric-string cost aggregation,
- skipping non-finite/bool costs,
- parsing numeric-string latest-day cost.

Why

Some cost payloads can contain numeric values serialized as strings.
The previous logic only accepted int/float, so valid string costs were silently dropped.
Filtering non-finite and boolean values makes output more robust and avoids propagating invalid totals.

Testing

✅ source .venv/bin/activate && pytest -q skills/model-usage/scripts/test_model_usage.py
✅ scripts/clone_and_test.sh openclaw/openclaw

greptile-apps · 2026-03-06T13:59:08Z

Greptile Summary

This PR introduces a parse_cost() helper in model_usage.py to robustly normalize cost values before aggregation. The helper correctly handles three previously unguarded cases: numeric strings (e.g. "1.75"), boolean values (which Python silently treats as int without an explicit guard), and non-finite floats such as NaN or Infinity. All three call sites (aggregate_costs, pick_current_model, latest_day_cost) are updated consistently to use the helper, and the existing logic is unchanged for valid numeric inputs.

Key observations:

The bool guard (isinstance(value, bool) before isinstance(value, (int, float))) is essential and correctly placed, since bool is a subclass of int in Python.
math.isfinite cleanly covers both NaN and ±Infinity in a single check.
Two new tests validate string-cost parsing in aggregate_costs and latest_day_cost, but pick_current_model — which received the same change — has no new test coverage for these edge cases.

Confidence Score: 4/5

Safe to merge; the changes are backward-compatible with no logical errors.
This PR is well-scoped and correct. The implementation properly handles all edge cases with good defensive checks. The one gap is incomplete test coverage for pick_current_model's use of parse_cost with string/bool/non-finite costs, which prevents a perfect score but does not represent a functional risk.
skills/model-usage/scripts/test_model_usage.py — consider adding test coverage for pick_current_model with the new cost type handling, to match coverage of aggregate_costs and latest_day_cost.

_{Last reviewed commit: 9316b09}

greptile-apps · 2026-03-06T13:59:12Z

+    def test_aggregate_costs_accepts_numeric_strings_and_skips_non_finite_values(self):
+        entries = [
+            {
+                "modelBreakdowns": [
+                    {"modelName": "gpt-5", "cost": "1.25"},
+                    {"modelName": "gpt-5", "cost": "nan"},
+                    {"modelName": "gpt-5", "cost": True},
+                    {"modelName": "gpt-5-mini", "cost": 0.75},
+                ]
+            }
+        ]
+
+        totals = aggregate_costs(entries)
+
+        self.assertEqual(totals["gpt-5"], 1.25)
+        self.assertEqual(totals["gpt-5-mini"], 0.75)
+
+    def test_latest_day_cost_parses_numeric_string(self):
+        entries = [
+            {"date": "2026-03-04", "modelBreakdowns": [{"modelName": "gpt-5", "cost": 0.5}]},
+            {"date": "2026-03-05", "modelBreakdowns": [{"modelName": "gpt-5", "cost": "1.75"}]},
+        ]
+
+        latest_date, latest_cost = latest_day_cost(entries, "gpt-5")
+
+        self.assertEqual(latest_date, "2026-03-05")
+        self.assertEqual(latest_cost, 1.75)
+


Missing coverage for pick_current_model with new cost types

pick_current_model was updated to use parse_cost, but there is no test exercising the new behaviour with string costs, boolean values, or non-finite values. The two new tests only cover aggregate_costs and latest_day_cost. Consider adding a test such as:

def test_pick_current_model_accepts_numeric_string_cost(self): entries = [ {"date": "2026-03-05", "modelBreakdowns": [ {"modelName": "gpt-5", "cost": "2.00"}, {"modelName": "gpt-5-mini", "cost": True}, # should be ignored ]} ] model, _ = pick_current_model(entries) self.assertEqual(model, "gpt-5")

Without this, a regression in pick_current_model's use of parse_cost would go undetected.

Prompt To Fix With AI

This is a comment left during a code review. Path: skills/model-usage/scripts/test_model_usage.py Line: 38-65 Comment: Missing coverage for `pick_current_model` with new cost types `pick_current_model` was updated to use `parse_cost`, but there is no test exercising the new behaviour with string costs, boolean values, or non-finite values. The two new tests only cover `aggregate_costs` and `latest_day_cost`. Consider adding a test such as: ```python def test_pick_current_model_accepts_numeric_string_cost(self): entries = [ {"date": "2026-03-05", "modelBreakdowns": [ {"modelName": "gpt-5", "cost": "2.00"}, {"modelName": "gpt-5-mini", "cost": True}, # should be ignored ]} ] model, _ = pick_current_model(entries) self.assertEqual(model, "gpt-5") ``` Without this, a regression in `pick_current_model`'s use of `parse_cost` would go undetected. How can I resolve this? If you propose a fix, please make it concise.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9316b090e0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-06T14:02:27Z

+    if isinstance(value, (int, float)):
+        parsed = float(value)


Handle overflow when parsing integer costs

parse_cost now does float(value) for any int without catching OverflowError, and this commit calls parse_cost before modelName validation in both aggregate_costs and pick_current_model. With a malformed row such as {"modelName": null, "cost": 10**1000}, the script now raises and exits instead of skipping the bad row, so a single corrupt payload entry can break the whole usage report.

Useful? React with 👍 / 👎.

openclaw-barnacle · 2026-03-06T19:05:37Z

Closing this PR because the author has more than 10 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit.

model-usage: accept numeric string costs and ignore non-finite values

9316b09

openclaw-barnacle Bot added the size: S label Mar 6, 2026

shuofengzhang mentioned this pull request Mar 6, 2026

model-usage: handle numeric-string costs and ignore non-finite values #37878

Closed

greptile-apps Bot reviewed Mar 6, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Mar 6, 2026

View reviewed changes

thewilloftheshadow added the r: too-many-prs Auto-close: author has more than twenty active PRs. label Mar 6, 2026

openclaw-barnacle Bot closed this Mar 6, 2026

clawsweeper Bot mentioned this pull request Jun 19, 2026

fix(model-usage): coerce numeric-string costs and ignore non-finite values #87861

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

model-usage: parse numeric-string costs and skip non-finite values#37877

model-usage: parse numeric-string costs and skip non-finite values#37877
shuofengzhang wants to merge 1 commit into
openclaw:mainfrom
shuofengzhang:fix/model-usage-parse-cost

shuofengzhang commented Mar 6, 2026

Uh oh!

greptile-apps Bot commented Mar 6, 2026

Uh oh!

greptile-apps Bot Mar 6, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 6, 2026

Uh oh!

openclaw-barnacle Bot commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

shuofengzhang commented Mar 6, 2026

What changed

Why

Testing

Uh oh!

greptile-apps Bot commented Mar 6, 2026

Greptile Summary

Confidence Score: 4/5

Uh oh!

greptile-apps Bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

openclaw-barnacle Bot commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants