fix(OMN-20833): the cost-accounting hook prices from the infra pricing manifest (plugin 2.4.59)

[jake-b-omni]

What

The PostToolUse cost-accounting hook stops keeping its own prices. plugins/onex/hooks/lib/cost_accounting.py drops PRICING_USD_PER_1M and the hand-written PRICING_MANIFEST_VERSION. _cost_usd now reads each model's rate from the omnibase_infra pricing manifest, the one pricing authority:

• A model the manifest lacks stays unpriced, as omniclaude#2566 made it.
• If omnibase_infra cannot be imported in the hook's interpreter, every cloud model is unpriced and no cost record is written. It is never priced from a stale copy.
• local stays at zero marginal API cost.
• Each record's pricing_manifest_version now names the omnibase_infra release it was priced from.

The literals were off the published rates. Opus 4.6 and 4.5 were at $15 / $75 against a published $5 / $25, and Haiku 4.5 was at $0.80 / $4 against $1 / $5. Because BASELINE_MODEL is Opus 4.6, every baseline and saving the hook wrote was three times too high on the Opus rate. Today's omnibase_infra release still carries the old $15 / $75 Opus row, so the hook reads the corrected figure once a release carries omnibase_infra#4631. omnimarket's pin moves with that release under OMN-20833 AC1.

The four config/hardcoded_model_config_baseline.yaml rows the literals held are removed. The ratchet reported them stale.

Plugin 2.4.59, with plugin.json and the dev marketplace entry bumped together.

Acceptance criteria (OMN-20833)

• AC3: tests/unit/hooks/lib/test_cost_accounting_manifest_omn20833.py::test_the_hook_carries_no_price_literal and test_a_model_is_priced_at_the_manifest_rate.
• AC4, the hook's half: test_a_planted_unknown_model_is_unpriced_and_writes_no_record and test_without_the_manifest_every_cloud_model_is_unpriced. omnimarket's half is omnimarket#3667.

Tests

RED first, at origin/dev: uv run pytest tests/unit/hooks/lib/test_cost_accounting_manifest_omn20833.py gave 3 failed and 3 passed. The failures were the no-literal test, the Haiku 4.5 rate and the no-manifest test. Opus and Sonnet passed at dev only because today's omnibase_infra release still carries the old Opus figure.

After the change:

• tests/unit/hooks/lib/test_cost_accounting.py, tests/unit/hooks/lib/test_cost_accounting_manifest_omn20833.py and tests/hook_measurement/: 37 passed.
• ruff and mypy are clean.
• pre-commit run --files <changed>: every hook passed.

Lab

Lab: host=mac-scratch lane=omn20833-manifest-pricing command=uv run pytest tests/unit/hooks/lib/test_cost_accounting.py tests/unit/hooks/lib/test_cost_accounting_manifest_omn20833.py tests/hook_measurement/ -q observed=37 passed head=bae4b65968c8c048c5ca20171a19233392c9caff

This is a detached host. No lab lane, lab host or cluster was touched.

🤖 Generated with Claude Code

Evidence-Ticket: OMN-20833
Evidence-Source: OCC#13468

[github-actions]

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 14
Models succeeded: qwen3-review,local-studio-planner

Findings

Severity	Location	Finding
error	tests/unit/hooks/lib/test_cost_accounting_manifest_omn20833.py:1	ai-reviewer:qwen3-review:correctness Unit test asserts no decimal literals, but code contains 0.0 | The test test_the_hook_carries_no_price_literal strips comments and asserts that re.findall(r"\b\d+\.\d*[1-9]\d*\b", code) == []. However, the source code in cost_accounting.py contains return 0.0 in _cost_usd and return 0.0 in _savings_method (via the LOCAL_MODEL check). The regex \b\d+\.\d*[1-9]\d*\b requires at least one non-zero digit after the decimal point (the [1-9] class). 0.0 has 0 after the decimal, so it does NOT
warning	plugins/onex/hooks/lib/cost_accounting.py:1	ai-reviewer:qwen3-review:correctness Unit cost conversion factor is wrong: divides by 1_000 instead of 1_000_000 | The old code used PRICING_USD_PER_1M and divided by 1_000_000. The new code reads input_cost_per_1k and output_cost_per_1k (per 1K tokens) and divides by 1_000. This is arithmetically correct IF the manifest truly stores per-1K rates. However, the test test_opus_baseline_cost computes the expected value as (entry.input_cost_per_1k + entry.output_cost_per_1k) * 1_000 for 1M tokens each. That equals `input_cost_per_1k
warning	plugins/onex/hooks/lib/cost_accounting.py:1	ai-reviewer:qwen3-review:performance _pricing_table() re-imports and re-loads YAML on every call | _pricing_table() calls importlib.import_module and ModelPricingTable.from_yaml() on every invocation of _cost_usd. _cost_usd is called at least twice per record_tool_call (once for actual model, once for baseline). There is no caching. If from_yaml() reads a file from disk each time, this introduces I/O on the hot path of every tool-call hook. The old code used a module-level constant dict with zero I/O. This is a performance regres
info	plugins/onex/hooks/lib/cost_accounting.py:1	ai-reviewer:qwen3-review:security Broad except in _pricing_table swallows all exceptions silently | _pricing_table() catches Exception and returns None. This means a misconfigured YAML, a missing file, a permission error, or a bug in ModelPricingTable.from_yaml() all silently degrade to 'no pricing'. The caller then writes no cost record. In a production environment, this means cost accounting silently stops without any log, alert, or error surface. The comment says 'rather than one priced from a stale copy', which is a valid design
warning	tests/unit/hooks/lib/test_cost_accounting_manifest_omn20833.py:1	ai-reviewer:qwen3-review:completeness No test for the case where omnibase_infra is not installed | The new test file imports ModelPricingTable at module level: from omnibase_infra.models.pricing.model_pricing_table import ModelPricingTable. If omnibase_infra is not installed in the test environment, the entire test module fails to import and all tests in it are skipped or error. The production code handles the missing dependency gracefully (returns None, no record written), but the test suite does not verify this path. The test `test_with
info	plugins/onex/hooks/lib/cost_accounting.py:1	ai-reviewer:qwen3-review:correctness _manifest_version() returns 'unavailable' on any exception, losing diagnostic info | _manifest_version() catches all exceptions and returns the string 'unavailable'. This is written into the pricing_manifest_version field of every cost record. If the omnibase-infra package is installed but its metadata is corrupted, or if the version string is malformed, the record will say 'unavailable' with no way to distinguish 'package not installed' from 'metadata broken'. This reduces the auditability of cos
info	tests/unit/hooks/lib/test_cost_accounting_manifest_omn20833.py:1	ai-reviewer:qwen3-review:testing Test regex for price literals is fragile and likely to produce false negatives | The regex \b\d+\.\d*[1-9]\d*\b matches a decimal number only if the fractional part contains at least one non-zero digit. This means 0.0, 0.00, 1.0, 2.0 all do NOT match. If a future developer adds a price like 15.00 (which has a zero in the fractional part but is a real price), the regex would miss it because the [1-9] requires a non-zero digit somewhere in the fractional part. Actually 15.00 would not match be
info	plugins/onex/hooks/lib/cost_accounting.py:1	ai-reviewer:qwen3-review:architecture Hard dependency on omnibase_infra in a hook that may run in a minimal interpreter | The hook now imports omnibase_infra.models.pricing.model_pricing_table at call time. The comment acknowledges that if the import fails, no cost record is written. However, hooks in Claude Code plugins may run in a sandboxed or minimal Python environment where omnibase_infra is not installed. The design choice to silently drop all cost records in that case is a significant operational gap. There is no fallback, no warning
info	tests/unit/hooks/lib/test_cost_accounting_manifest_omn20833.py:1	ai-reviewer:qwen3-review:completeness No test verifies that LOCAL_MODEL returns 0.0 when pricing table is available | The test test_without_the_manifest_every_cloud_model_is_unpriced asserts mod._cost_usd("local", 1_000, 1_000) == 0.0 when the table is None. But there is no test that verifies mod._cost_usd("local", ...) returns 0.0 when the pricing table IS available. The LOCAL_MODEL check happens before the table lookup, so it should always return 0.0, but this is not explicitly tested in the positive case. | Evidence: assert mod._cost
error	tests/unit/hooks/lib/test_cost_accounting.py:1	ai-reviewer:local-studio-planner:correctness Unit test logic error in cost calculation assertion | The test test_opus_baseline_cost and test_a_model_is_priced_at_the_manifest_rate contain a mathematical error in the expected value calculation. The function _cost_usd divides by 1,000,000 (converting per-1M tokens to total cost). The test assertions multiply the per-1k rates by 1,000, which is equivalent to dividing by 1,000. This means the tests assert that the cost for 1M input and 1M output tokens is $1/1000$th of the actual price. For example,
warning	plugins/onex/hooks/lib/cost_accounting.py:1	ai-reviewer:local-studio-planner:correctness Incorrect token count scaling in cost calculation | The implementation of _cost_usd divides the total weighted cost by 1,000. The comment and previous hardcoded manifest indicated prices were 'per 1M tokens'. If input_cost_per_1k is indeed per 1,000 tokens, the divisor should be 1,000,000 to match the input/output token counts which are absolute counts. Dividing by 1,000 results in costs that are 1,000x higher than intended if the rates are per-1k. If the rates are per-1M, the divisor is correct but the
warning	plugins/onex/hooks/lib/cost_accounting.py:1	ai-reviewer:local-studio-planner:security Silent failure masks pricing data loss | The _pricing_table function catches all exceptions and returns None. This means if the external dependency omnibase_infra is corrupted, missing, or throws a non-ImportError exception (e.g., YAML parsing error, file not found), the system silently falls back to unpriced models. While this prevents crashes, it results in no cost records being written for cloud models, potentially hiding billing discrepancies or operational issues from monitoring. | Evidence: diff
info	tests/unit/hooks/lib/test_cost_accounting.py:1	ai-reviewer:local-studio-planner:testing Test relies on external manifest state | The tests test_opus_baseline_cost and test_a_model_is_priced_at_the_manifest_rate directly call ModelPricingTable.from_yaml().get_entry(model). This couples the unit test to the current state of the external pricing manifest. If the manifest changes (e.g., prices are updated), these tests will fail or pass incorrectly without any code change, making them brittle and dependent on external data rather than verifying the logic itself. | Evidence: diff --git a/test
info	plugins/onex/hooks/lib/cost_accounting.py:1	ai-reviewer:local-studio-planner:correctness Inconsistent model name handling for local models | The code uses a constant LOCAL_MODEL = "local" to identify local models. However, the previous hardcoded manifest included a key "local". If the delegation result or hook event passes a different string representing a local model (e.g., "localhost", "self-hosted"), it will not be recognized as local and will attempt to look up in the pricing table, likely resulting in None cost. This is a minor logic gap if the input source of truth for 'local' i

---

Gate semantics (pilot phase)

Verdict	Meaning	Blocks merge?
passed	No critical/error findings	No
blocked	CRITICAL or ERROR findings found	Yes
degraded	Reviewer ran but every model failed mid-run	No (pilot)
infra_error	cli_review failed to run at all (install/crash) after 2 attempts	Yes
unavailable_fork_isolation	Fork PR — the LAN-only model is unreachable and the fleet is off limits	Yes

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)