# Considering-state + transport timeout Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** While a model call is in flight, the client shows a rotating in-character "the Master considers…" indicator instead of a frozen surface, and a hung call gives way to the authored fallback instead of spinning forever.

**Architecture:** All client-side, no server change. A pure `ConsideringPhrases` (cycles an authored phrase pool loaded from content, robust to a missing file) drives a thin `ConsideringIndicator` node (owns a `Timer`, rotates a `Label`). `ProxyConfig` gains a request-timeout setting; `HttpTransport` applies it to its `HTTPRequest`, so a hang maps through the existing `RESULT_TIMEOUT → failed() → degrade` path. Both harnesses show the indicator around their `await`.

**Tech Stack:** Godot 4.7 / GDScript / GUT.

## Global Constraints

- **§2 — this is text/UI only.** No game state, no AI, no canon-log mutation. The phrases are authored content.
- **§13 — loading is content.** Phrases live in `client/content/considering.json`, loaded like the fallback lines; a missing/malformed file degrades to one hardcoded constant so the indicator itself cannot fail.
- **JSON-parse GOTCHA:** in any code the tests touch, parse with `var j := JSON.new(); j.parse(text)` and check `!= OK` — NOT `JSON.parse_string()` (the project's `.gutconfig.json` `failure_error_types` turns its engine error into a false test failure).
- **Timeout default 35.0s** — just above the server's `OLLAMA_TIMEOUT_SECONDS=30`, so a slow *model* surfaces the server's own error first while a fully-hung proxy is still bounded. A hang reuses the existing `HttpTransport` `result != RESULT_SUCCESS → DmResponse.failed()` path (already covered by `test_transport_failure_degrades`); do not add a new service-level degrade test.
- **Rotation index is instance state, not reset per call** (spec decision 4): one `ConsideringPhrases`/`ConsideringIndicator` instance persists on the harness; `next()` advances a running index that `start()`/`stop()` never reset.
- **Branch:** all work on `feature/considering-state`, branched off `dev`. Never commit to `dev`/`master` directly (CLAUDE.md §18); the human owns the `dev` merge and push.
- **Client tests:** `cd client && ./run_tests.sh` (full) or `./run_tests.sh -gtest=res://tests/unit/test_X.gd` (one file). A new `class_name` may need `cd client && godot --headless --import` once before it resolves.

---

## File Structure

- `client/scripts/ui/considering_phrases.gd` — CREATE: pure phrase-pool cycler.
- `client/content/considering.json` — CREATE: authored phrase pool.
- `client/scripts/net/proxy_config.gd` — MODIFY: add `request_timeout_seconds()`.
- `client/scripts/net/http_transport.gd` — MODIFY: set `_http.timeout`.
- `client/scripts/ui/considering_indicator.gd` — CREATE: node shim (Timer + Label).
- `client/scripts/harness/narrate_harness.gd` — MODIFY: show indicator around the await.
- `client/scripts/harness/npc_harness.gd` — MODIFY: add a status Label + show indicator.
- Tests: `client/tests/unit/test_considering_phrases.gd` (create), `client/tests/unit/test_considering_indicator.gd` (create), `client/tests/unit/test_net_primitives.gd` (add ProxyConfig + HttpTransport timeout tests).

---

## Task 1: `ConsideringPhrases` (pure) + authored pool

**Files:**
- Create: `client/scripts/ui/considering_phrases.gd`
- Create: `client/content/considering.json`
- Test: `client/tests/unit/test_considering_phrases.gd`

**Interfaces:**
- Produces: `ConsideringPhrases.new(path := PATH)`; `next() -> String` (returns the pool entry at a running index and advances it, wrapping; `LAST_RESORT` when the pool is empty/missing). Constants `PATH`, `LAST_RESORT`.

- [ ] **Step 1: Write the failing test**

```gdscript
# client/tests/unit/test_considering_phrases.gd
extends "res://addons/gut/test.gd"

const ConsideringPhrases = preload("res://scripts/ui/considering_phrases.gd")


func test_missing_file_returns_last_resort():
	var p = ConsideringPhrases.new("res://content/does_not_exist.json")
	assert_eq(p.next(), ConsideringPhrases.LAST_RESORT)
	assert_eq(p.next(), ConsideringPhrases.LAST_RESORT)


func test_cycles_pool_in_order_and_wraps():
	# Real authored file has at least 3 lines; assert order + wrap on it.
	var p = ConsideringPhrases.new()  # default path — real content
	var first = p.next()
	var second = p.next()
	assert_ne(first, "")
	assert_ne(first, second)  # advances, not stuck
	# Drain a full cycle back to `first` to prove it wraps (bounded loop).
	var seen := [first, second]
	for i in range(20):
		var n = p.next()
		if n == first:
			return  # wrapped — success
		seen.append(n)
	assert_true(false, "did not wrap within 22 calls: %s" % [seen])


func test_index_persists_across_calls_not_reset():
	var p = ConsideringPhrases.new()
	var a = p.next()
	var b = p.next()
	assert_ne(a, b)  # second call did not re-return the first phrase
```

- [ ] **Step 2: Run to verify it fails**

Run: `cd client && ./run_tests.sh -gtest=res://tests/unit/test_considering_phrases.gd`
Expected: FAIL — script does not exist.

- [ ] **Step 3: Implement**

Create `client/content/considering.json`:

```json
{
  "considering": [
    "The Master considers your words.",
    "Somewhere, dice are gathered.",
    "The tale weighs its next turn.",
    "The dark shifts, thinking.",
    "A page is turned, unhurried."
  ]
}
```

Create `client/scripts/ui/considering_phrases.gd`:

```gdscript
class_name ConsideringPhrases
extends RefCounted
## Authored "the GM is thinking" lines (charter §13 — even loading is content).
## Pure: cycles a phrase pool in order on a running index that is instance state
## (spec decision 4 — the indicator reuses one instance so the first phrase varies
## turn to turn without randomness). A missing/malformed file degrades to one
## hardcoded constant, so the indicator itself cannot fail.

const PATH := "res://content/considering.json"
const LAST_RESORT := "The Master considers your words."

var _pool: Array = []
var _index: int = 0


func _init(path := PATH) -> void:
	_pool = _load(path)


func next() -> String:
	if _pool.is_empty():
		return LAST_RESORT
	var phrase := str(_pool[_index % _pool.size()])
	_index += 1
	return phrase


static func _load(path: String) -> Array:
	if not FileAccess.file_exists(path):
		return []
	var text := FileAccess.get_file_as_string(path)
	# Instance parse() (not JSON.parse_string) so malformed input returns an
	# Error code instead of the engine error the test config promotes to a fail.
	var json := JSON.new()
	if json.parse(text) != OK:
		return []
	var data = json.get_data()
	if typeof(data) != TYPE_DICTIONARY:
		return []
	var lines = data.get("considering", [])
	return lines if typeof(lines) == TYPE_ARRAY else []
```

- [ ] **Step 4: Run to verify it passes**

Run: `cd client && godot --headless --import >/dev/null 2>&1; ./run_tests.sh -gtest=res://tests/unit/test_considering_phrases.gd`
Expected: PASS (3 tests).

- [ ] **Step 5: Commit**

```bash
git add client/scripts/ui/considering_phrases.gd client/content/considering.json client/tests/unit/test_considering_phrases.gd
git commit -m "feat(client): ConsideringPhrases — authored rotating think-lines (§13)"
```

---

## Task 2: `ProxyConfig.request_timeout_seconds`

**Files:**
- Modify: `client/scripts/net/proxy_config.gd`
- Test: `client/tests/unit/test_net_primitives.gd`

**Interfaces:**
- Produces: `ProxyConfig.request_timeout_seconds() -> float` (a `ProjectSettings` key, default `TIMEOUT_DEFAULT := 35.0`; a non-positive/invalid value falls back to the default so a hang can never be re-enabled). Constants `TIMEOUT_SETTING`, `TIMEOUT_DEFAULT`.

- [ ] **Step 1: Write the failing test**

Add to `client/tests/unit/test_net_primitives.gd`:

```gdscript
func test_request_timeout_default():
	var ProxyConfig = preload("res://scripts/net/proxy_config.gd")
	if ProjectSettings.has_setting(ProxyConfig.TIMEOUT_SETTING):
		ProjectSettings.clear(ProxyConfig.TIMEOUT_SETTING)
	assert_eq(ProxyConfig.request_timeout_seconds(), ProxyConfig.TIMEOUT_DEFAULT)


func test_request_timeout_override_and_reject_nonpositive():
	var ProxyConfig = preload("res://scripts/net/proxy_config.gd")
	ProjectSettings.set_setting(ProxyConfig.TIMEOUT_SETTING, 12.5)
	assert_eq(ProxyConfig.request_timeout_seconds(), 12.5)
	# A non-positive override must NOT re-enable the hang — falls back to default.
	ProjectSettings.set_setting(ProxyConfig.TIMEOUT_SETTING, 0.0)
	assert_eq(ProxyConfig.request_timeout_seconds(), ProxyConfig.TIMEOUT_DEFAULT)
	ProjectSettings.clear(ProxyConfig.TIMEOUT_SETTING)
```

- [ ] **Step 2: Run to verify it fails**

Run: `cd client && ./run_tests.sh -gtest=res://tests/unit/test_net_primitives.gd`
Expected: FAIL — `request_timeout_seconds` / `TIMEOUT_SETTING` not found.

- [ ] **Step 3: Implement**

In `client/scripts/net/proxy_config.gd`, add after the `base_url` block:

```gdscript
const TIMEOUT_SETTING := "coc_rpg/proxy_request_timeout_seconds"
const TIMEOUT_DEFAULT := 35.0


static func request_timeout_seconds() -> float:
	# Just above the server's OLLAMA_TIMEOUT_SECONDS (30) so a slow model surfaces
	# the server's own error first, while a fully-hung proxy is still bounded. A
	# non-positive value would re-enable the infinite hang, so reject it.
	var v := float(ProjectSettings.get_setting(TIMEOUT_SETTING, TIMEOUT_DEFAULT))
	return v if v > 0.0 else TIMEOUT_DEFAULT
```

- [ ] **Step 4: Run to verify it passes**

Run: `cd client && ./run_tests.sh -gtest=res://tests/unit/test_net_primitives.gd`
Expected: PASS.

- [ ] **Step 5: Commit**

```bash
git add client/scripts/net/proxy_config.gd client/tests/unit/test_net_primitives.gd
git commit -m "feat(client): ProxyConfig.request_timeout_seconds (default 35s, reject <=0)"
```

---

## Task 3: `HttpTransport` applies the request timeout

**Files:**
- Modify: `client/scripts/net/http_transport.gd`
- Test: `client/tests/unit/test_net_primitives.gd`

**Interfaces:**
- Consumes: `ProxyConfig.request_timeout_seconds()` (Task 2).
- Produces: constructing `HttpTransport.new(http)` sets `http.timeout` to the configured value. Closes the `_http.timeout=0` gap; a hang now completes with `RESULT_TIMEOUT`, which the existing code maps to `DmResponse.failed()`.

- [ ] **Step 1: Write the failing test**

Add to `client/tests/unit/test_net_primitives.gd`:

```gdscript
func test_http_transport_sets_request_timeout():
	var HttpTransport = preload("res://scripts/net/http_transport.gd")
	var ProxyConfig = preload("res://scripts/net/proxy_config.gd")
	var http := HTTPRequest.new()
	autofree(http)
	HttpTransport.new(http)
	assert_eq(http.timeout, ProxyConfig.request_timeout_seconds())
	assert_gt(http.timeout, 0.0)  # the whole point — no longer disabled
```

- [ ] **Step 2: Run to verify it fails**

Run: `cd client && ./run_tests.sh -gtest=res://tests/unit/test_net_primitives.gd`
Expected: FAIL — `http.timeout` is `0.0` (the default), not the configured value.

- [ ] **Step 3: Implement**

In `client/scripts/net/http_transport.gd`, in `_init` (it already `preload`s `ProxyConfig`), set the timeout:

```gdscript
func _init(http: HTTPRequest) -> void:
	_http = http
	# Bound a hung proxy so the request completes with RESULT_TIMEOUT (→ failed()
	# → the service degrades) instead of never returning. Without this the
	# considering-state indicator would rotate forever on a hang.
	_http.timeout = ProxyConfig.request_timeout_seconds()
```

(Keep the rest of `_init`/`post_json` unchanged.)

- [ ] **Step 4: Run to verify it passes**

Run: `cd client && ./run_tests.sh -gtest=res://tests/unit/test_net_primitives.gd`
Expected: PASS.

- [ ] **Step 5: Commit**

```bash
git add client/scripts/net/http_transport.gd client/tests/unit/test_net_primitives.gd
git commit -m "fix(client): HttpTransport sets request timeout (no more infinite hang)"
```

---

## Task 4: `ConsideringIndicator` (node shim)

**Files:**
- Create: `client/scripts/ui/considering_indicator.gd`
- Test: `client/tests/unit/test_considering_indicator.gd`

**Interfaces:**
- Consumes: `ConsideringPhrases` (Task 1).
- Produces: `ConsideringIndicator.new(phrases := null)` (defaults to a fresh `ConsideringPhrases`); `start(label: Label)` shows the first phrase immediately and starts a repeating `Timer` (`ROTATE_SECONDS := 2.0`) that rotates the label; `stop()` stops the timer and clears the label. Extends `Node` (needs a `Timer` in the tree). Holds one `ConsideringPhrases` for its lifetime.

- [ ] **Step 1: Write the failing test**

```gdscript
# client/tests/unit/test_considering_indicator.gd
extends "res://addons/gut/test.gd"

const ConsideringIndicator = preload("res://scripts/ui/considering_indicator.gd")
const ConsideringPhrases = preload("res://scripts/ui/considering_phrases.gd")


func test_start_shows_first_phrase_and_stop_clears():
	# Point phrases at a missing file so next() is deterministically LAST_RESORT.
	var phrases = ConsideringPhrases.new("res://content/does_not_exist.json")
	var ind = ConsideringIndicator.new(phrases)
	add_child_autofree(ind)  # _ready() creates the Timer child
	var label := Label.new()
	add_child_autofree(label)

	ind.start(label)
	assert_eq(label.text, ConsideringPhrases.LAST_RESORT)  # first phrase shown at once

	ind.stop()
	assert_eq(label.text, "")  # cleared on stop
```

- [ ] **Step 2: Run to verify it fails**

Run: `cd client && ./run_tests.sh -gtest=res://tests/unit/test_considering_indicator.gd`
Expected: FAIL — script does not exist.

- [ ] **Step 3: Implement**

```gdscript
# client/scripts/ui/considering_indicator.gd
class_name ConsideringIndicator
extends Node
## Thin node shim (charter §13). While a call is in flight it rotates an authored
## ConsideringPhrases line onto a Label every ROTATE_SECONDS, reframing the wait
## as the GM thinking. The phrase logic is the pure, tested ConsideringPhrases;
## the Timer/tree wiring here is the untested shim (verified by running the
## harness). One instance persists on the harness so the rotation index is not
## reset per call (spec decision 4).

const ROTATE_SECONDS := 2.0

var _phrases: ConsideringPhrases
var _timer: Timer
var _label: Label


func _init(phrases: ConsideringPhrases = null) -> void:
	_phrases = phrases if phrases != null else ConsideringPhrases.new()


func _ready() -> void:
	_timer = Timer.new()
	_timer.wait_time = ROTATE_SECONDS
	_timer.one_shot = false
	_timer.timeout.connect(_advance)
	add_child(_timer)


func start(label: Label) -> void:
	_label = label
	_advance()  # show a phrase immediately, don't wait ROTATE_SECONDS
	_timer.start()


func stop() -> void:
	_timer.stop()
	if _label != null:
		_label.text = ""
		_label = null


func _advance() -> void:
	if _label != null:
		_label.text = _phrases.next()
```

- [ ] **Step 4: Run to verify it passes**

Run: `cd client && godot --headless --import >/dev/null 2>&1; ./run_tests.sh -gtest=res://tests/unit/test_considering_indicator.gd`
Expected: PASS.

- [ ] **Step 5: Commit**

```bash
git add client/scripts/ui/considering_indicator.gd client/tests/unit/test_considering_indicator.gd
git commit -m "feat(client): ConsideringIndicator — rotates think-lines on a Timer"
```

---

## Task 5: Wire the indicator into both harnesses

**Files:**
- Modify: `client/scripts/harness/narrate_harness.gd`
- Modify: `client/scripts/harness/npc_harness.gd`
- (No unit test — throwaway harnesses, verified by running.)

**Interfaces:**
- Consumes: `ConsideringIndicator` (Task 4).

- [ ] **Step 1: Wire `narrate_harness.gd`**

It already has a `_status: Label`. Add the indicator field, build it in `_ready`, and wrap the await.

Add the preload near the other consts:

```gdscript
const ConsideringIndicator = preload("res://scripts/ui/considering_indicator.gd")
```

Add the field near the other vars:

```gdscript
var _indicator: ConsideringIndicator
```

At the end of `_ready()` (after the status label is created), add:

```gdscript
	_indicator = ConsideringIndicator.new()
	add_child(_indicator)
```

Replace the body of `_on_narrate_pressed()` with (the change: `start`/`stop` around the await instead of the static "…narrating…" text):

```gdscript
func _on_narrate_pressed() -> void:
	_button.disabled = true
	_indicator.start(_status)
	var r := await _service.narrate(_log)
	_indicator.stop()
	_label.text = r.display_text
	for f in r.facts:
		_log.add_fact(f)
	_status.text = "facts harvested: %d%s" % [r.facts.size(), "   (degraded)" if r.degraded else ""]
	_button.disabled = false
```

- [ ] **Step 2: Wire `npc_harness.gd`**

It has no status label. Add one, add the indicator, and wrap the await.

Add the preload near the other consts:

```gdscript
const ConsideringIndicator = preload("res://scripts/ui/considering_indicator.gd")
```

Add the fields near the other vars:

```gdscript
var _status: Label
var _indicator: ConsideringIndicator
```

In `_ready()`, after the Speak button is added to the vbox, add a status label and the indicator:

```gdscript
	_status = Label.new()
	vbox.add_child(_status)

	_indicator = ConsideringIndicator.new()
	add_child(_indicator)
```

In `_on_speak()`, wrap the service call — start after appending "You:", stop before rendering Fenn's reply:

```gdscript
	_append("[b]You:[/b] %s" % utterance)

	var disposition := int(_game_state.npc_dispositions.get(NPC_ID, 0))
	var available := NpcContent.available_moves(_content.npc(NPC_ID), _game_state, _canon_log)

	_indicator.start(_status)
	var r: NpcResult = await _service.speak(
		NPC_ID, utterance, _canon_log, disposition, available)
	_indicator.stop()
```

(The rest of `_on_speak()` — MoveApplier, fact harvest, appends, `ends_conversation` — is unchanged.)

- [ ] **Step 3: Verify the project imports and the suite is green**

Run: `cd client && godot --headless --import >/dev/null 2>&1; ./run_tests.sh`
Expected: all unit tests PASS (the harness changes must not break import/parse or any existing test).

- [ ] **Step 4: Run the harness by hand (proxy + Ollama up)**

In the Godot editor, run `npc_harness.tscn` (F6). Confirm: on Speak, the status line rotates through the authored think-lines (~2s cadence) while waiting; it clears and Fenn's prose appears on return. Then stop the proxy and Speak again: the line rotates, then after ~35s the harness shows the authored fallback (not an infinite spin).

- [ ] **Step 5: Commit**

```bash
git add client/scripts/harness/narrate_harness.gd client/scripts/harness/npc_harness.gd
git commit -m "feat(client): harnesses show the considering-state around the call"
```

---

## Self-Review

**Spec coverage** — every spec section maps to a task:
- Decision 1–2 (reframe; loading is content) → Task 1 (`ConsideringPhrases` + `considering.json`).
- Decision 3 (rotate ~2s; pure core / node shim) → Task 1 (pure) + Task 4 (shim, `ROTATE_SECONDS`).
- Decision 4 (index is instance state, not reset) → Task 1 (running `_index`, test `test_index_persists…`) + Task 4 (one persistent instance).
- Decision 5 (timeout fix, default 35, reject ≤0) → Task 2 + Task 3.
- Decision 6 (no server change) → honoured; no server files in any task.
- Decision 7 (both harnesses) → Task 5.
- Components list → Tasks 1–5 one-to-one.
- Error handling (hang → timeout → degrade; missing file → LAST_RESORT) → Task 3 (relies on existing `test_transport_failure_degrades`, noted in constraints, not duplicated) + Task 1 (`test_missing_file_returns_last_resort`).
- Testing section → each task's tests; the Timer-fire path is explicitly run-verified in Task 5 Step 4, not unit-tested (matches the spec).

**Placeholder scan** — no TBD/TODO. Task 5 has no unit test *by design* (throwaway harness), verified by Step 3 (suite) + Step 4 (by-hand run); every code step shows complete code.

**Type consistency** — `ConsideringPhrases.next() -> String` and `.LAST_RESORT`/`.PATH` used identically across Tasks 1/4; `ConsideringIndicator.new(phrases)` / `start(label: Label)` / `stop()` match between Task 4 and Task 5; `ProxyConfig.request_timeout_seconds()` / `TIMEOUT_SETTING` / `TIMEOUT_DEFAULT` match between Tasks 2 and 3; `_status` is a `Label` in both harnesses (narrate already has it; npc adds it) so `start(_status)` type-checks.