refactor(cost): rename experiments→runs + add path discriminator (#409)#522
Merged
Conversation
) Resolves the long-standing namespace collision between cost-tracking's internal "experiment_id" concept and MLflow's separate experiment notion. The cost-tracking table tracks *runs* through a project — distinct invocations on a shared project_id, not MLflow experiments. Schema: - experiments -> runs - experiments.experiment_id -> runs.run_id - token_events.experiment_id -> token_events.run_id - runs.path TEXT NOT NULL DEFAULT 'unknown' (new: direct/marcus/posidonius) - indexes renamed: idx_experiments_* -> idx_runs_*, idx_te_exp* -> idx_te_run* - new idx_runs_path Migration: one-shot idempotent rename in _runs_rename_migration(). Detects legacy `experiments` table, runs ALTER TABLE RENAME, adds path column with default 'unknown'. Safe on fresh installs (no-op) and on already-migrated DBs (no-op). Attribution: - create_project pre-generates run_id at wrapper entry and rebinds on success. Defaults path="direct" so unwrapped Direct MCP callers get a correct discriminator without code changes. - experiments/spawn_agents.py sets path="marcus" by default; Posidonius config can override to "posidonius". API renames: - CostStore.record_experiment -> record_run - CostAggregator.list_experiments -> list_runs - CostAggregator.experiment_summary -> run_summary - TokenEvent.experiment_id -> run_id - PlannerContext.experiment_id -> run_id - AgentBinding.experiment_id -> run_id - get_cost_summary MCP tool: experiment_id arg -> run_id, response key experiments -> runs - Dataclass Experiment -> Run (with new path field) MLflow's start_experiment MCP tool is untouched — that namespace remains MLflow's. The cost-tracking "experiment" was always a different concept; this rename makes that explicit. Simon: 7ed3074d (decision) Closes the rename work agreed on after Kaia's interpretation review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @lwgray's task —— View job PR Review: experiments→runs refactor + path discriminatorI've completed my review of this pull request. Here's my comprehensive feedback: ✅ Overall Assessment: Strong refactor with good practicesThis is a well-executed rename with comprehensive test coverage and careful migration handling. The code quality is high and the rationale is sound. 🎯 Strengths1. Well-reasoned architectural decision
2. Safe migration strategy
3. Excellent test coverage
4. API consistency
🔍 Code QualityMigration Logic (cost_store.py:576-644): def _runs_rename_migration(self) -> None:
Schema Design:
|
3 tasks
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 151863f634
ℹ️ 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".
Comment on lines
+27
to
+29
| def _mock_api_key(monkeypatch: pytest.MonkeyPatch) -> None: | ||
| """Config validation needs CLAUDE_API_KEY set.""" | ||
| monkeypatch.setenv("CLAUDE_API_KEY", "test-key-for-unit-tests") |
There was a problem hiding this comment.
In a fresh checkout without config_marcus.json, MarcusConfig.from_file() returns the default config and validation does not read CLAUDE_API_KEY directly, so this fixture still leaves anthropic_api_key unset for tests that reach get_config(). I checked pytest -q tests/unit/marcus_mcp/test_create_project_run_recording.py::TestRunRecording::test_one_run_row_per_successful_call, and it fails before the patched creator is used; patch get_config()/set MARCUS_CONFIG to a test config or otherwise reset the config object so CI doesn't depend on a developer-local config file.
Useful? React with 👍 / 👎.
Comment on lines
+221
to
+229
| state.cost_store.record_run( | ||
| Run( | ||
| run_id=_run_id, | ||
| project_id=_canonical, | ||
| project_name=project_name, | ||
| started_at=_started_at, | ||
| path=_path, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
When a client retries create_project within the existing 10-minute dedup window after the first call succeeded, _create_project_inner() returns the cached success without doing another project traversal or emitting costs. This wrapper still treats that cached result as a fresh success and inserts a new runs row with a new run_id, so dashboards/counts will show extra zero-cost runs for timeout/retry scenarios; record only when the inner call actually performed work or reuse the original run metadata.
Useful? React with 👍 / 👎.
P1 — make run-recording tests independent of local config
The _mock_api_key fixture set CLAUDE_API_KEY in env, but MarcusConfig.
from_file() only does ${CLAUDE_API_KEY} substitution when a config
file is actually loaded. On a fresh checkout without config_marcus.
json the substitution never fires, defaults stay unset, and the
get_config()/validate() call from create_project's kanban-provider
lookup raises. Codex reproduced this in a clean tree.
Replace the env-var fixture with a direct stub of the module-level
_config singleton, so the test is hermetic on any developer machine
and in CI regardless of local files.
P2 — don't insert phantom runs rows on dedup-cached retries
When create_project is invoked within the 10-minute dedup window
after a successful first call, _create_project_inner returns the
cached result immediately — no decomposition, no token_events. The
wrapper still pre-generated a fresh run_id at entry, so on success
it would insert a brand-new runs row for the cached replay. Result:
zero-cost phantom rows piling up per retry storm, breaking dashboard
counts and the runs picker.
Mark cached replays with an internal _dedup_cached flag inside the
inner function (set on a shallow copy so the cache entry stays
clean). The wrapper pops the flag and skips record_run +
rebind_project_id when set, so the agent's response is unchanged
but the runs table holds exactly one row per real invocation.
Also:
- Add pytestmark = pytest.mark.unit to both touched test files so
the 16 cost/run-recording tests actually run under `pytest -m unit`
(the PR-blocking CI marker). Mirrors the convention in sibling
files like test_lease_recovery_state_sync.py.
- New regression test TestCodexP2DedupReplay verifies three identical
calls in a row still produce exactly one runs row.
Baseline: 1313 passed, 1 skipped under `pytest -m unit` (was 1297;
+16 tests now actually run in CI).
Refs: #522 (Codex review)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @lwgray's task —— View job PR Review: experiments→runs refactor + path discriminatorI've completed a comprehensive review of this pull request. Here's my detailed feedback: ✅ Overall Assessment: Strong refactor with good engineering practicesThis is a well-executed rename with comprehensive test coverage and careful migration handling. The code quality is high and the rationale is sound. 🎯 Strengths1. Well-reasoned architectural decision
2. Safe migration strategy
3. Excellent test coverage
4. API consistency
🔍 Code Quality ReviewMigration Logic (cost_store.py:576-644):
Deduplication Handling:
|
Owner
Author
|
Addressed both Codex findings in P1 — test hermeticity. Replaced the P2 — phantom runs on cached retries. Real bug, thanks for catching. Reasoning:
Fix: inner now marks the cached return with a shallow-copied Plus: added Baseline:
|
This was referenced May 12, 2026
Closed
lwgray
added a commit
that referenced
this pull request
May 13, 2026
What changed ------------ - ``CostStore.close_run`` stamps ``ended_at`` plus task/agent counters with a COALESCE-guarded UPDATE (live values win, NULL preserves existing). - ``CostStore.close_open_runs_for_project`` bulk-closes every open run for a project; called from ``end_experiment`` after ``monitor.stop()``. - ``CostAggregator.run_audit`` reports ``run_open``; ``project_audit`` reports ``runs_total`` and ``runs_open`` so the audit can detect the "every run open since insertion" failure mode. - ``scripts/backfill_run_close_state.py`` walks every NULL row and derives ``ended_at`` from ``MAX(token_events.timestamp) WHERE run_id = ?`` — best on-disk approximation for unattended close. Why --- The ``runs`` table has lifecycle columns the schema anticipates, but nothing in the Python code ever wrote them after #522. Every row has been "open" since insertion, blocking dashboard queries on ``ended_at IS NOT NULL`` and any wall-clock-time analysis (run duration, cost-per-minute, coordination-tax rate). The token-audit work in #528 confirmed token attribution but never asserted lifecycle closure — so the gap stayed silent. Real-data validation -------------------- Ran backfill on ``~/.marcus/costs.db``: $ python scripts/backfill_run_close_state.py Closed 1 runs. $ sqlite3 ~/.marcus/costs.db \ "SELECT COUNT(*), SUM(ended_at IS NULL) FROM runs;" 1|0 Test plan --------- - [x] ``pytest tests/unit/cost_tracking/`` — 95 passed (11 new) - [x] ``mypy src/cost_tracking/cost_store.py src/cost_tracking/cost_aggregator.py`` clean - [x] Backfill script ran end-to-end against real costs.db Related ------- - Closes #537 - Follows #522 (which added ``record_run`` without a close path) and #528 (which audited tokens but not lifecycle) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
lwgray
added a commit
that referenced
this pull request
May 13, 2026
* fix(cost): close runs rows on completion (#537) What changed ------------ - ``CostStore.close_run`` stamps ``ended_at`` plus task/agent counters with a COALESCE-guarded UPDATE (live values win, NULL preserves existing). - ``CostStore.close_open_runs_for_project`` bulk-closes every open run for a project; called from ``end_experiment`` after ``monitor.stop()``. - ``CostAggregator.run_audit`` reports ``run_open``; ``project_audit`` reports ``runs_total`` and ``runs_open`` so the audit can detect the "every run open since insertion" failure mode. - ``scripts/backfill_run_close_state.py`` walks every NULL row and derives ``ended_at`` from ``MAX(token_events.timestamp) WHERE run_id = ?`` — best on-disk approximation for unattended close. Why --- The ``runs`` table has lifecycle columns the schema anticipates, but nothing in the Python code ever wrote them after #522. Every row has been "open" since insertion, blocking dashboard queries on ``ended_at IS NOT NULL`` and any wall-clock-time analysis (run duration, cost-per-minute, coordination-tax rate). The token-audit work in #528 confirmed token attribution but never asserted lifecycle closure — so the gap stayed silent. Real-data validation -------------------- Ran backfill on ``~/.marcus/costs.db``: $ python scripts/backfill_run_close_state.py Closed 1 runs. $ sqlite3 ~/.marcus/costs.db \ "SELECT COUNT(*), SUM(ended_at IS NULL) FROM runs;" 1|0 Test plan --------- - [x] ``pytest tests/unit/cost_tracking/`` — 95 passed (11 new) - [x] ``mypy src/cost_tracking/cost_store.py src/cost_tracking/cost_aggregator.py`` clean - [x] Backfill script ran end-to-end against real costs.db Related ------- - Closes #537 - Follows #522 (which added ``record_run`` without a close path) and #528 (which audited tokens but not lifecycle) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(cost): canonicalize project_id in close_open_runs_for_project (#537) ``record_run`` stored the dashless UUID form via ``canonical_project_id``, but ``end_experiment`` passed ``monitor.project_id`` straight through. When the monitor was handed the dashed UUID form (ProjectRegistry path), the live close lookup matched zero rows and silently closed nothing — leaving the backfill script as the only safety net. Normalize at the close-helper entry: ``canonical_project_id(...)`` or fall back to the original string for non-UUID inputs (matches the rest of the cost layer, which normalizes on the way in). Regression test asserts a dashed input still closes the dashless row in ``runs``. Caught by Kaia on PR #538 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * comment to not fix code * fix(cost): close only latest open run per project on end_experiment (#537) The cost schema allows multiple ``runs`` rows per ``project_id`` (retries, repeated traversals — ``run_id`` is the PK, project views group/list runs). The bulk close in ``close_open_runs_for_project`` stamped every historical open row for the project with the just-finished experiment's ``ended_at`` and final counters, corrupting older runs. Replace with ``close_latest_open_run_for_project``: scope to the ``MAX(started_at)`` open run only. Older open rows are left to the periodic ``backfill_run_close_state.py`` script, which derives ``ended_at`` from each row's own ``MAX(token_events.timestamp)`` — the on-disk approximation that's correct per-run rather than a batch-overwrite from the live hook. Return type tightened from ``int`` (count) to ``Optional[str]`` (the closed run_id) so callers can log which row was touched. Caught by Codex P2 on PR #538. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7 tasks
lwgray
added a commit
that referenced
this pull request
May 14, 2026
* fix(experiments): isolate completion check from MLflow logging in monitor loop (#495)
Move _check_completion() into its own try/except in
LiveExperimentMonitor._monitor_loop so a ProjectMonitor /
MLflow failure (e.g. unreachable Planka board) no longer
swallows the kanban-DB completion signal. is_running now
flips to False and experiment_complete.json is written even
when the configured kanban provider can't be reached.
Also:
- Add tracked example configs derived from config_marcus.json:
config_marcus.planka.example.json, config_marcus.cloud.example.json,
and a refreshed config_marcus.local.example.json on the
canonical schema.
- Refresh config_marcus.example.json: enable features.memory by
default so TASK_STARTED / TASK_COMPLETED events reach
marcus.db (Cato's progress timeline depends on them); drop
unused sqlite_db_path / sqlite_attachments_dir keys; add
header comments.
- Migrate docs/assets/*.mp4 to LFS pointers (matches
.gitattributes filter).
- Refresh .secrets.baseline.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost-tracking): SQLite cost foundation — schema, capture, aggregation (#409 phases 1-3) (#497)
* feat(cost-tracking): SQLite-backed token event store (#409 phase 1)
Foundation for the unified cost dashboard described in #409. Introduces
src/cost_tracking/cost_store.py with three tables and one view:
- experiments — registry of runs (project, model, totals)
- token_events — one row per LLM call (immutable token counts;
generated total_tokens column)
- model_prices — versioned by effective_from; designed to be
edited by the Cato UI without repo changes
- v_event_cost — view joining events × the price active at each
event's timestamp, exposing cost_usd at query time
Tokens are truth (immutable). Cost is derived (prices change). This
schema lets us tweak pricing without rewriting historical experiments —
old runs keep their original cost.
CostStore handles inserts only; aggregation queries land in a follow-up
phase (cost_aggregator.py). WAL mode enabled so background ingesters
(planner middleware, worker JSONL tail) can write while Cato reads.
Tests: 15 cases, 98.89% coverage on cost_store.py, mypy --strict clean.
Validates schema creation, WAL mode, generated total_tokens column,
upsert semantics on experiments, versioned price coexistence, and the
v_event_cost view's historical-price selection.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost-tracking): planner-side capture in providers (#409 phase 2)
- New src/cost_tracking/cost_recorder.py: ContextVar-based recorder
with PlannerContext stack. Providers call record_planner_call after
each successful API request. Failures are swallowed so the recorder
cannot break the call path.
- anthropic_provider._call_claude captures all four token fields incl.
cache_creation_input_tokens and cache_read_input_tokens (which the
legacy middleware was dropping).
- local_provider._call_local_llm + cloud_provider._call_cloud_llm
capture prompt_tokens / completion_tokens. Provider tag distinguished
via _cost_provider_name hook ('local' vs 'cloud').
Tests: 11 new (8 recorder + 3 provider integration), 98%+ coverage on
new files. All 573 existing AI provider tests still pass. mypy --strict
clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost-tracking): read-only cost aggregator (#409 phase 3)
Adds src/cost_tracking/cost_aggregator.py — the query layer that powers
Cato's /api/cost/* endpoints. Returns dicts shaped like the API
responses documented in #409.
Methods: list_experiments, experiment_summary, session_turns,
cache_hit_rate_by_agent, project_totals. All hit v_event_cost so
historical cost stays correct after price edits.
Tests: 11 cases, 97.56% coverage, mypy --strict clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost-tracking): activate planner recorder at server startup (#409)
Wires CostStore + CostRecorder into MarcusServer.__init__ so planner-side
LLM calls start landing in ~/.marcus/costs.db immediately. set_recorder()
makes the singleton available to providers patched in phase 2.
Without an active PlannerContext, events fall back to 'unassigned' ids;
they still record correctly. MCP request handlers will push a context
in a follow-up phase.
84/84 marcus_mcp unit tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost-tracking): seed legacy Anthropic models + ISO timestamp default (#497)
Addresses Codex P1 + P2 on PR #497.
P1: DEFAULT_SEED now covers claude-3-haiku-20240307 (Marcus's default
config), claude-3-sonnet-20241022 (historic settings default),
claude-3-5-sonnet-20241022, claude-3-5-haiku-20241022, claude-3-opus.
Without these, v_event_cost INNER JOIN dropped out-of-box runs.
P2: token_events.timestamp default now uses
strftime('%Y-%m-%dT%H:%M:%fZ', 'now'). The bare CURRENT_TIMESTAMP
produced 'YYYY-MM-DD HH:MM:SS' which sorted before record_price()'s
isoformat() output, causing same-day price/event joins to misbehave.
4 new regression tests; 41/41 cost-tracking tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost-tracking): worker JSONL ingester (#409 phase 4) (#498)
Batch ingester that reads Claude Code worker session JSONL files and
writes one token_events row per assistant turn carrying message.usage.
- Filters non-assistant records (queue-operation, user, etc)
- Skips assistant records lacking usage block
- Captures all 4 token fields incl. cache_creation/cache_read
- Per-session turn_index counter, independent across sessions
- UUID-based dedup so re-ingest is a no-op
- Caller supplies resolve_binding callable (decoupled from spawn registry)
API: WorkerJSONLIngester.ingest_file(path) / .ingest_directory(path)
11 tests, 95.89% coverage, mypy --strict clean. Smoke-tested against
real 549-turn Claude Code session: ingested all 549 events, $5.61
cost rollup arithmetically correct via the haiku-4-5 seed prices.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost-tracking): MCP tool get_cost_summary (#409 phase 5) (#499)
* feat(cost-tracking): MCP tool get_cost_summary (#409 phase 5)
Exposes CostAggregator's experiment + project rollups via MCP so agents
and Cato can query cost data without direct DB access.
API: get_cost_summary(experiment_id|project_id) → API-shaped dict.
Read-only. Argument errors return {success: False, error} envelopes
rather than raising — friendlier for MCP clients.
Wired through handlers.py: imports, get_all_tool_definitions registry,
human_tools list, dispatch case in handle_tool_call.
8 new tests, mypy --strict clean. All 92 marcus_mcp unit tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost-tracking): add get_cost_summary to observer ROLE_TOOLS (#499)
Addresses Codex P1 on PR #499.
handle_tool_call filters through get_client_tools()/ROLE_TOOLS in
src/marcus_mcp/tools/auth.py before dispatch. The previous PR
registered get_cost_summary in handlers.py but missed the role list,
so Cato (authenticated as observer) would have seen the tool omitted
from list_tools and gotten access-denied before ever reaching the
new dispatch branch.
Adds get_cost_summary to ROLE_TOOLS['observer'] alongside the existing
get_usage_report. Regression test in TestCodexP1ObserverAccess pins
the entry so future ROLE_TOOLS edits don't drop it silently.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(kanban): handle dataclass / pydantic objects in JSON columns (#502) (#504)
* fix(kanban): handle dataclass / pydantic objects in JSON columns (#502)
Per-feature Implement and Test tasks were silently dropped during
SQLite kanban writes because their ``source_context`` carries
``UserOutcome`` dataclass instances attached by the outcome-coverage
pipeline (#449). ``json.dumps`` raised ``TypeError`` on these,
the kanban write failed per-task, and the symptom propagated as
"recipe-scale projects have zero Implement parents on the board"
while Design / foundation tasks (carrying plain dicts) survived.
Adds a ``_json_default`` encoder fallback covering dataclasses,
Pydantic v1/v2 models, and ``__dict__`` objects. Wires it into
all three JSON columns: ``source_context``, ``completion_criteria``,
``acceptance_criteria``.
Diagnosis: ``logs/marcus_20260510_215120.log`` recipe-revert-haiku
run shows ``Successfully generated all 15 tasks`` followed by
``TypeError: Object of type UserOutcome is not JSON serializable``
on each per-feature task during ``create_tasks_on_board``.
Two regression tests pin both fields against a synthetic dataclass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(kanban): handle nested non-JSON primitives in encoder (Codex P2)
Codex review on PR #504 flagged that ``model_dump()`` (Pydantic v2)
and ``dataclasses.asdict()`` return Python objects unchanged for
``datetime`` / ``UUID`` / ``Path`` / ``Enum`` / ``set`` fields.
``json.dumps`` then recursively invokes ``_json_default`` on those
nested values, finds no protocol match, and raises — the same silent
task-drop symptom as the original bug, one level deeper.
Two fixes:
- Switch Pydantic v2 path to ``model_dump(mode="json")``, which
recursively emits JSON-safe primitives in one pass. Falls back to
the kwarg-less form for Pydantic v1 / older signatures.
- Add explicit handlers for ``datetime``/``date`` (ISO 8601),
``UUID`` (string), ``Path`` (string), ``Enum`` (``.value``),
``set``/``frozenset`` (list), and ``bytes`` (base64). These catch
the recursive case when a dataclass field embeds them.
Reorder the protocol: Pydantic first (it knows its own nested types),
dataclass second, stdlib third, ``__dict__`` last. Also wrap
``.dict()`` callable in TypeError-only catch so a broken stub method
falls through instead of propagating.
Two new regression tests pin the nested-primitive and broken-dict-method
cases. 100/100 unit tests pass, mypy strict clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost): project-axis aggregation + automatic PlannerContext push (#409) (#503)
After Kaia review of phase 8: Marcus's identity is project_id (GH-388,
spawn_agents.py); experiment_id is an MLflow tracking handle. Refactor
the cost layer to lead with project_id.
CostAggregator gains list_projects() and unassigned_totals().
list_projects derives rollups from token_events.project_id with no
dependence on the experiments table. unassigned_totals surfaces the
'no PlannerContext' gap as a visible row.
handle_tool_call in marcus_mcp/handlers.py now pushes a PlannerContext
for every tool dispatch via ExitStack. Resolves project_id from
arguments → state.agent_project_map → state.selected_project_id, in
that order. Falls back to 'unassigned' if nothing resolves.
10 new tests (4 aggregator, 6 resolver). All 156 pass.
mypy --strict clean.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): don't attribute project-creation LLM work to the active project (#503) (#507)
Codex P1: resolver fell back to state.selected_project_id for
create_project, so its LLM decomposition cost landed under the
previously-active project. Adds _PROJECT_CREATION_TOOLS guard
(create_project, add_project, switch_project, update_project) — for
these tools the resolver skips the active-project fallback so events
land in the visible 'unassigned' bucket rather than the wrong project.
Explicit project_id in args still wins.
6 new regression tests; 12/12 resolver tests pass; mypy clean.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(contract-validation): loosen _normalize_type to ignore stylistic noise (#505)
* fix(contract-validation): loosen _normalize_type to ignore stylistic noise
The cross-contract type consistency gate fires on stylistic LLM phrasing
rather than real type disagreements, forcing contract_first → feature_based
fallback on Haiku-shaped runs. Three recipe-platform runs in
``logs/marcus_20260510_230613.log`` all failed on the same false positives:
- ``userid (string, UUID v4)`` vs ``userid (string, UUID)`` — same type
- ``description (string)`` vs ``description (string, 0-500 chars)``
- ``limit (number, default 20)`` vs ``limit (integer, default 20)``
- ``offset (number, default 0)`` vs ``offset (integer, default 0)``
None are real disagreements; they're format hints, length constraints,
and the ``number``/``integer`` JSON Schema synonym.
Loosening strategy in ``_normalize_type``:
1. Drop everything after the first comma. Trailing constraints describe
properties of the base type, not the type itself.
2. Canonicalize ``integer`` -> ``number`` per JSON Schema (integer is
a number subtype).
Regression-pinned must-still-catch cases:
- ``string`` vs ``number`` (WidgetPosition collision)
- ``array of strings`` vs ``array of IngredientItem objects``
Five new tests cover both directions. 142 contract-adjacent unit tests
still pass. Mypy strict clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(contract-validation): canonicalize integer inside union types (Codex P2)
Codex P2 on PR #505 flagged that the integer->number synonym swap only
triggered on whole-string equality. Nullable pagination fields like
``limit (integer | null, optional)`` vs ``limit (number | null, optional)``
slipped through (both reduced to ``integer|null`` vs ``number|null``
after the comma drop) and still forced contract_first fallback.
Apply the canonicalization per union member when ``|`` is present.
One new regression test pins the case. 12/12 tests green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost): plumb project_name through list_projects (Marcus #409) (#508)
* feat(cost): plumb project_name through list_projects (Marcus #409)
Kaia review of Cato PR #33 caught that the dashboard's project picker
shows project_id.slice(0, 12) — Marcus IDs are 19-digit numbers so
the truncation makes runs indistinguishable in the dropdown.
Fix flows from the SQL outward: list_projects now LEFT JOINs the
experiments table on experiment_id and surfaces MAX(project_name) per
project. NULL when no MLflow run was registered for the project —
the dashboard falls back to the truncated id in that case.
MAX(project_name) handles the rare case of multiple experiments under
the same project_id having different names; deterministic and avoids
GROUP BY non-determinism. LEFT JOIN means projects without any
registered MLflow run still appear (project_name=NULL), preserving
the existing 'projects with events but no run' surface.
Tests: 2 new (project_name attached when experiment exists, NULL
when none registered). 18/18 aggregator tests pass. mypy --strict
clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(cost): tighten list_projects docstring + JOIN comments (Kaia review of #508)
Three documentation touch-ups from Kaia's review:
1. Docstring no longer claims 'no dependence on the experiments table' —
that was true before the LEFT JOIN landed, now it's misleading.
Reframes the contract: token_events is the primary source, experiments
is best-effort enrichment for project_name.
2. JOIN comment now pins the safety invariant: experiments.experiment_id
must be unique (it's the PK in cost_store.SCHEMA_SQL). If a future
migration relaxes that, the LEFT JOIN fans out and silently inflates
every total. Early warning lives next to the JOIN.
3. MAX(project_name) rationale reframed honestly: it's masking-not-fixing
behavior. A name conflict usually means data drift. Intentional drift
detection is a follow-up; today we stay deterministic.
No behavior change. 18/18 tests still pass; mypy --strict clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(outcome-coverage): promote 'Task X' LLM artifact to 'Implement X' (#509)
* fix(outcome-coverage): promote 'Task X' LLM artifact to 'Implement X'
Haiku and qwen-class models pattern-match the literal word ``task`` out
of the gap-fill schema's ``"<short task name>"`` field description and
stamp it as a name prefix. Result on the kanban board was a mix of
``Implement X`` parents (from feature_based decomposer at
``advanced_parser.py:2980``) alongside ``Task X`` parents (from
``gap_fill_contract`` synthesis) — same semantic role, two different
verbs, neither informative on a board where everything is a task.
The phenomenon was visible only post-PR #479 because the snake_case
slug normalizer cleaned up ``task_signup_form`` into the deceptively-
deliberate-looking ``Task Signup Form``. The LLM had always emitted
the slug; #479 just made the artifact look intentional.
Fix extends ``_normalize_gap_task_name`` to promote ``Task X`` /
``Task: X`` prefixes to ``Implement X``. Slug pass runs first so
``task_signup_form`` -> ``Task Signup Form`` -> ``Implement Signup Form``
composes. Bare ``"Task"`` with no payload is left alone (signals
upstream prompt failure; don't silently rewrite into ``"Implement "``).
This is normalization of LLM artifact noise, not a HOW prescription —
agents still choose implementation freely. Kaia review confirmed the
fix preserves the multi-agency proclamation's WHAT-vs-HOW boundary.
Five new tests pin the new behavior. 72/72 file tests green, mypy
strict clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(outcome-coverage): promote only slug-origin Task prefixes (Codex P2 #509)
Codex P2 on PR #509 flagged that an unconditional ``Task `` -> ``Implement ``
rewrite mangles legitimate domain nouns in task-management products
(``Task Creation Form``, ``Task Assignment Rules``, ``Task Queue``)
where ``Task`` IS the domain term.
Restrict promotion to inputs where the slug pass actually fired
(underscores present, no spaces). That's the only confirmed LLM
artifact signature in the logged failure mode — Haiku/qwen emit
``task_signup_form`` slugs because they pattern-match the literal
word ``task`` out of the schema's ``"<short task name>"`` field
description. Human-readable ``Task X`` names from the LLM are
trusted as intentional.
Tests updated: dropped the too-aggressive direct-prefix promotion
assertions; added regression test pinning ``Task Creation Form`` /
``Task Assignment Rules`` / ``Task Queue`` as untouched.
13/13 normalizer tests green. Mypy strict clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): seed Anthropic prices from the official pricing page (#409) (#510)
* fix(cost): seed Anthropic prices from the official pricing page (#409)
Corrects three real seed errors (Opus 4.5-4.7 at $15/$75 instead of
$5/$25, Haiku 4.5 at $0.80/$4 instead of $1/$5, invented
claude-3-sonnet-20241022 identifier) and adds missing models from the
official pricing page (Opus 4.5/4.6 family, dated identifiers, Sonnet
4.5/4.0, Sonnet 3.7).
cache_creation_per_million maps to the 5-min cache write multiplier.
1-hour cache writes aren't representable in the current single-column
schema; documented for users to override via Cato.
All 59 cost-tracking tests pass; mypy --strict clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): set seed effective_from to 2026-05-11 (today)
The seed was using 2025-01-01 as a placeholder — that's misleading.
We don't actually know when each price became effective on Anthropic's
side, only that these are current values as of today. Date the seed
accordingly.
Implication: v_event_cost picks the latest price with effective_from
<= event.timestamp, so any pre-existing token_events row with a
timestamp before today drops from cost rollups (INNER JOIN). For users
with existing data, the workaround is to insert a backdated price row
via Cato's Pricing > Override form — the schema is built for exactly
this.
All 59 cost-tracking tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(cost): refresh stale 'as of 2025-01-01' comment above DEFAULT_SEED
Kaia review of PR #510 caught that the comment block still claimed the
seed values came from 2025-01-01 pricing. They're actually the values
read from the official page on _SEED_DATE (which the seed block already
documents). Drops the date claim and points readers to the populating
block instead.
* fix(cost): compat seed for configured default model (Codex P2 on #510)
Marcus's built-in config (src/config/settings.py, pm_agent_config.json)
sets model to ``claude-3-sonnet-20241022`` — a non-canonical identifier
that record_usage stamps onto events verbatim. Without a matching seed
row, v_event_cost's inner join drops those events from all aggregations.
Add a compat ModelPrice using Sonnet 3.5 pricing under that exact
identifier so default-config installs price correctly out of the box.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): dedup token_events on request_id (#409) (#511)
* fix(cost): dedup token_events on request_id (#409)
Cato's dashboard polls run_ingest every 30s, and run_ingest builds a
fresh WorkerJSONLIngester each call — whose in-memory ``_seen_uuids``
set is empty. Without DB-level dedup, every poll re-inserts every event
and token counts double silently.
- Add partial UNIQUE INDEX on token_events(request_id) WHERE NOT NULL.
Partial index keeps legacy/non-Claude rows (NULL request_id) unconstrained.
- Switch record_event to INSERT OR IGNORE; return existing event_id on
duplicate so callers see idempotent behavior.
- Regression tests cover the dup and NULL cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): dedup migration in _init_schema before unique index (Kaia on #511)
CREATE UNIQUE INDEX IF NOT EXISTS on token_events(request_id) raises
IntegrityError on any DB that accumulated duplicates pre-PR-#511 —
which is every install that ran Cato's 30s polling for any length of
time. Marcus wouldn't start.
- Add _dedup_pre_index_migration: one-shot DELETE keeping MIN(event_id)
per request_id. No-op on fresh installs (table absent) and on clean
DBs (no matching rows).
- Call from _init_schema before executescript so executescript's
CREATE UNIQUE INDEX always sees compacted data.
- Regression test builds a pre-#511-shape DB with duplicate request_ids
+ NULLs, reopens via CostStore, asserts dedup and index enforcement.
- Fix three pre-existing tests that used hardcoded duplicate request_ids
for distinct logical events (worker_ingester turn_index tests + the
basic two-record ingest test); real records always have distinct
requestIds per call.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): make dedup migration truly no-op + busy_timeout (#511 followup) (#512)
* fix(cost): make dedup migration truly no-op + add busy_timeout (#511 followup)
After PR #511 merged, restarting Marcus while Cato was polling caused
``OperationalError: database is locked``. Two issues:
1. _dedup_pre_index_migration ran a write-locking DELETE on every
startup, even on clean DBs where the unique index already existed.
Now short-circuits if ux_te_request_id is present — index existence
implies zero duplicates, so the DELETE is provably unnecessary.
2. sqlite3 connections default to busy_timeout=0, so any concurrent
writer (Cato's 30s run_ingest sweep) caused startup DDL to fail
immediately instead of waiting. Set PRAGMA busy_timeout=5000.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(cost): assert dedup DELETE is skipped once index exists
Spy via sqlite3 set_trace_callback during a second CostStore open;
verify no DELETE FROM token_events statement is issued. Covers Kaia's
nit on PR #512: the short-circuit behavior on ux_te_request_id presence
is simple to read but worth a regression test now that we depend on it
to avoid contending with concurrent Cato polling.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost): project_summary aggregator (#409 dashboard precursor) (#513)
* feat(cost): add project_summary aggregator method (#409)
Mirrors experiment_summary but scoped to project_id. Marcus's main
code path doesn't open MLflow experiments; the project axis is the
only universal identity for cost data. This unlocks the project-first
Cato dashboard surface (token events exist with project_id even when
the experiments table is empty).
Returns the same shape as experiment_summary (summary + by_role,
by_agent, by_task, by_operation, by_model) so the dashboard can drop
in project_id wherever it previously used experiment_id.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost): UUID normalization + project budget caps (Kaia followups on #513)
Two Kaia-spotted issues addressed:
1. UUID normalization at the write path. Marcus has two project-id
generators that disagree on format: ProjectRegistry uses dashed
canonical UUIDs (str(uuid.uuid4())), SQLiteKanban auto-discovery
uses dashless hex (.hex). Both end up in projects.json depending
on which code path created the project. token_events stored
whatever it got handed, so cost rows ended up in mixed format.
- New canonical_project_id() helper picks dashless as the
canonical form (matches the bulk of existing rows + cheapest
path).
- PlannerContext normalizes on construction (frozen dataclass +
__post_init__).
- WorkerJSONLIngester applies the same normalization to bindings
coming from spawn_agents project_info.json.
- Cato's dual-index name overlay becomes defense-in-depth; new
writes are guaranteed to land in the canonical form.
2. project_budgets table for project-level cost caps. Lives in the
cost DB (not ProjectRegistry) because the cap is a cost concept,
and keeping it here lets Cato own the write path without touching
project metadata. One row per project_id; set_project_budget
upserts in place; setting <= 0 clears the cap.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): count unpriced events in aggregator queries (Codex P2 on #513)
v_event_cost INNER-joins model_prices, so events whose
(model, provider) has no matching price get silently dropped before
COUNT(*). Real-world data has 905 such rows on this account
('<synthetic>' planner artifacts + local 'qwen-25-coder-q5'), causing
project_summary to undercount events / tokens, miss whole projects
that only use unpriced models, and return None for projects that
actually do have token events.
- Add v_event_cost_inclusive view (LEFT JOIN, cost_usd=0 when no
matching price). Same shape as v_event_cost so the swap is mechanical.
- Switch every aggregator query that needs true event/token counts:
project_summary, experiment_summary, list_projects, list_experiments,
project_totals, unassigned_totals, session_turns. v_event_cost
itself is kept for any caller that explicitly wants the priced-only
semantics (none exists today, but the contract is preserved).
- Regression test: add a '<synthetic>' event to the populated fixture
and assert project_summary counts it, tokens add up, and the
by_model row has cost=$0 (correctly unpriced).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost): per-operation/model token + cache breakdown (#513 followup)
User feedback on PR #513 dashboard: 'I need to know which calls are
using the most tokens and if they are at least cached tokens. I need
to know where I need to tighten my prompts.'
Enrich by_operation and by_model in project_summary with:
- input_tokens, cache_creation_tokens, cache_read_tokens,
output_tokens — the full token-type split per row
- cache_hit_rate per row (cache_read / (input + cache_creation +
cache_read)) so heavy non-cached operations surface immediately
- Sort by tokens DESC (was cost) so the highest-volume operation
comes first — that's the prompt-tightening target
Same shape addition to by_model lets users see whether a specific
provider/model is benefiting from prompt caching.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost): persistent project names + create_project rebind + monitor attribution (#515)
* feat(cost): persistent project names + create_project rebind + monitor attribution (#409)
User feedback on dashboard: "'Unnamed' is unacceptable, every token has
to be accounted for. No LLM calls happen before create_project so any
calls during that period all belong to that project."
Three coordinated fixes:
1. Persistent project_names table. Cost data outlives Marcus's project
registry — when a project is deleted, its name was lost and the
picker fell back to opaque hex IDs. New table snapshots
(project_id, name) whenever a PlannerContext is pushed with a name.
Cato reads from it as the primary name source. Names persist
forever, even after registry deletion.
2. create_project two-phase attribution. Tool entry pushes a
placeholder PlannerContext (project_id="pending:<random hex>") so
heavy decomposition LLM calls land with attribution instead of in
the 'unassigned' bucket. After create_project returns the real id,
rebind_project_id UPDATEs every placeholder row to the real id.
Concurrency-safe by construction: ContextVar scopes the placeholder
per asyncio task, random UUID per call prevents collisions across
parallel create_project calls. Note: ProjectRegistry.add_project
itself is NOT race-safe yet — tracked separately as #514.
3. ProjectMonitor attribution. _analyze_project_health runs in a
background loop outside any MCP context, so its AI calls landed in
'unassigned'. Now wraps the analyze call in a synthetic
"monitor:<board_id>" PlannerContext with the project name from
current_state, keeping monitor LLM cost visible against the
project being analyzed.
Also: spawn_agents project_info.json now includes project_name so
worker session ingestion can carry the name end-to-end.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): Kaia review nits on #515
Four issues caught in Kaia's architecture review:
1. Orphan placeholder rows in project_names. Every create_project
left a dead 'pending:<hex>' entry indexed by a key nothing would
ever look up. rebind_project_id now DELETEs the placeholder row
from project_names alongside the token_events UPDATE.
2. Hot-path SQL on every planner_context push. The recorder did one
SQL upsert per tool call even when the (project_id, name) pair
had already been snapshotted this process. Added a process-
lifetime Set[(id, name)] cache so repeated pushes short-circuit.
Renames bypass the cache and re-write. SQL stays the source of
truth — cache is purely a hot-path optimization.
3. Dead fallback in handlers.py. result.get("project", {}).get("id")
was never hit — the canonical return shape is result["project_id"].
Removed.
4. Leaky abstraction note on registry._cache. Added a TODO pointing
at the right fix (expose ProjectRegistry.get_cached_project)
without ripping out the working code today.
Tests: 93 pass (3 new across orphan cleanup, dedup short-circuit,
rename re-write).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): attribute monitor LLM calls to real project_id (Codex P2 on #515)
The previous attribution wrapper used ``f"monitor:{state.board_id}"``
as the cost project_id. ``board_id`` and ``project_id`` are distinct
in every provider that separates the two (Planka, the SQLiteKanban
auto-discovery path) — verified against CostAggregator.list_projects
which GROUP BYs token_events.project_id, so a monitor:<board_id> id
shows up as a synthetic ghost project rather than rolling into the
real project's totals.
Fix: pull the real Marcus project_id off the kanban client
(self.kanban_client.project_id, which is the canonical id, not the
board id). When no project_id is available (kanban not initialized
or no active board), fall through to 'unassigned' rather than mint
a synthetic id — that's correct because without a project_id we
genuinely don't know what to attribute the call to.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): plug 100% planner-cost attribution leak (Marcus #409) (#516)
* fix(cost): plug 100% planner-cost attribution leak (#409 followup)
Diagnostic showed that despite #515 wiring create_project placeholder
attribution through handlers.py, every recent planner LLM call still
landed in 'unassigned' (267 events / 932k tokens). Root cause: TWO
independent bugs stacked:
1. FastMCP entry point bypasses handlers.py. Marcus exposes
create_project via two paths: the legacy mcp.server.Server (stdio,
routes through handlers.py:handle_tool_call) and FastMCP HTTP
(routes directly to nlp.create_project via @app.tool() at
server.py:1608). HTTP is what users actually run. So #515's
placeholder push in handlers.py never fired in practice.
Fix: move the placeholder push + rebind logic inside
nlp.create_project itself. Now every caller path — FastMCP, legacy
handler, direct Python — gets the same attribution behavior.
2. OpenAIProvider had no recorder hook. The other three providers
(anthropic, cloud, local) call get_recorder().record_planner_call()
after a successful response. openai_provider._call_openai did not.
On accounts where the Anthropic key is missing/invalid, every
fallback to OpenAI silently bypassed cost tracking — which made
the 'planner' role disappear from cost breakdowns even though
Marcus was making heavy decomposition calls.
Fix: add the recorder hook to _call_openai with the same shape as
the other providers. Recorder failures are swallowed so a
cost-store outage cannot break the provider call path.
Validated live: a fresh create_project now produces 18 events
correctly attributed to the new project_id (rebound from placeholder).
Zero new 'unassigned' rows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): shadow placeholder in background design phase (Codex P1 on #516)
``create_project`` wraps ``_create_project_inner`` in a
``planner_context`` carrying a synthetic ``pending:<uuid>``
placeholder project_id, then runs a one-shot ``rebind_project_id``
after the inner call returns. ``_run_design_phase`` is spawned via
``asyncio.ensure_future`` *during* the inner call, so the new task
inherits the placeholder via the parent's ContextVar state copy.
After the wrapper's one-shot rebind fires, the background design
phase keeps writing rows under the placeholder — those rows are
never rebound and stay hidden as ``pending:*``.
Fix: ``_run_design_phase`` pushes a fresh ``PlannerContext``
carrying the real project_id (resolved from ``kanban_client``) as
its very first action. That shadows the inherited placeholder for
the design phase's whole lifetime — every LLM call inside records
against the real project from the start. The wrapper's rebind
still catches the synchronous rows under the placeholder; the
background design phase no longer contributes any.
The body is split into ``_run_design_phase_body`` so the wrapper
can do the context push without forcing the entire 280-line body
into an extra indentation level. Behavior identical otherwise —
the existing 22 design-autocomplete tests still pass without
modification.
Defensive ``isinstance(_raw_pid, str)`` check: tests use
MagicMock-backed kanban_clients whose ``project_id`` resolves to
another MagicMock. Without the check we'd push a context with a
non-string project_id and the recorder would silently swallow the
SQL error.
Two new tests in ``TestRunDesignPhaseCostAttribution`` lock in the
fix: one captures ``recorder.current()`` from inside the body and
asserts it sees the real project_id (not the placeholder), the
other proves the MagicMock-style fallback doesn't crash.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost): per-operation drill-down for planner LLM calls (#409) (#517)
* fix(cost): plug 100% planner-cost attribution leak (#409 followup)
Diagnostic showed that despite #515 wiring create_project placeholder
attribution through handlers.py, every recent planner LLM call still
landed in 'unassigned' (267 events / 932k tokens). Root cause: TWO
independent bugs stacked:
1. FastMCP entry point bypasses handlers.py. Marcus exposes
create_project via two paths: the legacy mcp.server.Server (stdio,
routes through handlers.py:handle_tool_call) and FastMCP HTTP
(routes directly to nlp.create_project via @app.tool() at
server.py:1608). HTTP is what users actually run. So #515's
placeholder push in handlers.py never fired in practice.
Fix: move the placeholder push + rebind logic inside
nlp.create_project itself. Now every caller path — FastMCP, legacy
handler, direct Python — gets the same attribution behavior.
2. OpenAIProvider had no recorder hook. The other three providers
(anthropic, cloud, local) call get_recorder().record_planner_call()
after a successful response. openai_provider._call_openai did not.
On accounts where the Anthropic key is missing/invalid, every
fallback to OpenAI silently bypassed cost tracking — which made
the 'planner' role disappear from cost breakdowns even though
Marcus was making heavy decomposition calls.
Fix: add the recorder hook to _call_openai with the same shape as
the other providers. Recorder failures are swallowed so a
cost-store outage cannot break the provider call path.
Validated live: a fresh create_project now produces 18 events
correctly attributed to the new project_id (rebound from placeholder).
Zero new 'unassigned' rows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): shadow placeholder in background design phase (Codex P1 on #516)
``create_project`` wraps ``_create_project_inner`` in a
``planner_context`` carrying a synthetic ``pending:<uuid>``
placeholder project_id, then runs a one-shot ``rebind_project_id``
after the inner call returns. ``_run_design_phase`` is spawned via
``asyncio.ensure_future`` *during* the inner call, so the new task
inherits the placeholder via the parent's ContextVar state copy.
After the wrapper's one-shot rebind fires, the background design
phase keeps writing rows under the placeholder — those rows are
never rebound and stay hidden as ``pending:*``.
Fix: ``_run_design_phase`` pushes a fresh ``PlannerContext``
carrying the real project_id (resolved from ``kanban_client``) as
its very first action. That shadows the inherited placeholder for
the design phase's whole lifetime — every LLM call inside records
against the real project from the start. The wrapper's rebind
still catches the synchronous rows under the placeholder; the
background design phase no longer contributes any.
The body is split into ``_run_design_phase_body`` so the wrapper
can do the context push without forcing the entire 280-line body
into an extra indentation level. Behavior identical otherwise —
the existing 22 design-autocomplete tests still pass without
modification.
Defensive ``isinstance(_raw_pid, str)`` check: tests use
MagicMock-backed kanban_clients whose ``project_id`` resolves to
another MagicMock. Without the check we'd push a context with a
non-string project_id and the recorder would silently swallow the
SQL error.
Two new tests in ``TestRunDesignPhaseCostAttribution`` lock in the
fix: one captures ``recorder.current()`` from inside the body and
asserts it sees the real project_id (not the placeholder), the
other proves the MagicMock-style fallback doesn't crash.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost): per-operation drill-down for planner LLM calls (#409)
Tags every Marcus planner LLM call with a logical operation key so the
Cato dashboard can render *which* operation spent the tokens, not just
that some did. Fix 2 of the cost-tracking follow-ups after #515 landed
the attribution rebind.
What changed
- New ``src.cost_tracking.operations`` catalog mapping operation keys
to ``{label, description, category}``. 32 entries grouped into
``decomposition``, ``runtime``, ``monitoring``, ``other``.
- New ``CostRecorder.operation_context(operation)`` context manager
that pushes a child PlannerContext with ``operation_override`` set.
Innermost wins; works under nested asyncio tasks via ContextVar.
- ``LLMAbstraction.analyze(..., operation=...)`` threads the key
through to the recorder so call sites can tag without touching every
provider HTTP path. High-level methods
(``analyze_task_semantics``, ``analyze_blocker_and_suggest_solutions``,
etc.) wrap themselves in the appropriate operation_context.
- ``AIAnalysisEngine.generate_structured_response(..., operation=...)``
threads through too, so the decomposer / dependency wiring /
contract generation / task-completeness validator all stamp the
right key.
- Updated every call site to pass an explicit operation (decompose_prd,
extract_outcomes, outcome_coverage_check, outcome_gap_fill,
filter_outcomes, discover_domains, synthesize_foundation_tasks,
generate_design_artifact, generate_design_decisions,
generate_project_scaffold, generate_contracts, generate_task_detail,
decompose_task, validate_task_completeness, validate_work,
analyze_blocker, infer_dependencies, enrich_task,
analyze_task_semantics, estimate_effort, plus post_analysis_*
for the post-project analyzers).
Tests
- New TestOperationContext suite verifies override/pop semantics and
no-parent no-op behavior.
- New test_operations_catalog.py verifies catalog shape, fallback for
unknown keys, and that all call-site keys are registered.
- Existing analyze mocks updated to accept the new ``operation``
kwarg (via ``**kwargs``) so they still work as side_effects.
The recorder is unchanged in its error-swallowing contract: failure
to attribute an operation never breaks the provider call path. Keys
that don't exist in the catalog fall through to a synthesized
"Unregistered operation" tooltip on the dashboard side.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): operation-taxonomy review fixes from Kaia
P2 — drift detection at record time
- ``CostRecorder.record_planner_call`` now warns once per process
per unregistered operation key. A typo at a call site (or a
newly-added operation nobody remembered to catalog) silently
lands in the dashboard's fallback bucket otherwise. The warning
surfaces it in dev logs without spamming production.
P3 — AnalysisType ↔ catalog drift test
- New ``TestAnalysisTypeCoverage`` asserts every ``AnalysisType``
enum value maps to a ``post_analysis_<value>`` catalog entry.
Catches future drift when a new analyzer type is added without
a corresponding catalog entry, instead of letting it silently
fall through to the synthesized fallback label.
Risk — Protocol-based test double
- New ``LLMAnalyzeClient`` Protocol in
``src/ai/providers/protocols.py`` pins the ``analyze()``
contract: ``async def analyze(prompt, context, *,
operation: Optional[str] = None) -> str``.
- New ``make_analyze_mock`` helper in ``tests/unit/conftest.py``
builds AsyncMocks that absorb unknown kwargs, so future
signature additions to ``analyze()`` won't require fanning out
``**kwargs`` across every test mock.
- Test coverage for both the Protocol and the helper.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): preserve operation override without parent (Codex P2 + Kaia nit)
Codex P2 on PR #517
- ``operation_context`` used to yield ``None`` when no parent
``PlannerContext`` was active. ``record_planner_call`` then never
saw the ``operation_override`` and recorded the provider's generic
``'analyze'`` bucket instead of the call site's intended operation.
- Background / standalone planner calls (e.g., the post-analysis
path that builds ``operation=f"post_analysis_..."`` from a
request but doesn't push a recorder context first) lost their
per-operation drill-down entirely.
- Fix: synthesize an ``'unassigned'`` ``PlannerContext`` carrying
just the override when no parent exists. Operation tag survives;
project / experiment fall through to ``'unassigned'`` as before.
- Test ``test_synthesizes_unassigned_parent_when_no_context`` locks
this in: an ``operation_context`` outside any project scope must
still stamp the override onto ``token_events.operation``.
Kaia review nit on PR #517
- The five high-level ``LLMAbstraction`` methods
(``analyze_task_semantics``, ``infer_dependencies_semantic``,
``generate_enhanced_description``, ``estimate_effort_intelligently``,
``analyze_blocker_and_suggest_solutions``) all repeated the same
four-line preamble that pushed an ``operation_context``.
- Extracted to a ``@_tagged_operation("<key>")`` decorator. The
recorder import stays inside the wrapper so importing
``llm_abstraction`` doesn't force the cost-tracking module load
at startup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(cost): rename experiments→runs + add path discriminator (#409) (#522)
* refactor(cost): rename experiments -> runs, add path discriminator (#409)
Resolves the long-standing namespace collision between cost-tracking's
internal "experiment_id" concept and MLflow's separate experiment notion.
The cost-tracking table tracks *runs* through a project — distinct
invocations on a shared project_id, not MLflow experiments.
Schema:
- experiments -> runs
- experiments.experiment_id -> runs.run_id
- token_events.experiment_id -> token_events.run_id
- runs.path TEXT NOT NULL DEFAULT 'unknown' (new: direct/marcus/posidonius)
- indexes renamed: idx_experiments_* -> idx_runs_*, idx_te_exp* -> idx_te_run*
- new idx_runs_path
Migration: one-shot idempotent rename in _runs_rename_migration().
Detects legacy `experiments` table, runs ALTER TABLE RENAME, adds path
column with default 'unknown'. Safe on fresh installs (no-op) and on
already-migrated DBs (no-op).
Attribution:
- create_project pre-generates run_id at wrapper entry and rebinds on
success. Defaults path="direct" so unwrapped Direct MCP callers get a
correct discriminator without code changes.
- experiments/spawn_agents.py sets path="marcus" by default; Posidonius
config can override to "posidonius".
API renames:
- CostStore.record_experiment -> record_run
- CostAggregator.list_experiments -> list_runs
- CostAggregator.experiment_summary -> run_summary
- TokenEvent.experiment_id -> run_id
- PlannerContext.experiment_id -> run_id
- AgentBinding.experiment_id -> run_id
- get_cost_summary MCP tool: experiment_id arg -> run_id, response key
experiments -> runs
- Dataclass Experiment -> Run (with new path field)
MLflow's start_experiment MCP tool is untouched — that namespace
remains MLflow's. The cost-tracking "experiment" was always a different
concept; this rename makes that explicit.
Simon: 7ed3074d (decision)
Closes the rename work agreed on after Kaia's interpretation review.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cost): address Codex review on #522 (P1+P2)
P1 — make run-recording tests independent of local config
The _mock_api_key fixture set CLAUDE_API_KEY in env, but MarcusConfig.
from_file() only does ${CLAUDE_API_KEY} substitution when a config
file is actually loaded. On a fresh checkout without config_marcus.
json the substitution never fires, defaults stay unset, and the
get_config()/validate() call from create_project's kanban-provider
lookup raises. Codex reproduced this in a clean tree.
Replace the env-var fixture with a direct stub of the module-level
_config singleton, so the test is hermetic on any developer machine
and in CI regardless of local files.
P2 — don't insert phantom runs rows on dedup-cached retries
When create_project is invoked within the 10-minute dedup window
after a successful first call, _create_project_inner returns the
cached result immediately — no decomposition, no token_events. The
wrapper still pre-generated a fresh run_id at entry, so on success
it would insert a brand-new runs row for the cached replay. Result:
zero-cost phantom rows piling up per retry storm, breaking dashboard
counts and the runs picker.
Mark cached replays with an internal _dedup_cached flag inside the
inner function (set on a shallow copy so the cache entry stays
clean). The wrapper pops the flag and skips record_run +
rebind_project_id when set, so the agent's response is unchanged
but the runs table holds exactly one row per real invocation.
Also:
- Add pytestmark = pytest.mark.unit to both touched test files so
the 16 cost/run-recording tests actually run under `pytest -m unit`
(the PR-blocking CI marker). Mirrors the convention in sibling
files like test_lease_recovery_state_sync.py.
- New regression test TestCodexP2DedupReplay verifies three identical
calls in a row still produce exactly one runs row.
Baseline: 1313 passed, 1 skipped under `pytest -m unit` (was 1297;
+16 tests now actually run in CI).
Refs: lwgray/marcus#522 (Codex review)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(outcome-coverage): inject success_signal into acceptance_criteria (#523 Slice A) (#524)
* docs: require college-student-readable GitHub issues and PR descriptions
Adds explicit style guidance to CLAUDE.md and the
github-issue-manager agent so issue bodies and PR descriptions are
written for readers with zero codebase context. Names the always/never
checklist (open with plain-English project framing, define every
internal term on first use, state the user-facing problem before the
technical one, include file paths, worked numerical examples for
measurable quantities, a "Where to look in the code first" table, and
a verification procedure). Style applies to new artifacts and rewrites
of existing ones.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(outcome-coverage): inject success_signal into acceptance_criteria (#523 Slice A)
Threads each in-scope user outcome's success_signal text into the
acceptance_criteria of every task the coverage mapping says addresses
it (both organic decomposition output and synthesized gap-fill tasks).
The existing WorkAnalyzer LLM static gate reads acceptance_criteria at
task completion, so this teaches that gate what user-observable
outcome a task must satisfy without changing WorkAnalyzer itself.
This is the static-layer half of #523. The runtime-layer half (extend
the integration smoke gate to accept a list of verifications driven by
success_signals) lands in a follow-up slice.
Changes
-------
* New helper `_enrich_acceptance_criteria_with_signals` — pure
function; returns new task list with signal text appended,
idempotent, skips out-of-scope outcomes, identity-passes uncovered
tasks.
* New helper `_translate_stub_ids_to_real_ids` — rewrites the
`_synth_for_coverage_<idx>` recoverage stub ids in
`coverage_after_fill` to the real `gap_fill_<uuid>` ids so the
enricher matches against the actual augmented task list.
* `SIGNAL_CRITERION_PREFIX = "User outcome verifiable: "` — stamped on
every appended criterion so downstream consumers can tell signal
criteria apart from template/LLM-emitted criteria and so re-running
enrichment is idempotent.
* Wired into both `apply_outcome_coverage_to_feature_graph` and
`apply_outcome_coverage_to_contract_graph` after gap-fill synthesis.
Tests
-----
* +13 unit tests for the enricher (purity, identity passthrough,
out-of-scope skip, multi-outcome / multi-task, idempotence,
empty / None / missing-mapping no-ops, unknown task id ignored)
* +6 unit tests for the stub-id translator
* +2 graph-level integration pins (feature + contract paths) verifying
the signal lands on synthesized gap-fill tasks end-to-end
* Updated one existing test that asserted strict task identity post-
coverage; now allows the enriched copy
Full unit suite: 3566 passed, 61 skipped, 0 failed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(outcome-coverage): log signal-enrichment summary at INFO (#523 Slice A patch)
Adds a one-line INFO log when the enrichment pass changes the task
list ("Signal enrichment: N task(s) gained M signal criterion(s)
from K in-scope outcome(s)"). Silent on no-op so steady-state
idempotent re-runs don't spam logs.
Lets operators debug "did the signal land?" from logs alone, without
inspecting the task graph directly. Sibling to the existing
"Outcome coverage: score=..." line emitted by the wrapping
apply_outcome_coverage_to_*_graph helpers — together they let
operators correlate coverage score with enrichment effect for a
given run.
Tests
-----
* +1 unit test pinning the INFO log fires with the expected counts
* +1 unit test pinning silence on no-op idempotent re-runs
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat: runtime smoke gate accepts per-outcome verifications (#523 Slice B) (#525)
* feat(product-smoke): VerificationSpec + multi-spec runner foundation (#523 Slice B)
Pure addition — no behavior change to existing callers. Establishes
the primitive Slice B's smoke-gate update will consume.
A ``VerificationSpec`` is a shell command pinned to a
``UserOutcome.success_signal`` via the outcome's ``signal_id``. Integration
agents declare one spec per in-scope outcome at task completion; Marcus
runs each via the existing ``verify_deliverable`` primitive and aggregates.
Single-primitive design (per #523 issue body): Marcus does not learn
about curl, Playwright, pytest, or any other tooling family. The agent
picks tools appropriate to the deliverable shape; Marcus only knows how
to run shell commands and check exit codes.
Changes
-------
* ``VerificationSpec`` dataclass: ``signal_id``, ``command``,
``description``, optional ``readiness_probe``.
* ``VerificationsResult`` dataclass: aggregate + per-spec breakdown +
agent-facing blocker.
* ``verify_verification_specs(specs, cwd)``: runs each spec through
``verify_deliverable``, returns aggregate. Does NOT short-circuit on
failure so operators see the full diagnostic in one run.
* Two blocker renderers: ``_render_no_specs_blocker`` for empty input,
``_render_verifications_failure_blocker`` for the first failing spec
(names ``signal_id``, ``description``, ``command``, exit code, stderr
tail — Slice B acceptance criterion).
Tests
-----
* +8 unit tests covering: dataclass fields, empty input rejection,
all-pass aggregation, mixed-result failure rendering, readiness_probe
forwarding, signal_id fallback when description is empty, and
``to_dict`` telemetry serialisation.
Full module: 36 passed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(smoke-gate): route through verify_verification_specs when verifications declared (#523 Slice B)
Extends ``report_task_progress`` to accept a ``verifications`` list and
routes the smoke gate through the multi-spec runner when present.
Legacy ``start_command`` path is preserved unchanged for backward
compatibility.
Contract precedence:
* Non-empty ``verifications`` → runs ``verify_verification_specs``;
``start_command``/``readiness_probe`` are ignored.
* ``verifications`` absent or empty list → legacy single-command path
runs unchanged.
* Both absent on an integration task → existing missing-declaration
rejection still fires.
Rejection shape grows ``error="verifications_failed"`` for the new
path; the runner's blocker (signal_id, description, command, exit
code, stderr — Slice B acceptance criterion) flows through unchanged.
Changes
-------
* ``_run_product_smoke_gate`` accepts ``verifications``, coerces dicts
to ``VerificationSpec``, routes to multi-spec runner when non-empty.
* ``report_task_progress`` impl gains ``verifications`` parameter;
docstring documents the precedence rules.
* MCP server wrappers (both registration sites in
``src/marcus_mcp/server.py``) expose ``verifications``.
Tests
-----
* +5 unit tests covering precedence, passthrough, rejection shape,
dict-to-dataclass coercion, and empty-list fallthrough.
* Existing 12 smoke-gate tests pass unchanged — backward compat verified.
Full unit suite: 3581 passed, 61 skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(integration-task): accept user outcomes and surface them on the task (#523 Slice B)
Wires the outcomes extracted by ``outcome_extractor`` (issue #449)
into the integration verification task so the agent knows which
``UserOutcome`` records the smoke gate's coverage check (next commit)
will require ``VerificationSpec`` entries for.
Changes
-------
1. ``IntegrationTaskGenerator.create_integration_task`` accepts an
optional ``outcomes`` list. Filters to in-scope, stores
``in_scope_outcome_ids`` on ``Task.source_context``, and appends
a "Verifications required" section to the description listing each
outcome's id, action, and success_signal with a worked
``report_task_progress`` example.
2. ``enhance_project_with_integration`` accepts and forwards
``outcomes`` to the generator.
3. Contract-first call site in ``nlp_tools.py`` stashes
``prd_analysis.user_outcomes`` alongside the existing requirements
stash, reads it back at the enhance call, and clears on cleanup.
Feature-based wiring is deferred to a follow-up. The snake-game
class of failure (#523's motivating reproduction) ships via the
default ``contract_first`` decomposer, so wiring the default path is
what closes the original failure case.
Backward compatible: ``outcomes=None`` preserves legacy description
shape (no source_context, no Verifications section, no coverage
requirement at the smoke gate).
Tests
-----
* +7 unit tests covering in_scope IDs on source_context, out-of-scope
filtering, description growth (id/action/signal/example), legacy
``outcomes=None`` shape, empty list vs None distinction, all-out-of-
scope handling, and enhance_project_with_integration forwarding.
* Existing 55 IntegrationTaskGenerator tests pass unchanged.
Full unit suite: 3588 passed, 61 skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(smoke-gate): reject completion when verifications miss required outcomes (#523 Slice B)
Closes the loop on Slice B: when an integration task was created
with outcomes (commit 228ca479 stores ``in_scope_outcome_ids`` on
``Task.source_context``), the smoke gate rejects completion if any
required outcome has no matching ``VerificationSpec.signal_id`` in
the declared ``verifications`` list.
The check fires BEFORE any subprocess runs so an agent that forgot
a signal_id gets immediate, structural feedback without paying
verify-deliverable latency. Retrying with the same incomplete list
will fail the same way — the fix is "add the missing entries to
verifications," not "wait for a flaky subprocess."
Rejection shape gains ``error="verifications_missing_coverage"`` and
three new fields: ``missing_outcome_ids``, ``required_outcome_ids``,
and ``declared_signal_ids``. The blocker lists all three so the
agent can diff and fix in one pass.
Coverage rule does NOT fire when:
* ``task.source_context`` is ``None`` (legacy tasks — backward compat)
* ``in_scope_outcome_ids`` is missing or empty (trivially satisfied)
Tests
-----
* +5 unit tests covering coverage satisfied, missing-coverage
rejection before subprocess, all-missing edge case, legacy
``source_context=None`` bypass, and empty-list trivial pass.
Full unit suite: 3593 passed, 61 skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(smoke-gate): close verifications=None escape hatch (#523 Slice B, Kaia review on PR #525)
Slice B's coverage check fires only inside ``if verifications:``.
An agent could send ``verifications=None`` (or ``[]``) plus a working
legacy ``start_command``, fall through to ``verify_deliverable``,
and ship a project whose user-observable outcomes were never
verified — the exact failure mode Slice B was designed to prevent
(snake-game class).
Fix: preliminary check BEFORE the verifications/legacy branch
decision. When the integration task carries declared in-scope
outcomes on ``source_context["in_scope_outcome_ids"]`` AND
``verifications`` is empty/None, reject with
``error="verifications_required_but_missing"``.
Tasks WITHOUT declared outcomes bypass the check and use the legacy
path — backward compatibility preserved.
Also adds defensive ``isinstance(..., list)`` on the
``in_scope_outcome_ids`` read at both the new check and the existing
coverage check. Kanban providers that rehydrate ``source_context``
from JSON could in theory return a string here; ``list("o_play")``
would silently iterate characters and corrupt the required set.
Treat malformed values as wiring-absent.
Tests
-----
* +5 unit tests: outcomes+None rejects; outcomes+[] rejects;
outcomes+matching verifications proceeds (happy path);
legacy ``source_context=None`` bypasses check; malformed string
``in_scope_outcome_ids`` treated as absent.
Full unit suite: 3598 passed, 61 skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cost): token audit + per-role by_operation split (#527 Phase 1) (#528)
When the dashboard says a snake-game cost 125M tokens, today you can
see one number but no way to drill in: 99.85% of all tokens land in
a single bucket called `turn`. That bucket comes from
worker_ingester.py hardcoding `operation="turn"` for every agent
LLM call, and the chart shows it because by_operation aggregates
both planner and worker rows together.
This change makes the cost data answer the question "where did the
tokens go?" by:
1. Splitting by_operation by agent_role so planner rows (semantic
operations like parse_prd, decompose_prd) can be charted on
their own — the chart isn't polluted by the worker `turn` bucket
that dominates the total. Both run_summary and project_summary
now emit `role` on every by_operation slice.
2. Adding `run_audit(run_id)` and `project_audit(project_id)` —
token-attribution audits that answer "is every token I recorded
for this scope accounted for?" They check: (a) sum-of-by-role
tokens reconciles with grand-total tokens, (b) zero worker rows
are missing task_id, (c) zero worker rows are missing agent_id.
Both summaries now include the audit inline.
The architectural framing (issue #527, Simon decision `308a0951`):
`operation` is a planner-only attribution axis. Worker rows are
naturally attributed via task_id / agent_id / session_id / (in
Phase 2) tool_intent. The audit makes coverage explicit so the
dashboard can show "every token attributed" or surface the gap.
Tests:
- TestRunAudit (4 tests): reconciliation, zero-orphan, orphan
detection when a worker event lacks task_id, zero-state on
unknown run.
- TestProjectAudit (2 tests): project-scoped audit, scope
isolation between projects.
- TestByOperationSplitByRole (4 tests): every by_operation slice
carries role, planner/worker turn aggregate separately, audit
field present on both summary types.
- Added `pytestmark = pytest.mark.unit` to test_cost_aggregator.py
so the existing 34 tests + 10 new ones run in CI under
`pytest -m unit` (closes the same CI gap fixed for sibling files
in PR #525).
Baseline: 1400 passed, 1 skipped under `pytest -m unit`. mypy clean
on `src/cost_tracking/cost_aggregator.py`.
Ships with Cato Phase 1 dashboard work (sibling PR) — the new
backend fields are consumed by AuditBanner, TaskSpendPanel, and
the updated Oper…
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.
Summary
experimentstable →runs(resolving namespace collision with MLflow's separate experiment concept).pathcolumn (direct|marcus|posidonius|unknown) so the three entry points are distinguishable in cost analytics._runs_rename_migration()covers legacy DBs, fresh installs, and already-migrated DBs.create_projectnow records onerunsrow per successful call, pre-generatingrun_idat wrapper entry and rebinding on success. Defaultspath="direct"so unwrapped Direct MCP callers get the correct discriminator without code changes.experiments/spawn_agents.pysetspath="marcus"(Posidonius config overrides toposidonius).Why
"experiment_id" in cost-tracking was always about the path through a project — distinct invocations on a shared
project_id. MLflow uses the same word for an unrelated namespace, which has caused recurring confusion. This makes the cost-tracking model say what it means and the MLflow tool keep its own namespace cleanly.API renames (no compat aliases — pre-public)
CostStore.record_experiment→record_runCostAggregator.list_experiments→list_runs;experiment_summary→run_summaryTokenEvent.experiment_id/PlannerContext.experiment_id/AgentBinding.experiment_id→run_idget_cost_summaryMCP tool: argexperiment_id→run_id; response keyexperiments→runsExperiment→Run(with newpathfield)Test plan
pytest tests/unit/cost_tracking/(1297 unit tests pass, 1 skipped)pytest tests/unit/marcus_mcp/test_cost_tracking_tool.py tests/unit/marcus_mcp/test_create_project_run_recording.pymypy src/cost_tracking/ src/marcus_mcp/tools/cost_tracking.py src/marcus_mcp/tools/nlp.pycleanTestRunsRenameMigration)Related
🤖 Generated with Claude Code