# Track Specification: fix_test_failures_20260624

## Overview

3 surgical fixes to make the full test suite green after the polish merge. The 14 failing tests cluster into 3 root causes; each is a single-file change.

## Current State Audit (commit 4c2bb3c9, master)

3 tier failures from `uv run python scripts/run_tests_batched.py`:

### Root cause 1: NormalizedResponse API mismatch (12 tests)

`src/openai_schemas.py:75-80` defines `NormalizedResponse` with 4 fields: `text`, `tool_calls`, `usage: UsageStats`, `raw_response`. The `@dataclass` auto-generates `__init__` requiring `usage: UsageStats`.

But 12 tests + 1 production site at `src/ai_client.py:908` call it with the OLD flat-kwarg API:
```python
NormalizedResponse(
    text=..., tool_calls=...,
    usage_input_tokens=10, usage_output_tokens=5,
    usage_cache_read_tokens=0, usage_cache_creation_tokens=0,
    raw_response=None,
)
```

**Failing tests:**
- `tests/test_ai_client_cli.py::test_ai_client_send_gemini_cli`
- `tests/test_ai_client_tool_loop.py` (5 tests)
- `tests/test_ai_client_tool_loop_builder.py::test_run_with_tool_loop_calls_request_builder_each_round`
- `tests/test_ai_client_tool_loop_send_func.py` (2 tests)
- `tests/test_gemini_cli_integration.py::test_gemini_cli_full_integration`
- `tests/test_gemini_cli_parity_regression.py::test_send_invokes_adapter_send`
- `tests/test_gemini_cli_edge_cases.py::test_gemini_cli_loop_termination`

**Fix:** add a custom `__init__` to `NormalizedResponse` (using `@dataclass(frozen=True, init=False)`) that accepts BOTH signatures. The legacy flat kwargs are auto-converted to a `UsageStats` instance.

### Root cause 2: Session dataclass frozen (1 test)

`tests/test_auto_whitelist.py:20` does `reg.data[session_id]["whitelisted"] = True`. `Session` is `@dataclass(frozen=True)` so attribute assignment fails.

**Failing test:** `tests/test_auto_whitelist.py::test_auto_whitelist_keywords`

**Fix:** update the test to use `dataclasses.replace(reg.data[session_id], whitelisted=True)` (or equivalent setter on the registry).

### Root cause 3: Toggle race (1 test)

`tests/test_command_palette_sim.py::test_palette_starts_hidden` calls `_toggle_command_palette` (non-deterministic) and asserts `show_command_palette is False`. The toggle is open-if-closed, close-if-open, so the assertion depends on prior state.

**Failing test:** `tests/test_command_palette_sim.py::test_palette_starts_hidden`

**Fix:** call a deterministic close callback (e.g., `_close_command_palette`) instead of toggle. If no such callback exists, push the toggle twice (open then close).

## Goals

| ID | Goal | Acceptance |
|---|---|---|
| G1 | `NormalizedResponse` accepts both the new `usage: UsageStats` and the legacy flat `usage_input_tokens=...` kwargs | All 12 affected tests pass; existing usage in `src/ai_client.py:908` continues to work |
| G2 | `test_auto_whitelist_keywords` no longer attempts to mutate a frozen `Session` | The test passes; no production code change |
| G3 | `test_palette_starts_hidden` uses a deterministic close path | The test passes; the test no longer depends on prior fixture state |
| G4 | Full test suite (`scripts/run_tests_batched.py`) is green | 0 failed, 0 new failures vs baseline |

## Non-Goals

- Refactoring `NormalizedResponse` to remove the legacy support (the fix is additive; legacy kwargs continue to work)
- Migrating `src/ai_client.py:908` from the legacy API to the new API (out of scope; the fix makes both work)
- Adding new tests beyond fixing the 14 failing ones
- Touching any other files

## Functional Requirements

### FR1: NormalizedResponse dual-signature __init__

`src/openai_schemas.py` line 75-80: change `@dataclass(frozen=True)` to `@dataclass(frozen=True, init=False)` and add a custom `__init__` that:
- Accepts all 4 canonical kwargs: `text`, `tool_calls`, `usage`, `raw_response` (with defaults)
- Accepts all 4 legacy flat kwargs: `usage_input_tokens`, `usage_output_tokens`, `usage_cache_read_tokens`, `usage_cache_creation_tokens` (all `Optional[int]`, default `None`)
- If `usage` is None AND any legacy flat kwarg is non-None, builds a `UsageStats` from the legacy kwargs
- If both `usage` and legacy kwargs are None, builds `UsageStats(0, 0)`
- Uses `object.__setattr__` for all 4 field assignments (required because `frozen=True`)

### FR2: test_auto_whitelist_keywords uses replace

`tests/test_auto_whitelist.py:20`: change `reg.data[session_id]["whitelisted"] = True` to use `dataclasses.replace(reg.data[session_id], whitelisted=True)` and assign back to `reg.data[session_id]`.

### FR3: test_palette_starts_hidden uses deterministic close

`tests/test_command_palette_sim.py:38`: change the `_toggle_command_palette` callback to a deterministic close. Options (in preference order):
- (a) Call `_close_command_palette` directly if it exists in `_predefined_callbacks`
- (b) If no close callback exists, push `_toggle_command_palette` once, wait for state to settle, then push it again to guarantee close
- The test must end with `show_command_palette is False` regardless of starting state

## Non-Functional Requirements

- NFR1: 1-space indentation
- NFR2: CRLF line endings (Windows)
- NFR3: No comments in source code
- NFR4: Per-task atomic commits
- NFR5: No new pip dependencies
- NFR6: 0 new test failures introduced

## Architecture Reference

- `conductor/code_styleguides/error_handling.md` — `Result[T]` convention (no changes needed; no Result changes in this fix)
- `conductor/code_styleguides/data_oriented_design.md` — DOD reference (the NormalizedResponse fix is purely additive)
- `src/openai_schemas.py:75-93` — the file being modified (FR1)
- `tests/test_auto_whitelist.py:20` — the file being modified (FR2)
- `tests/test_command_palette_sim.py:38` — the file being modified (FR3)

## Out of Scope

- Migrating `src/ai_client.py:908` from the legacy API to `usage: UsageStats(...)` (the additive fix makes both work; explicit migration is a separate concern)
- Adding regression tests for the new NormalizedResponse dual-signature behavior (the 12 existing tests ARE the regression test)
- Refactoring Session to not be frozen (the test fix is simpler)
- Fixing the root cause of `_toggle_command_palette` being non-deterministic (the test fix is simpler)
- Any changes to test infrastructure or conftest

## Verification Criteria (Definition of Done)

| # | Criterion | Verification command |
|---|---|---|
| VC1 | The 12 NormalizedResponse tests pass | `uv run pytest tests/test_ai_client_cli.py tests/test_ai_client_tool_loop.py tests/test_ai_client_tool_loop_builder.py tests/test_ai_client_tool_loop_send_func.py tests/test_gemini_cli_integration.py tests/test_gemini_cli_parity_regression.py tests/test_gemini_cli_edge_cases.py -v` |
| VC2 | `test_auto_whitelist_keywords` passes | `uv run pytest tests/test_auto_whitelist.py -v` |
| VC3 | `test_palette_starts_hidden` passes | `uv run pytest tests/test_command_palette_sim.py -v` |
| VC4 | Full batched test suite is green | `uv run python scripts/run_tests_batched.py` (all 11 tiers PASS) |
| VC5 | The 4 mandatory audit gates remain clean | `uv run python scripts/audit_exception_handling.py` (informational); `uv run python scripts/audit_weak_types.py`; `uv run python scripts/audit_main_thread_imports.py`; `uv run python scripts/audit_no_models_config_io.py` |
| VC6 | No new test failures introduced | diff `uv run python scripts/run_tests_batched.py` summary table against pre-fix baseline (the 14 known failures are now green; no new failures added) |

## Risks

| # | Risk | Likelihood | Mitigation |
|---|---|---|---|
| R1 | The custom `__init__` with `init=False` breaks other NormalizedResponse callers that pass positional args | low | The class is only constructed in 1 production site (`src/ai_client.py:908`) and the 12 test sites, all of which use kwargs. Verify by running the 12 tests. |
| R2 | The `dataclasses.replace()` for Session assignment to `reg.data[session_id]` mutates the registry's internal state in a way that breaks other tests | low | The registry stores sessions in a dict. `reg.data[session_id] = replace(...)` is a normal dict assignment, equivalent to the old `reg.data[session_id]["whitelisted"] = True` semantics. |
| R3 | A deterministic close callback doesn't exist in `_predefined_callbacks` | medium | Fallback option: push toggle twice. Verify by checking the available callbacks in the API hooks registry. |
| R4 | `UsageStats(0, 0)` syntax (when no kwargs and no legacy) is wrong if UsageStats requires explicit args | low | Verified: `UsageStats` has `input_tokens: int` and `output_tokens: int` as required, with `cache_*_tokens: int = 0` defaults. `UsageStats(0, 0)` is valid. |