q792602257
367fd0d412
Tests / Test passed: 855
Forced tool_choice ("any"/"required") makes both Anthropic and OpenAI
skip any text/thinking block before the tool call, which silently made
rationale and thinking always None despite the planner-reflection-history
change's capture code being correct. Switch the primary call to
tool_choice="auto" (Anthropic: type=auto, disable_parallel_tool_use=true;
OpenAI: "auto") so the model can emit its reflection text, and add a
one-time forced retry (Anthropic "any", OpenAI "required", thinking
disabled) if the model responds without a tool call, guaranteeing a step
never stalls. Also add OpenAI text_output capture from message.content,
which was never extracted before (Anthropic-only gap).
Update planner-reflection-history design.md/tasks.md to document the bug
found during the pending manual smoke test (task 8.5) and the fix (new
section 9).
63 lines
7.1 KiB
Markdown
63 lines
7.1 KiB
Markdown
## 1. ToolCallDecision — capture thinking and text output
|
|
|
|
- [x] 1.1 Add `thinking: str | None = None` and `text_output: str | None = None` fields to `ToolCallDecision` in `runtime/tool_calling_client.py`
|
|
- [x] 1.2 Update `_decision_from_anthropic_response()` to iterate all content blocks: collect first `thinking`-type block text into `thinking`, concatenate all `text`-type blocks into `text_output`, continue to `tool_use` as before
|
|
- [x] 1.3 Update `_decision_from_openai_response()` to extract `reasoning_content` from the first choice's message into `thinking` when present
|
|
- [x] 1.4 Add unit tests: Anthropic response with thinking block → `decision.thinking` populated; Anthropic response with text block → `decision.text_output` populated; response with only `tool_use` → both `None`; OpenAI response with `reasoning_content` → `decision.thinking` populated
|
|
|
|
## 2. PlannerConfig — extended thinking configuration
|
|
|
|
- [x] 2.1 Add `thinking_budget_tokens: int | None = None` to `PlannerConfig` in `runtime/planner_config.py`
|
|
- [x] 2.2 Add `AI_PLANNER_THINKING_BUDGET_TOKENS` env var constant and `_parse_thinking_budget()` helper; wire into `load_config()`
|
|
- [x] 2.3 Update `AnthropicToolCallingClient._create_message()`: when `thinking_budget_tokens` is set, add `thinking: {type: "enabled", budget_tokens: N}` to kwargs, add `interleaved-thinking-2025-05-14` beta header via `betas` parameter, and enforce `max_tokens >= thinking_budget_tokens + 1`
|
|
- [x] 2.4 Add unit tests: `load_config()` with env var set → `thinking_budget_tokens` parsed; `_create_message()` with budget set → kwargs include thinking param and beta header; without budget → no thinking param in kwargs
|
|
|
|
## 3. PlannedStep — carry rationale and thinking
|
|
|
|
- [x] 3.1 Add `rationale: str | None = None` and `thinking: str | None = None` to the `PlannedStep` dataclass in `runtime/planner.py`
|
|
- [x] 3.2 Update `AIPlanner.plan()` in `runtime/ai_planner.py`: populate `PlannedStep.rationale = decision.text_output` and `PlannedStep.thinking = decision.thinking`
|
|
|
|
## 4. Planner prompts — reflection instruction
|
|
|
|
- [x] 4.1 Update `PLANNER_SYSTEM_PROMPT` in `runtime/planner_prompts.py` to instruct the AI to output a short text block (1–2 sentences) before each tool call: first assessing whether the previous action achieved its intended effect (or noting "first step" if no history), then stating the intent of the current action
|
|
- [x] 4.2 ~~Verify via prompt review that the instruction is consistent with `tool_choice: {type: "any"}` (text blocks are already allowed before tool_use blocks — no API change needed)~~ — **superseded, see section 9**: this assumption was wrong; forced `tool_choice` suppresses text/thinking entirely
|
|
|
|
## 5. History format — compact rationale-based representation
|
|
|
|
- [x] 5.1 Update `_history_summary()` in `runtime/ai_planner.py` to return compact dicts `{page, rationale, action, success}` instead of `[event.to_dict() for event in world.history]`
|
|
- [x] 5.2 Add unit test: `_history_summary()` with a populated `WorldState` returns list of compact dicts without scene element data
|
|
|
|
## 6. WorldEvent — rationale, thinking, page fields
|
|
|
|
- [x] 6.1 Update `WorldEvent` in `world/models.py`: make `scene_summary` optional (`SemanticScene | Scene | None = None`), add `rationale: str | None = None`, `thinking: str | None = None`, `page: str | None = None`
|
|
- [x] 6.2 Update `WorldEvent.to_dict()` to include `rationale`, `thinking`, `page` fields; keep `scene_summary` serialisation for backward compatibility
|
|
- [x] 6.3 Update `WorldModel._append_history()` in `world/model.py` to pass `rationale=step.rationale`, `thinking=step.thinking`, `page=world_state.current_page` when constructing `WorldEvent`; `scene_summary` becomes optional (pass `None` or existing value as appropriate)
|
|
- [x] 6.4 Add unit tests: `WorldEvent` with rationale/thinking/page → `to_dict()` includes those fields; `WorldEvent` with all-None optional fields → readable without error; `_append_history()` picks up `current_page` from `WorldState` at time of call
|
|
|
|
## 7. Cloud DB — planner_decision_log extension
|
|
|
|
- [x] 7.1 Add `thinking` and `rationale` nullable TEXT columns to the `planner_decision_log` table in `packages/cloud-platform/cloud/db_models.py`
|
|
- [x] 7.2 Create a new Alembic migration (e.g. `0010_planner_decision_log_reflection`) adding the two nullable columns; verify it applies cleanly on both SQLite and PostgreSQL migration paths
|
|
- [x] 7.3 Add `thinking: str | None = None` and `rationale: str | None = None` to `PlannerDecisionRecord` in `packages/cloud-platform/cloud/internal_api/models.py`
|
|
- [x] 7.4 Update `record_planner_decision()` in `packages/cloud-platform/cloud/internal_api/api.py` to write `thinking` and `rationale` from the incoming `PlannerDecisionRecord` to the new columns
|
|
- [x] 7.5 Add unit tests: decision record with rationale/thinking → both stored; decision record with nulls → stored without error; existing rows queried → both columns read as NULL without error
|
|
|
|
## 8. End-to-end verification
|
|
|
|
- [x] 8.1 Run full non-integration test suite (`uv run --all-packages pytest -m "not integration"`) and confirm no regressions
|
|
- [x] 8.2 Run `ruff check` and `ruff format --check` on all modified files
|
|
- [x] 8.3 Run `python -m compileall` on modified packages
|
|
- [x] 8.4 Run `openspec validate --strict --change "planner-reflection-history"` and confirm all artifacts pass validation
|
|
- [x] 8.5 Manual smoke test (requires Host Agent + Appium + iPhone): ran a live task and found `rationale`/`thinking` were **always** `None` in `WorldEvent` history — root cause diagnosed and fixed in section 9 below
|
|
|
|
## 9. Bug fix — forced `tool_choice` was suppressing rationale/thinking (found via 8.5)
|
|
|
|
- [x] 9.1 `AnthropicToolCallingClient._create_message()`: add required `forced: bool` param; `tool_choice` is `{"type": "auto", "disable_parallel_tool_use": True}` when `forced=False`, `{"type": "any", "disable_parallel_tool_use": True}` when `forced=True`; thinking/`betas` kwargs only added when `forced=False`
|
|
- [x] 9.2 `AnthropicToolCallingClient.decide()`: call with `forced=False` first; if `_anthropic_response_has_tool_use(response)` is `False`, retry once with `forced=True`; parse whichever response has the tool call
|
|
- [x] 9.3 `OpenAIToolCallingClient._create_completion()`: add required `forced: bool` param; `tool_choice` is `"auto"` when `forced=False`, `"required"` when `forced=True`
|
|
- [x] 9.4 `OpenAIToolCallingClient.decide()`: same auto-then-forced-retry pattern using `_openai_response_has_tool_call()`
|
|
- [x] 9.5 `_decision_from_openai_response()`: extract `message.content` into `text_output` (previously never captured for OpenAI, forced or not)
|
|
- [x] 9.6 Add/rename unit tests in `tests/test_tool_calling_client.py` covering: default request uses `tool_choice: "auto"`; retry sequence when first response has no tool call (Anthropic and OpenAI); thinking/`betas` dropped on the forced retry; OpenAI `text_output` capture
|
|
- [x] 9.7 Update `design.md` (D1 correction + new D8) and this file to document the bug and fix
|
|
- [x] 9.8 Re-run `uv run --all-packages pytest -m "not integration"`, `ruff check`/`ruff format --check`, `python -m compileall`, and `openspec validate --strict --change "planner-reflection-history"` after the fix
|