Phillip Tarrant
2c32bba320
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>
521 lines
20 KiB
Markdown
521 lines
20 KiB
Markdown
# 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.
|