Files
code_of_conquest_dnd/docs/superpowers/plans/2026-07-10-considering-state.md
Phillip Tarrant 2c32bba320 docs(plan): considering-state + transport timeout implementation plan
5 TDD tasks: ConsideringPhrases (pure) + authored considering.json;
ProxyConfig.request_timeout_seconds (default 35s, reject <=0); HttpTransport
applies it (closes the infinite-hang gap); ConsideringIndicator node shim
(Timer rotates a Label); wire both harnesses around the await.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-10 14:55:00 -05:00

20 KiB
Raw Permalink Blame History

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

# 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:

{
  "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:

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
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:

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:

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
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:

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 preloads ProxyConfig), set the timeout:

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
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

# 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
# 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
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:

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

Add the field near the other vars:

var _indicator: ConsideringIndicator

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

	_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):

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:

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

Add the fields near the other vars:

var _status: Label
var _indicator: ConsideringIndicator

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

	_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:

	_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
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 12 (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 15 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 consistencyConsideringPhrases.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.