Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/ai_company/budget/category_analytics.py`:
- Around line 116-135: OrchestrationRatio is dropping the uncategorized_tokens
bucket from its public model so callers cannot tell if a zero ratio was due to
true zero overhead or uncategorized traffic; add an uncategorized_tokens: int =
Field(ge=0, description="Uncategorized tokens") attribute to the
OrchestrationRatio model (and mirror the same change in the similar
model/section referenced around lines 204-233) so the denominator and token
breakdown are exposed; update any constructor/factory that builds
OrchestrationRatio to populate this new field from the computed
uncategorized_tokens value.
- Around line 8-20: This module lacks the observability hook: add "from
ai_company.observability import get_logger" at the top and declare "logger =
get_logger(__name__)" so the file-level logger is available; then instrument the
existing business logic functions build_category_breakdown and
compute_orchestration_ratio to emit events using the domain event constants from
ai_company.observability.events (import the appropriate event names) rather than
raw strings, ensuring calls to logger.info/debug/error use those constants and
that logging occurs at entry/exit and on error paths; place the imports
alongside other top-level imports and add the logger declaration near them so
all functions in this file (including lines ~138-247) use the repo-wide
observability contract.

In `@src/ai_company/budget/coordination_metrics.py`:
- Around line 86-90: Update the error-rate field validators and the computation
to enforce a max of 1.0: change the Pydantic Field on error_rate_mas to include
le=1.0 (keep ge=0.0) and change error_rate_sas to use ge=0.0, le=1.0 (instead of
gt=0), and then in compute_error_amplification() defensively clamp both inputs
to min(value, 1.0) before doing any math so out-of-range inputs can't produce
nonsensical amplification ratios; reference error_rate_mas, error_rate_sas, and
compute_error_amplification when making the changes.
- Around line 1-12: Add logging to this business-logic module by importing
get_logger and initializing logger = get_logger(__name__), then update every
invalid-input branch in the coordination metric helpers (functions efficiency,
overhead, error_amplification, message_density, redundancy_rate) to call
logger.warning or logger.error with contextual details (input values and reason)
immediately before raising ValueError; ensure the import line "from
ai_company.observability import get_logger" is added and that each bad-input
path logs at WARNING/ERROR level with a clear message including the function
name and offending parameters.

In `@src/ai_company/budget/tracker.py`:
- Around line 243-244: The public query entry points that accept agent_id and
task_id currently allow blank/whitespace strings which then propagate into
_filter_records; change the parameter types from str | None to
core.types.NotBlankStr | None for all identifier/name parameters (e.g., the
methods taking agent_id: str | None, task_id: str | None) and add an upfront
normalization/validation step that rejects or normalizes purely-blank values
(trim and treat "" as None or raise) before calling _filter_records; apply the
same change to the other affected signatures/paths mentioned (the other method
overloads that accept task_id/agent_id) so blank-only inputs are validated at
the system boundary.

In `@src/ai_company/engine/cost_recording.py`:
- Around line 76-78: The code is currently persisting turns even when
turn.call_category is None; modify the recording path in cost_recording.py so
that before creating/saving the record (the block that sets
call_category=turn.call_category and timestamp=datetime.now(UTC)) you check if
turn.call_category is None and handle it instead of writing: either log an
error/warning with context (e.g., include turn id or session) and skip
recording, or raise an exception to fail fast; update the surrounding function
that constructs the record to early-return/raise when call_category is missing
so no uncategorized cost rows are persisted.

In `@src/ai_company/engine/loop_helpers.py`:
- Around line 258-274: In the branch where tool_invoker is None (the if
tool_invoker is None check), clear the recorded tool calls before logging the
error and returning so the turn history and ExecutionResult.total_tool_calls
aren’t inflated; specifically, empty response.tool_calls (or call
tool_calls_made.clear() if that list variable is used elsewhere) immediately
before the logger.error(...) / build_result(...) sequence that returns
TerminationReason.ERROR. Ensure the clearing happens before the error log and
return so no stale tool call entries remain.

In `@src/ai_company/engine/plan_execute_loop.py`:
- Line 531: The TurnRecord created by make_turn_record is missing call_category,
so planner/replanner turns are uncategorized; update the make_turn_record calls
that append to turns (the ones in the planner/replanner code path and the
per-step executor path) to pass call_category="SYSTEM" for planning/replanning
turns and call_category="PRODUCTIVE" for per-step execution turns so TurnRecord
instances are correctly categorized for orchestration breakdowns.
- Around line 274-279: The code updates an immutable ExecutionPlan via
_update_step_status but never writes the replacement back into all_plans, so
later serialization/_finalize and _handle_step_failure operate on stale objects;
update every site that calls _update_step_status (e.g., the block guarded by
step_ok and the other uses around the ranges that call _update_step_status and
_handle_step_failure) to assign the returned ExecutionPlan into the original
all_plans entry (use the same index/key you used when appending the original
plan), and ensure any local variable `plan` is also replaced with that returned
object before proceeding to _finalize() or returning an ExecutionResult so all
subsequent logic operates on the new immutable instance.
- Around line 667-676: Replace using response.finish_reason as the success check
with an explicit step result/status from the LLM response (e.g.,
response.step_result or response.success) so that "STOP" vs "MAX_TOKENS" are
treated only as generation end signals and not as step success; keep the
existing logger.warning(EXECUTION_PLAN_STEP_COMPLETE, ..., truncated=True) when
response.finish_reason == FinishReason.MAX_TOKENS but determine success by
calling/inspecting the explicit field used by self._assess_step_success (or by
adapting _assess_step_success to accept the explicit step result), and apply the
same change to the other identical block around lines 870-882.
- Around line 531-535: The code calls response_to_message(response)
unconditionally which raises when CompletionResponse finished with finish_reason
'error' or 'content_filter' (no content/tool calls); change the flow in the
block that uses make_turn_record and ctx.with_turn_completed to first detect
when response.finish_reason is 'error' or 'content_filter' or when response has
no content and no tool calls, and in that case: (1) avoid calling
response_to_message(), (2) create and return a clean ExecutionResult
representing planner failure (including response.finish_reason and any
error/details from CompletionResponse), and (3) still call
ctx.with_turn_completed(response.usage, None) or otherwise mark the turn
completed without an assistant ChatMessage; keep references to make_turn_record,
ctx.with_turn_completed, response_to_message, CompletionResponse, and
ExecutionResult so the fix is easy to locate.
- Around line 105-913: The module and several long methods violate
size/complexity rules: split parsing and replanning orchestration into separate
helper modules/classes so execute() and _run_steps() fall under 50 lines and the
file is <800 lines. Extract the plan parsing logic (_parse_plan,
_parse_json_plan, _parse_text_plan, _data_to_plan, _extract_task_summary) into a
new PlanParser class/module and replace calls to self._parse_* with a PlanParser
instance; likewise extract planner orchestration and replan creation
(_generate_plan, _call_planner, _replan, _handle_step_failure) into a
PlannerOrchestrator helper that exposes methods used by execute()/_run_steps;
update PlanExecuteLoop to delegate to these helpers so its execute() and
_run_steps() only orchestrate high-level control flow.

In `@src/ai_company/engine/react_loop.py`:
- Around line 24-35: make_turn_record is currently called without a category in
react_loop.py so ReAct turns are recorded as uncategorized; update the calls to
make_turn_record in this module (both ReAct branch and the other occurrence) to
pass an explicit call_category (e.g., call_category="productive" or "react") so
ReAct turns are counted in the per-category breakdowns.

In `@tests/unit/engine/test_cost_recording.py`:
- Around line 141-155: Add a third case for the COORDINATION category to the
call-category propagation test by converting test_call_category_propagated into
a parametrized test (use pytest.mark.parametrize) over call_category values
including LLMCallCategory.PRODUCTIVE, LLMCallCategory.SYSTEM, and
LLMCallCategory.COORDINATION; for each param, construct the turn via
_turn(call_category=...) and run record_execution_costs with the same
_FakeTracker, then assert the tracker.records entry preserves the passed
call_category (ensure you reference the existing test_call_category_propagated,
record_execution_costs, _turn, and _FakeTracker symbols).

In `@tests/unit/engine/test_plan_execute_loop.py`:
- Around line 470-472: The test currently only checks provider.call_count == 2
which doesn't guarantee different model IDs were used; instead inspect the
provider's recorded calls (e.g., provider.recorded_calls or provider.calls) and
assert that the first call (planner invocation) used model id "test-planner-001"
and the second call (executor invocation) used model id "test-executor-001",
while keeping the existing assertion that result.termination_reason ==
TerminationReason.COMPLETED; update the assertions to check the request payloads
or model_id fields of the two recorded provider calls rather than just the call
count.
- Around line 435-439: The test currently allows both
TerminationReason.MAX_TURNS and TerminationReason.COMPLETED for
result.termination_reason, which masks a false positive when running with
max_turns=2 against a 3-step plan; change the assertion to only accept
TerminationReason.MAX_TURNS (e.g., assert result.termination_reason ==
TerminationReason.MAX_TURNS) so the test correctly verifies max-turn termination
for the given setup and keep references to result.termination_reason,
TerminationReason.MAX_TURNS, TerminationReason.COMPLETED and the max_turns=2 /
3-step plan in your reasoning while editing.
- Around line 267-297: The test uses FinishReason.ERROR which is intercepted by
check_response_errors() before reaching _handle_step_failure(), so max_replans=0
never gets exercised; change the mocked provider sequence so the second response
is a non-error CompletionResponse (e.g., FinishReason.FINISHED) whose content
decodes into a step result that causes _handle_step_failure() to run (i.e., a
successful LLM completion that represents a failed step execution), then keep
PlanExecuteLoop(PlanExecuteConfig(max_replans=0)) and assert the
exhausted-replan behavior (TerminationReason.ERROR and loop_type) to exercise
the max-replans branch.

---

Outside diff comments:
In `@DESIGN_SPEC.md`:
- Around line 1567-1575: The prose in §10.5 is inconsistent with the
implementation: the orchestration ratio used in code is (coordination + system)
/ total but the text defines it as coordination / total. Update the prose and
any alert guidance in §10.5 to define the orchestration ratio as (coordination +
system) / total (and adjust explanatory text about thresholds/alerts
accordingly), referencing the CostRecord categories (`coordination` and
`system`) so readers understand the formula matches the implementation.

In `@src/ai_company/engine/cost_recording.py`:
- Around line 68-75: CostRecord entries are being stamped with
identity.model.provider and identity.model.model_id even though ExecutionResult
turns may come from different models; update the record creation in CostRecord
construction to use the model metadata attached to each turn (e.g.,
turn.model.provider and turn.model.model_id), falling back to
identity.model.provider/identity.model.model_id only if the turn lacks its own
model info, so each CostRecord accurately reflects the per-call model used by
ExecutionResult/turn.

In `@src/ai_company/engine/react_loop.py`:
- Around line 230-249: The code treats FinishReason.MAX_TOKENS as a successful
completion; change it to return and log a non-success termination (e.g.,
TerminationReason.TRUNCATED or another appropriate non-success value) so callers
can detect truncation and trigger fallback/retry. Specifically, in the branch
checking response.finish_reason == FinishReason.MAX_TOKENS update the
logger.warning call's reason field to the non-success TerminationReason and pass
that same TerminationReason (instead of TerminationReason.COMPLETED) into
build_result(ctx, ..., turns), keeping truncated=True and execution/context
metadata (ctx.execution_id, turns) intact.