diff --git a/docs/architecture/proposal_object_contract.md b/docs/architecture/proposal_object_contract.md index 3eddaa39..1ebe0a64 100644 --- a/docs/architecture/proposal_object_contract.md +++ b/docs/architecture/proposal_object_contract.md @@ -57,6 +57,18 @@ must converge to the same typed input contract and the same execution path. (`component="surface_intent"`); it never becomes "pressed nothing". - Surfaces are resolved from durable state (draft store, session store), never from the in-memory focus cache. The 2026-09-03 incident is the shape this clause forbids. +- A thread whose root a surface posted belongs to that surface. User focus — the DM-wide + memory of who last answered — never outranks that ownership. A new surface registers its + root in the resolver chain (`handlers.route_slack_event`, the ordered resolvers before + agent selection) or its threads will be routed by focus. What ships today is the + planning-card case (#310, #320): a `timeboxing_agent` that no explicit per-thread binding + chose — whether it arrived by focus or as the channel default — is demoted to + `receptionist_agent`; an explicit per-thread binding (`/ff-focus`) still wins. The general + form — focus never applies inside any bot-posted thread — waits on #302 re-keying DM + session threads, where it cannot yet be verified. +- An agent that owns a workflow exposes `question` in every state its surface allows. Asked + is not started, and asked is not revised: a question changes nothing in the session it is + asked of (spec: `docs/superpowers/specs/2026-09-05-asked-not-started-design.md`). ## Current Scan (2026-03-06) diff --git a/docs/reference/setup/llm.md b/docs/reference/setup/llm.md index c28b3215..ee0e3a03 100644 --- a/docs/reference/setup/llm.md +++ b/docs/reference/setup/llm.md @@ -141,6 +141,15 @@ every site where it was: - **The timeboxing stage cards** (`core/runtime._build_timeboxing_intent_interpreter`) inherited `timeboxing_agent` — the pro pin at `high`, uncapped, under this repo's `.env`, which is the seam #325 is about. They now run pro/`low` capped at 1024: effort down, and a bound added. + `tests/integration/test_eval_timebox_question.py` measures this same row now — it used to build + its own client on `timeboxing_agent`, the pin production had already moved this interpreter off + of when #336 landed, so it was reporting a model nothing runs on. Re-baselined 2026-09-12 on the + row's default (pro/`low`/1024): all 16 positive cases 8/8. On flash at `minimal` the loss is the + revision case — 0/8 to `ReviseArtifact`, every draw answering `ProvidePlanningFacts` instead — + which is #406's starting point for this eval; the fact cases still hold there (8/8, 8/8, 7/8). + Its break-it convention is `planning_card`'s: assert the *flip* (the wrong decision + outnumbering the right one), never the absence of the right one, since an absence-based bar is + cleared by two lost calls with the discriminating clause doing nothing at all. - **The planning card** (`slack_bot/planning.PlanningCoordinator._ensure_intent_interpreter`) inherited `planner_agent` — the pro pin under `.env` at reasoning `low` (no planner branch exists in `_reasoning_effort_for_agent` and the table's floor is `low`), uncapped. It now runs diff --git a/docs/superpowers/plans/2026-09-05-asked-not-started.md b/docs/superpowers/plans/2026-09-05-asked-not-started.md new file mode 100644 index 00000000..f57057f7 --- /dev/null +++ b/docs/superpowers/plans/2026-09-05-asked-not-started.md @@ -0,0 +1,1222 @@ +# Asked ≠ Started 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:** A question typed to the Schedular is answered from the session and the calendar, and never turns into — or revises — a timeboxing session. + +**Architecture:** One new intent (`AskQuestion`) and one new outcome (`Asked`) in the session kernel; the kernel returns `Asked` *before* applying or saving anything, so the snapshot revision cannot move. The host renders `Asked` by describing the session in prose (derived from the `StageCard` already built for the user) and asking `planner_agent`, which holds the calendar tools. The no-day short-circuit in `derive_timebox_intent` becomes a judged `no_session` state — start, question, cancel — so the interpreter decides instead of an unconditional return. The routing rule #310 fixed one instance of is written into the proposal contract. + +**Tech Stack:** Python 3.11, Pydantic v2 (`_StrictModel`, discriminated unions), AutoGen (`TextMessage`, `AgentId`, `runtime.send_message`), the shared `SurfaceIntentInterpreter`, pytest + pytest-asyncio (auto mode), OpenRouter for the `@slow` eval. + +**Spec:** `docs/superpowers/specs/2026-09-05-asked-not-started-design.md`. **Tickets:** #316 (Task 1), #320 (Task 2), #317 (Task 3), #318 (Task 4), #319 (Task 5) on map #157. + +## Global Constraints + +- **No keyword matching, string matching, or regex on user content. Ever.** (`CLAUDE.md`). Whether a reply is a question is the interpreter's decision. The one exception is identifiers the system minted. +- **Asked is not started, asked is not revised.** An `AskQuestion` turn leaves `snapshot.revision`, `facts`, `artifacts`, `assumptions`, `approvals` and `status` byte-identical. The kernel returns `Asked` before `_apply_intent` and never calls `_save` for it. +- **The user's words reach the answerer verbatim.** `AskQuestion.question` is bound by the host from the Slack text, never from anything the model wrote. +- **Failure stays loud.** An answerer that raises or times out is reported in-thread and metered `record_error(component="surface_intent", error_type="answer_failure")`; never degraded to silence, never retried into a session start. +- **`cancelled` sessions stay closed.** `_display_context` keeps returning `()` for `status == "cancelled"`; `question` is not added there. +- **Worktree discipline.** All work in `.worktrees/asked-not-started` on branch `feat/asked-not-started`. Run every test as `PYTHONPATH=src ../../.venv/bin/python -m pytest …` from the worktree root — without `PYTHONPATH=src` the venv imports the *parent* checkout's `src`, and you would be testing code you did not change. +- **Package suite before reporting done:** `PYTHONPATH=src ../../.venv/bin/python -m pytest tests -m "not slow" -q`. Three failures are pre-existing on `main` and not yours: `tests/e2e/test_slack_handoff_flow.py::test_slack_handoff_sets_focus_and_forwards` and two date-dependent cases in `tests/unit/test_planning_reminder_suppression.py` (they fail on weekends). Anything else red is yours. +- **Commit style:** `(): (#)`, e.g. `feat(timeboxing): a question is a kernel outcome that changes nothing (#316)`. End every commit message with `Co-Authored-By: Claude Fable 5.1 `. Commit inside the worktree only. Never `git add -A`; name the files. +- **Never pin `temperature`** in the eval; never assert an exact model output string in a unit test — assert the decision it drove. +- **Map #157's governing constraint:** least code that holds the invariants; reuse what exists (`StageCard`, `SurfaceIntentInterpreter`, `timebox_failure_message`); say in your report what the next read-only intent would cost after this. + +--- + +## File Structure + +| File | Responsibility | Tasks | +|---|---|---| +| `src/fateforger/agents/timeboxing/session_contracts.py` | `AskQuestion` intent, `Asked` outcome, union membership | 1 | +| `src/fateforger/agents/timeboxing/adaptive_timeboxing.py` | `_turn_guarded` returns `Asked` before any apply/save | 1 | +| `src/fateforger/slack_bot/timeboxing_intents.py` | `question`/`start` decisions; `no_session` state; binding; prompt fragment split into base + `QUESTION_PARAGRAPH` | 1, 4 | +| `src/fateforger/slack_bot/timeboxing_host.py` | `derive_timebox_intent` loses the unconditional `StartSession()` | 4 | +| `src/fateforger/slack_bot/stage_cards.py` | `describe_session(snapshot, card)` — prose renderer beside the Block Kit one | 3 | +| `src/fateforger/slack_bot/handlers.py` | `_answer_question`; `Asked` branch in `_run_adaptive_timebox_turn`; F1 demotion to receptionist | 2, 3 | +| `docs/architecture/proposal_object_contract.md` | §7 gains the two ownership rules | 2 | +| `tests/unit/test_timeboxing_intents.py` | interpreter → `AskQuestion`, `no_session` bindings | 1, 4 | +| `tests/unit/test_adaptive_timeboxing.py` | kernel: `Asked` leaves the snapshot untouched | 1 | +| `tests/unit/test_slack_timeboxing_routing.py` | F1 tests | 2 | +| `tests/unit/test_asked_is_answered_in_the_turn.py` | host renders `Asked` via `planner_agent`; failure loud | 3 | +| `tests/unit/test_describe_session.py` | prose renderer fields | 3 | +| `tests/unit/test_no_session_is_judged.py` | `derive_timebox_intent` no-day cases + AST guard | 4 | +| `tests/integration/test_eval_timebox_question.py` | `@slow` n=8 eval with break-it check | 5 | + +--- + +## Task 1: `AskQuestion` intent, `Asked` outcome, `question` in every state (#316) + +**Files:** +- Modify: `src/fateforger/agents/timeboxing/session_contracts.py` (after `class CancelSession` ~line 490; after `class Cancelled` ~line 580; both unions) +- Modify: `src/fateforger/agents/timeboxing/adaptive_timeboxing.py` (`_turn_guarded`, between the committed-cancel refusal and `base_revision = snapshot.revision`, ~line 490) +- Modify: `src/fateforger/slack_bot/timeboxing_intents.py` (`InterpretedTimeboxTurn.decision` ~line 97; `_TIMEBOX_PROMPT_FRAGMENT` ~line 216; `_display_context` ~line 303; `TimeboxingIntentInterpreter.interpret` ~line 396; `_intent_from_interpreted` ~line 470) +- Test: `tests/unit/test_adaptive_timeboxing.py`, `tests/unit/test_timeboxing_intents.py` + +**Interfaces:** +- Produces: `AskQuestion(kind="ask_question", question: str)` and `Asked(kind="asked", question: str)` in `session_contracts`; `_intent_from_interpreted(interpreted, *, snapshot, pending, user_text)` gains the keyword `user_text: str`; `timeboxing_intents.QUESTION_PARAGRAPH: str` (module constant, public) and `_TIMEBOX_PROMPT_FRAGMENT = _TIMEBOX_PROMPT_FRAGMENT_BASE + QUESTION_PARAGRAPH`. Task 5's break-it check strips `QUESTION_PARAGRAPH` by monkeypatching `_TIMEBOX_PROMPT_FRAGMENT` to `_TIMEBOX_PROMPT_FRAGMENT_BASE`. +- Consumes: nothing from other tasks. + +- [ ] **Step 1: Write the failing kernel test** + +Append to `tests/unit/test_adaptive_timeboxing.py` (use that file's existing snapshot/kernel fixtures if it has them; otherwise the shape below, which mirrors `_kernel` in `tests/unit/test_timeboxing_intents.py`): + +```python +from fateforger.agents.timeboxing.session_contracts import AskQuestion, Asked + + +@pytest.mark.asyncio +async def test_a_question_is_asked_and_changes_nothing() -> None: + """Asked is not started and not revised: the snapshot the next load sees + is the one this turn loaded, field for field.""" + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=3, owner_user_id="U1", + planning_day=_planning_day(), status="open", + ) + repo = InMemoryPlanningSessionRepository([snapshot]) + kernel = AdaptiveTimeboxing( + repository=repo, requirements=TimeboxRequirements(), + planner=_ForbiddenDependency(), context=_ForbiddenDependency(), + commit=_ForbiddenDependency(), + ) + before = (await repo.load_or_create("C1:1.0", owner_user_id="U1")).model_dump() + + outcome = await kernel.turn( + TurnRequest( + session_key="C1:1.0", interaction_id="q1", actor_user_id="U1", + expected_revision=3, intent=AskQuestion(question="Is it planned?"), + ), + progress=_ProgressSink(), + ) + + assert isinstance(outcome, Asked) + assert outcome.question == "Is it planned?" + after = (await repo.load_or_create("C1:1.0", owner_user_id="U1")).model_dump() + assert after == before + assert after["revision"] == 3 + + +@pytest.mark.asyncio +async def test_a_question_to_a_committed_session_is_still_just_asked() -> None: + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=9, owner_user_id="U1", + planning_day=_planning_day(), status="committed", + ) + repo = InMemoryPlanningSessionRepository([snapshot]) + kernel = AdaptiveTimeboxing( + repository=repo, requirements=TimeboxRequirements(), + planner=_ForbiddenDependency(), context=_ForbiddenDependency(), + commit=_ForbiddenDependency(), + ) + outcome = await kernel.turn( + TurnRequest( + session_key="C1:1.0", interaction_id="q2", actor_user_id="U1", + expected_revision=9, intent=AskQuestion(question="when is deep work?"), + ), + progress=_ProgressSink(), + ) + assert isinstance(outcome, Asked) + assert (await repo.load_or_create("C1:1.0", owner_user_id="U1")).revision == 9 +``` + +`_planning_day`, `_ProgressSink`, `_ForbiddenDependency` exist in `tests/unit/test_timeboxing_intents.py`; copy them into this file if `test_adaptive_timeboxing.py` lacks equivalents (do not import test helpers across test files). + +- [ ] **Step 2: Run it to verify it fails** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_adaptive_timeboxing.py -k question -q` +Expected: FAIL with `ImportError: cannot import name 'AskQuestion'`. + +- [ ] **Step 3: Add the intent and the outcome** + +In `session_contracts.py`, after `class CancelSession`: + +```python +class AskQuestion(_StrictModel): + """A question about the day, the plan or the calendar. + + The one intent that changes nothing: the kernel returns `Asked` before it + applies or saves anything. `question` is the user's words as the host + received them -- never a model's paraphrase, so nothing the model wrote + reaches the answerer as if the user said it. + """ + + kind: Literal["ask_question"] = "ask_question" + question: str = Field(min_length=1) +``` + +Add `AskQuestion` to the `TimeboxIntent` union (after `CancelSession`). + +After `class Cancelled`: + +```python +class Asked(_StrictModel): + """The turn was a question. Nothing in the session moved; the host answers.""" + + kind: Literal["asked"] = "asked" + question: str = Field(min_length=1) +``` + +Add `Asked` to the `TurnOutcome` union (after `Cancelled`). + +- [ ] **Step 4: Return `Asked` in the kernel before anything is applied** + +In `adaptive_timeboxing.py` `_turn_guarded`, import `AskQuestion` and `Asked`, and insert immediately after the `session_committed` refusal block and before `base_revision = snapshot.revision`: + +```python + if isinstance(request.intent, AskQuestion): + # Asked is not started and not revised. Nothing is applied and + # nothing is saved: the revision the next load sees is the one + # this turn loaded. The host answers from the snapshot and the + # calendar; the kernel's whole job here is to say so. + return Asked(question=request.intent.question) +``` + +- [ ] **Step 5: Run the kernel tests** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_adaptive_timeboxing.py -q` +Expected: PASS, including the two new tests. + +- [ ] **Step 6: Write the failing interpreter tests** + +Append to `tests/unit/test_timeboxing_intents.py`: + +```python +from fateforger.agents.timeboxing.session_contracts import AskQuestion + + +@pytest.mark.asyncio +async def test_a_question_during_capture_binds_the_users_words_verbatim() -> None: + client = _SchemaOutputClient({"decision": "question", "facts": []}) + interpreter = TimeboxingIntentInterpreter(client) + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=2, owner_user_id="U1", + planning_day=_planning_day(), status="open", + ) + intent = await interpreter.interpret(" Is it planned? ", snapshot) + assert isinstance(intent, AskQuestion) + assert intent.question == " Is it planned? " # verbatim, not stripped, not paraphrased + _, json_output = client.calls[0] + assert "question" in get_args(json_output.model_fields["decision"].annotation) + + +@pytest.mark.asyncio +async def test_a_question_on_a_committed_session_is_offered_and_bound() -> None: + client = _SchemaOutputClient({"decision": "question", "facts": []}) + interpreter = TimeboxingIntentInterpreter(client) + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=9, owner_user_id="U1", + planning_day=_planning_day(), status="committed", + ) + intent = await interpreter.interpret("what did we settle on for lunch?", snapshot) + assert isinstance(intent, AskQuestion) + + +@pytest.mark.asyncio +async def test_a_cancelled_session_still_accepts_no_intent() -> None: + client = _SchemaOutputClient({"decision": "question", "facts": []}) + interpreter = TimeboxingIntentInterpreter(client) + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=4, owner_user_id="U1", + planning_day=_planning_day(), status="cancelled", + ) + with pytest.raises(ValueError, match="does not accept another intent"): + await interpreter.interpret("Is it planned?", snapshot) + assert client.calls == [] + + +def test_every_open_state_offers_question() -> None: + """The contract: an agent that owns a workflow exposes `question` in every + state its surface allows. Pinned per state so a new state cannot forget.""" + from fateforger.slack_bot.timeboxing_intents import _display_context + open_snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=2, owner_user_id="U1", + planning_day=_planning_day(), status="open", + ) + committed = open_snapshot.model_copy(update={"status": "committed", "revision": 9}) + for snapshot in (open_snapshot, committed): + _, allowed, _ = _display_context(snapshot) + assert "question" in allowed, snapshot.status +``` + +Add `from typing import get_args` at the top of the test file if absent. If `_SchemaOutputClient` in this file needs the decision schema to be a Pydantic model (`json_output`), it already is — `SurfaceIntentInterpreter` passes the narrowed schema class. + +- [ ] **Step 7: Run them to verify they fail** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_timeboxing_intents.py -k "question or cancelled_session or every_open_state" -q` +Expected: FAIL — `ValidationError` on `decision: "question"` (not in the Literal) and the `_display_context` assertion. + +- [ ] **Step 8: Add `question` to the schema, every open state, the binding, and the fragment** + +In `timeboxing_intents.py`: + +(a) `InterpretedTimeboxTurn.decision` Literal: add `"question"` after `"deny"`. + +(b) `_display_context`: add `"question"` to every returned tuple **except** the `cancelled` branch, which stays `()`. Concretely: the `committed` tuple becomes `("provide_facts", "revise", "question")`; the `planning_day` tuple `("confirm_planning_day", "cancel", "question")`; the `skeleton` tuple `("provide_facts", "approve", "revise", "back", "cancel", "question")`; the `review_commit` tuple `("approve", "revise", "back", "cancel", "question")`; the Stage 1 / `refine` returns gain `"question"` at the end of their tuples in the same way. Read each return in the function and add it — there are six or seven; do not miss the ones built from `*choose`, `*consent`, `*restore` unpacking. + +(c) Thread the user's words into the binding. In `TimeboxingIntentInterpreter.interpret`, change the final call to +`return _intent_from_interpreted(interpreted, snapshot=snapshot, pending=pending, user_text=user_text)`, add `user_text: str` as a keyword-only parameter of `_intent_from_interpreted`, and add as the **first** branch of its body: + +```python + if interpreted.decision == "question": + # The host's copy of the words, verbatim. The schema carries no text + # field for this decision on purpose: a paraphrase is the model's + # words reaching the answerer as if the user said them. + return AskQuestion(question=user_text) +``` + +Import `AskQuestion` from `session_contracts`. + +(d) Split the fragment. Rename the existing string to `_TIMEBOX_PROMPT_FRAGMENT_BASE` and add: + +```python +QUESTION_PARAGRAPH = """A reply that asks about the day, the plan, the calendar, or what was +decided -- "is it planned?", "did you add the gym?", "what did we settle on +for lunch?", "when is deep work?" -- is question. A reply that supplies a +fact, a correction, or an instruction against the plan is what it was +before. A reply that asks and also supplies a fact is that fact: the fact +changes the day and the question does not. +""" + +_TIMEBOX_PROMPT_FRAGMENT = _TIMEBOX_PROMPT_FRAGMENT_BASE + QUESTION_PARAGRAPH +``` + +Everything that referenced `_TIMEBOX_PROMPT_FRAGMENT` keeps working; it is the same name with more text. + +- [ ] **Step 9: Run the interpreter tests, then the package suite** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_timeboxing_intents.py tests/unit/test_timeboxing_intents_steer.py tests/unit/test_adaptive_timeboxing.py -q` +Expected: PASS. + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests -m "not slow" -q` +Expected: only the three pre-existing failures named in Global Constraints. + +- [ ] **Step 10: Commit** + +```bash +git add src/fateforger/agents/timeboxing/session_contracts.py src/fateforger/agents/timeboxing/adaptive_timeboxing.py src/fateforger/slack_bot/timeboxing_intents.py tests/unit/test_adaptive_timeboxing.py tests/unit/test_timeboxing_intents.py +git commit -m "feat(timeboxing): a question is a kernel outcome that changes nothing (#316) + +AskQuestion joins the intent union and Asked the outcome union. The kernel +returns Asked before it applies or saves anything, so the revision the next +load sees is the one the turn loaded. Every open state offers question; a +cancelled session still accepts nothing. The user's words are bound by the +host, verbatim. + +Co-Authored-By: Claude Fable 5.1 " +``` + +--- + +## Task 2: Focus demotes to the receptionist; the two rules in the contract (#320) + +**Files:** +- Modify: `src/fateforger/slack_bot/handlers.py` (the resolver block #310 reordered, ~lines 2676-2684: `if binding is None and agent_type == "timeboxing_agent": agent_type = channel_default_agent or default_agent`) +- Modify: `docs/architecture/proposal_object_contract.md` (§7, after the "Surfaces are resolved from durable state" bullet) +- Test: `tests/unit/test_slack_timeboxing_routing.py` + +**Interfaces:** +- Produces: nothing code-level other tasks consume. +- Consumes: nothing. + +- [ ] **Step 1: Write the two failing tests** + +Append to `tests/unit/test_slack_timeboxing_routing.py`, after `test_a_planning_thread_survives_a_sticky_dm_focus_on_timeboxing`: + +```python +@pytest.mark.asyncio +async def test_a_planning_thread_in_a_timeboxing_channel_is_demoted_to_the_receptionist(monkeypatch): + # #310 demoted to the channel default, which is a no-op when that default + # is itself timeboxing_agent. The planning card's thread goes to the + # receptionist, whatever the channel is for. + import fateforger.slack_bot.handlers as handlers + monkeypatch.setattr(handlers, "_agent_for_channel", lambda channel_id: "timeboxing_agent") + focus = FocusManager( + ttl_seconds=60, allowed_agents=["receptionist_agent", "timeboxing_agent"] + ) + runtime = _FakeRuntime([_FakeResult(TextMessage(content="answer", source="bot"))]) + runtime.timeboxing_session_store = _SessionStore({}) + client = _FakeClient() + planning = _PlanningReplyHandler( + ThreadReply(ThreadReplyOutcome.NO_PRESS, context="CARD CONTEXT"), owns=True + ) + + await route_slack_event( + runtime=runtime, focus=focus, default_agent="receptionist_agent", + event={"channel": "C9", "user": "U1", "text": "Is it planned?", "thread_ts": "root", "ts": "777"}, + bot_user_id=None, say=_unused_say, client=client, planning=planning, + ) + + assert planning.ownership_calls == [("C9", "root")] + assert len(runtime.calls) == 1 + assert runtime.calls[0][1].type == "receptionist_agent" + + +@pytest.mark.asyncio +async def test_an_explicit_thread_binding_beats_planning_ownership(): + # /ff-focus on this very thread is the one thing the user asked for by + # name; ownership does not take it away. #310 traced this and never pinned it. + focus = FocusManager( + ttl_seconds=60, allowed_agents=["receptionist_agent", "timeboxing_agent"] + ) + focus.set_focus("D1:root", "timeboxing_agent", by_user="U1", note="ff-focus") + runtime = _FakeRuntime([_FakeResult(TextMessage(content="ok", source="bot"))]) + runtime.timeboxing_session_store = _SessionStore({}) + client = _FakeClient() + planning = _PlanningReplyHandler( + ThreadReply(ThreadReplyOutcome.NO_PRESS, context="CARD CONTEXT"), owns=True + ) + + await _route(runtime=runtime, focus=focus, client=client, planning=planning, + event=_dm_reply_event("Is it planned?")) + + assert len(runtime.calls) == 1 + assert runtime.calls[0][1].type == "timeboxing_agent" +``` + +If the DM-route to `timeboxing_agent` in the second test trips the harness backend (`_timebox_backend() != "legacy"` → `_run_adaptive_timebox_turn`), set `monkeypatch.setenv("FF_TIMEBOX_BACKEND", "legacy")` in that test — the assertion is about which agent was chosen, not which backend served it. Look at how `test_routes_thread_reply_to_timeboxing_user_reply` in the same file handles this and do the same. + +- [ ] **Step 2: Run them to verify the first fails** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_slack_timeboxing_routing.py -k "demoted_to_the_receptionist or explicit_thread_binding" -q` +Expected: the demotion test FAILS with `'timeboxing_agent' == 'receptionist_agent'`; the binding test may already pass (it pins existing behaviour). + +- [ ] **Step 3: Demote to the receptionist** + +In `handlers.py`, replace the fallback line inside the `if claimed_by_planning:` branch: + +```python + if binding is None and agent_type == "timeboxing_agent": + # Not the channel default: when that default is itself + # timeboxing_agent the demotion was a no-op (#310's review). + # The receptionist is the one agent that refers rather than + # starts, which is what a card's thread needs. + agent_type = "receptionist_agent" +``` + +- [ ] **Step 4: Run the routing suite** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_slack_timeboxing_routing.py -q` +Expected: PASS, all tests. + +- [ ] **Step 5: Write the rules into the contract** + +In `docs/architecture/proposal_object_contract.md` §7, after the bullet beginning "Surfaces are resolved from durable state", add: + +```markdown +- A thread whose root a surface posted belongs to that surface. User focus — the DM-wide + memory of who last answered — never outranks that ownership. A new surface registers its + root in the resolver chain (`handlers.route_slack_event`, the ordered resolvers before + agent selection) or its threads will be routed by focus. What ships today is the + planning-card case (#310, #320): a `timeboxing_agent` that arrived by focus is demoted to + `receptionist_agent`; an explicit per-thread binding (`/ff-focus`) still wins. The general + form — focus never applies inside any bot-posted thread — waits on #302 re-keying DM + session threads, where it cannot yet be verified. +- An agent that owns a workflow exposes `question` in every state its surface allows. Asked + is not started, and asked is not revised: a question changes nothing in the session it is + asked of (spec: `docs/superpowers/specs/2026-09-05-asked-not-started-design.md`). +``` + +- [ ] **Step 6: Commit** + +```bash +git add src/fateforger/slack_bot/handlers.py docs/architecture/proposal_object_contract.md tests/unit/test_slack_timeboxing_routing.py +git commit -m "fix(slack): a planning card's thread falls back to the receptionist, and the contract says why (#320) + +The channel default is a no-op when it is itself timeboxing_agent. The +receptionist refers rather than starts, which is what a card's thread needs. +Two rules land in the proposal contract: a surface owns its thread over focus, +and a workflow-owning agent offers question in every state. + +Co-Authored-By: Claude Fable 5.1 " +``` + +--- + +## Task 3: `describe_session` from `StageCard`; the host answers `Asked` through `planner_agent` (#317) + +**Files:** +- Modify: `src/fateforger/slack_bot/stage_cards.py` (add `describe_session` at module end, before `__all__` if present) +- Modify: `src/fateforger/slack_bot/handlers.py` (add `_answer_question` near `_run_adaptive_timebox_turn`; add the `Asked` branch in `_run_adaptive_timebox_turn` right after the `finally: await progress_card.close()` block and before `present_outcome`) +- Test: `tests/unit/test_describe_session.py` (create), `tests/unit/test_asked_is_answered_in_the_turn.py` (create) + +**Interfaces:** +- Consumes: `Asked` from Task 1; `StageCard` and `_stage_cards.shown(session_key)` (returns an object with `.card: StageCard`, or `None`) already in handlers; `timebox_failure_message(snapshot=...)` from `timeboxing_cards`; `runtime.send_message(msg, recipient=AgentId(...))` and `_slack_payload_from_result(result)` already in handlers. +- Produces: `stage_cards.describe_session(snapshot: PlanningSessionSnapshot, card: StageCard | None) -> str`; `handlers._answer_question(*, runtime, session_key, actor_user_id, snapshot, card, question, logger) -> SlackBlockMessage`. + +- [ ] **Step 1: Write the failing prose-renderer test** + +Create `tests/unit/test_describe_session.py`: + +```python +"""`describe_session` says what the card says, in prose, for an agent that +cannot see the card. Fields, not sentences: the wording is free to move.""" + +from __future__ import annotations + +from datetime import date + +from fateforger.agents.timeboxing.session_contracts import ( + ArtifactKind, + PlanningArtifact, + PlanningDay, + PlanningSessionSnapshot, +) +from fateforger.slack_bot.stage_cards import ( + ContextItem, + DecidedItem, + StageCard, + describe_session, + stage, +) + + +def _planning_day() -> PlanningDay: + return PlanningDay.lock_default( + value=date(2026, 9, 5), timezone="Europe/Amsterdam", lock_revision=1 + ) + + +def test_a_stage_three_card_names_its_decided_items_and_the_day() -> None: + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=5, owner_user_id="U1", + planning_day=_planning_day(), status="open", + ) + card = StageCard( + stage=stage(3), session_key="C1:1.0", expected_revision=5, + context=[ContextItem(text="Oats two hours before gym", source="memory")], + decided=[ + DecidedItem(text="Gym at 18:00", kind="fact", ref="f1"), + DecidedItem(text="Lunch at 13:00", kind="assumption", ref="a1", filed_by="planner"), + ], + body="07:00 wake · 09:00 deep work · 18:00 gym", + ) + text = describe_session(snapshot, card) + assert "2026-09-05" in text + assert "Saturday" in text + assert "3/5" in text and "Sketch" in text + assert "Gym at 18:00" in text + assert "Lunch at 13:00" in text and "assumption" in text and "planner" in text + assert "Oats two hours before gym" in text + assert "deep work" in text + + +def test_a_committed_session_names_the_receipt() -> None: + receipt = PlanningArtifact.create( + kind=ArtifactKind.COMMIT_RECEIPT, revision=1, + payload={"tx_id": "tx_42", "calendar_id": "hugo@example.com", "applied": 14, + "calendar_backend": "google", "durable": True}, + dependency_revisions={}, + ) + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=9, owner_user_id="U1", + planning_day=_planning_day(), status="committed", artifacts=[receipt], + ) + text = describe_session(snapshot, card=None) + assert "committed" in text + assert "tx_42" in text + assert "hugo@example.com" in text + assert "14" in text + + +def test_a_fresh_session_says_so() -> None: + snapshot = PlanningSessionSnapshot(session_key="D1:dm", revision=0, owner_user_id="U1") + text = describe_session(snapshot, card=None) + assert "no planning day" in text.lower() or "not started" in text.lower() +``` + +Check `PlanningArtifact.create`'s real signature and the receipt payload keys `timeboxing_host.py` writes (~line 415: `tx_id`, `calendar_id`, `reason`, `candidate_digest`, `calendar_backend`, `durable`, and the applied count under whatever key it uses) and match them in the test — the assertion is that the description carries the receipt's identifying fields. + +- [ ] **Step 2: Run it to verify it fails** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_describe_session.py -q` +Expected: FAIL with `ImportError: cannot import name 'describe_session'`. + +- [ ] **Step 3: Write `describe_session`** + +Append to `stage_cards.py`: + +```python +def describe_session( + snapshot: PlanningSessionSnapshot, card: StageCard | None +) -> str: + """What the user is looking at, in prose, for an agent that cannot see it. + + The same fields the card renders and nothing the card does not show: the + prose renderer beside the Block Kit one, over the same `StageCard`. The + snapshot supplies what a card does not carry -- the day, the status, the + receipt -- so a session with no card on screen (a DM after a restart) + still describes itself. + """ + + lines: list[str] = [] + day = snapshot.planning_day + if day is None: + lines.append( + "The user is talking to their timeboxing session; no planning day " + "has been locked yet (the session has not started)." + ) + else: + lines.append( + f"The user is talking to their timeboxing session for " + f"{day.date.isoformat()} ({day.date.strftime('%A')}, " + f"{day.day_type.value} day, {day.timezone})." + ) + lines.append(f"Session status: {snapshot.status}.") + + if card is not None: + lines.append( + f"Stage {card.stage.index}/5 · {card.stage.name}" + + (f" — {card.done}" if card.done else "") + ) + if card.context: + lines.append("Context in use: " + "; ".join( + f"{item.text} (from {item.source})" for item in card.context + )) + if card.decided: + lines.append("Decided so far: " + "; ".join( + f"{item.text} ({item.kind}" + + (f", filed by {item.filed_by}" if item.filed_by else "") + + ")" + for item in card.decided + )) + if card.asking is not None: + lines.append(f"Open question to the user: {card.asking.question}") + if card.gate: + lines.append(f"Gate: {card.gate}") + if card.body: + lines.append("The card's body:\n" + card.body) + + receipt = next( + (a for a in reversed(snapshot.artifacts) if a.kind is ArtifactKind.COMMIT_RECEIPT), + None, + ) + if receipt is not None: + payload = receipt.payload if isinstance(receipt.payload, dict) else {} + lines.append( + "Commit receipt: " + + ", ".join(f"{k}={v}" for k, v in payload.items() if v is not None) + ) + return "\n".join(lines) +``` + +`ArtifactKind` is already imported in `stage_cards.py`. Match the exact `StageLine.index`/`.name` and `DecidedItem` fields already defined above in the file. + +- [ ] **Step 4: Run the renderer tests** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_describe_session.py -q` +Expected: PASS. + +- [ ] **Step 5: Write the failing turn-level tests** + +Create `tests/unit/test_asked_is_answered_in_the_turn.py`, copying the fixture shape of `tests/unit/test_turn_cancels_ladder.py` (Kernel/Repo/Runtime fakes plus the four monkeypatches — read that file first): + +```python +"""An `Asked` outcome is answered by planner_agent with the session described, +in the turn's own reply. No stage card is drawn, no session state moves, and +an answerer that fails is reported, never swallowed and never retried into a +session start.""" + +from __future__ import annotations + +import logging +from types import SimpleNamespace + +import pytest +from autogen_agentchat.messages import TextMessage + +import fateforger.slack_bot.handlers as handlers +from fateforger.agents.timeboxing.session_contracts import ( + Asked, + AskQuestion, + PlanningSessionSnapshot, +) +from fateforger.slack_bot.timeboxing_cards import timebox_failure_message + + +class _StubCard: + def __init__(self, *a, **k): + pass + + async def close(self): + pass + + +async def _question_intent(*a, **k): + return AskQuestion(question="Is it planned?") + + +def _fixture(monkeypatch, *, reply=None, raise_=None): + sent: list[tuple[object, object]] = [] + + class Kernel: + async def turn(self, request, progress): + assert isinstance(request.intent, AskQuestion) + return Asked(question=request.intent.question) + + class Repo: + async def load_or_create(self, key, owner_user_id): + return PlanningSessionSnapshot( + session_key=key, revision=4, owner_user_id=owner_user_id + ) + + class Runtime: + timeboxing_session_store = Repo() + + async def send_message(self, message, recipient): + sent.append((message, recipient)) + if raise_ is not None: + raise raise_ + return SimpleNamespace(chat_message=TextMessage(content=reply, source="planner_agent")) + + def _no_card(*a, **k): + raise AssertionError("present_outcome must not run for Asked") + + monkeypatch.setattr(handlers, "_timeboxing_kernel", lambda *a, **k: Kernel()) + monkeypatch.setattr(handlers, "derive_timebox_intent", _question_intent) + monkeypatch.setattr(handlers, "HarnessProgressCard", _StubCard) + monkeypatch.setattr(handlers, "present_outcome", _no_card) + return Runtime(), sent + + +async def _turn(runtime): + return await handlers._run_adaptive_timebox_turn( + runtime=runtime, client=object(), logger=logging.getLogger(__name__), + session_key="D1:dm", actor_user_id="U1", interaction_id="1.1", + progress_channel="D1", progress_ts="1.0", + card_channel="D1", card_thread_ts="dm", user_text="Is it planned?", + ) + + +@pytest.mark.asyncio +async def test_a_question_is_answered_by_planner_agent_with_the_session_described(monkeypatch): + runtime, sent = _fixture(monkeypatch, reply="No — nothing on the calendar today.") + message = await _turn(runtime) + assert len(sent) == 1 + msg, recipient = sent[0] + assert recipient.type == "planner_agent" + assert "Is it planned?" in msg.content + assert "timeboxing session" in msg.content # the description came along + assert message.text == "No — nothing on the calendar today." + + +@pytest.mark.asyncio +async def test_an_answerer_that_fails_is_reported_and_never_starts_a_session(monkeypatch): + errors: list[dict] = [] + monkeypatch.setattr(handlers, "record_error", lambda **kw: errors.append(kw)) + runtime, sent = _fixture(monkeypatch, raise_=RuntimeError("planner down")) + message = await _turn(runtime) + assert len(sent) == 1 # asked once, not retried + assert errors == [{"component": "surface_intent", "error_type": "answer_failure"}] + assert message.text == timebox_failure_message(snapshot=None).text +``` + +- [ ] **Step 6: Run them to verify they fail** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_asked_is_answered_in_the_turn.py -q` +Expected: FAIL — `AssertionError: present_outcome must not run for Asked` (the turn falls through to the card mapper). + +- [ ] **Step 7: Add `_answer_question` and the `Asked` branch** + +In `handlers.py`, next to `_run_adaptive_timebox_turn`: + +```python +async def _answer_question( + *, + runtime, + session_key: str, + actor_user_id: str, + snapshot: PlanningSessionSnapshot, + card: StageCard | None, + question: str, + logger, +) -> SlackBlockMessage: + """Answer an `Asked` outcome through planner_agent, the calendar's answerer. + + The session is described from the card the user is looking at plus the + snapshot, and the question travels verbatim after it. One ask; a failure + is one failure line and one metered error, never a second ask and never + a session start. + """ + + content = ( + f"{describe_session(snapshot, card)}\n\n" + f"The user's question:\n{question}" + ) + try: + result = await runtime.send_message( + TextMessage(content=content, source=actor_user_id), + recipient=AgentId("planner_agent", key=session_key), + ) + except Exception as exc: # noqa: BLE001 - one failure shape reaches Slack + logger.error( + "question answer failed session_key=%s error_type=%s error=%s", + session_key, type(exc).__name__, exc, exc_info=True, + ) + record_error(component="surface_intent", error_type="answer_failure") + return timebox_failure_message(snapshot=snapshot) + payload = _compact_slack_payload(**_slack_payload_from_result(result)) + text = payload.get("text", "") or "" + return SlackBlockMessage( + text=text, + blocks=payload.get("blocks") or [ + {"type": "section", "text": {"type": "mrkdwn", "text": text}} + ], + ) +``` + +Import `describe_session` and `StageCard` from `.stage_cards`, and `Asked` from `session_contracts`. Then in `_run_adaptive_timebox_turn`, immediately after the `finally: await progress_card.close()` block and **before** the `try: message, card = present_outcome(...)`: + +```python + if isinstance(outcome, Asked): + # Asked is not started and not revised: no card transition, no panel + # sync, no relabel. The thinking card becomes the answer. + shown = _stage_cards.shown(session_key) + return await _answer_question( + runtime=runtime, + session_key=session_key, + actor_user_id=actor_user_id, + snapshot=current, + card=shown.card if shown is not None else None, + question=outcome.question, + logger=logger, + ) +``` + +Check what `_stage_cards.shown(...)` actually returns (it is used at ~line 1688 as `previous.card`) and match it. + +- [ ] **Step 8: Run the turn tests, then the package suite** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_asked_is_answered_in_the_turn.py tests/unit/test_turn_cancels_ladder.py tests/unit/test_stage_receipts_in_the_turn.py -q` +Expected: PASS. + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests -m "not slow" -q` +Expected: only the three pre-existing failures. + +- [ ] **Step 9: Commit** + +```bash +git add src/fateforger/slack_bot/stage_cards.py src/fateforger/slack_bot/handlers.py tests/unit/test_describe_session.py tests/unit/test_asked_is_answered_in_the_turn.py +git commit -m "feat(slack): a question to the Schedular is answered by planner_agent with the session described (#317) + +describe_session is the prose renderer beside the Block Kit one, over the same +StageCard. The host answers an Asked outcome through planner_agent, which holds +the calendar tools; the thinking card becomes the answer and no stage card +moves. A failed answer is one failure line and one metered error. + +Co-Authored-By: Claude Fable 5.1 " +``` + +--- + +## Task 4: The no-day short-circuit becomes a judged `no_session` state (#318) + +**Files:** +- Modify: `src/fateforger/slack_bot/timeboxing_host.py` (`derive_timebox_intent`, ~lines 437-465) +- Modify: `src/fateforger/slack_bot/timeboxing_intents.py` (`_display_context` head; `InterpretedTimeboxTurn.decision`; `_intent_from_interpreted`) +- Test: `tests/unit/test_no_session_is_judged.py` (create), `tests/unit/test_timeboxing_intents.py` + +**Interfaces:** +- Consumes: `AskQuestion` and the `question` decision from Task 1; `_intent_from_interpreted(..., user_text=)` from Task 1. +- Produces: the `no_session` display state; the `start` decision. + +- [ ] **Step 1: Write the failing tests** + +Create `tests/unit/test_no_session_is_judged.py`: + +```python +"""Before a day is locked there *is* something to decide: start, ask, or +cancel. The interpreter decides it; nothing here reads the words.""" + +from __future__ import annotations + +import ast +import inspect +import json +from types import SimpleNamespace + +import pytest + +import fateforger.slack_bot.timeboxing_host as host_module +from fateforger.agents.timeboxing.session_contracts import ( + Advance, + AskQuestion, + CancelSession, + PlanningSessionSnapshot, + StartSession, +) +from fateforger.slack_bot.timeboxing_host import derive_timebox_intent +from fateforger.slack_bot.timeboxing_intents import ( + TimeboxingIntentInterpreter, + _display_context, +) + + +class _SchemaOutputClient: + def __init__(self, *responses): + self._responses = list(responses) + self.calls = [] + + async def create(self, messages, *, json_output): # noqa: ANN001 + self.calls.append((messages, json_output)) + return SimpleNamespace(content=json.dumps(self._responses.pop(0))) + + +def _fresh() -> PlanningSessionSnapshot: + return PlanningSessionSnapshot(session_key="D1:dm", revision=0, owner_user_id="U1") + + +def _runtime(*responses): + return SimpleNamespace( + timeboxing_intent_interpreter=TimeboxingIntentInterpreter(_SchemaOutputClient(*responses)) + ) + + +def test_a_fresh_session_offers_start_question_and_cancel() -> None: + state, allowed, pending = _display_context(_fresh()) + assert state == "no_session" + assert set(allowed) == {"start", "question", "cancel"} + assert pending is None + + +@pytest.mark.asyncio +async def test_start_opens_the_session_exactly_as_before() -> None: + intent = await derive_timebox_intent(_runtime({"decision": "start", "facts": []}), _fresh(), user_text="plan tomorrow") + assert intent == StartSession() + + +@pytest.mark.asyncio +async def test_a_question_before_a_day_is_asked_not_started() -> None: + intent = await derive_timebox_intent(_runtime({"decision": "question", "facts": []}), _fresh(), user_text="Is it planned?") + assert intent == AskQuestion(question="Is it planned?") + + +@pytest.mark.asyncio +async def test_a_cancel_before_a_day_reaches_the_kernel() -> None: + intent = await derive_timebox_intent(_runtime({"decision": "cancel", "facts": []}), _fresh(), user_text="never mind") + assert intent == CancelSession() + + +@pytest.mark.asyncio +async def test_empty_text_on_a_fresh_session_still_opens_it() -> None: + # The opening turn arrives with no words (the auto-start, a bare command); + # that is a start, as it always was. Only typed words are judged. + runtime = _runtime() # no interpreter response: it must not be asked + intent = await derive_timebox_intent(runtime, _fresh(), user_text=" ") + assert intent == StartSession() + assert runtime.timeboxing_intent_interpreter._core.model_client.calls == [] + + +@pytest.mark.asyncio +async def test_empty_text_on_a_started_session_is_still_advance() -> None: + from datetime import date + from fateforger.agents.timeboxing.session_contracts import PlanningDay + snapshot = PlanningSessionSnapshot( + session_key="D1:dm", revision=2, owner_user_id="U1", + planning_day=PlanningDay.lock_default(value=date(2026, 9, 5), timezone="Europe/Amsterdam", lock_revision=1), + ) + assert await derive_timebox_intent(_runtime(), snapshot, user_text="") == Advance() + + +def test_derive_timebox_intent_has_no_unconditional_start() -> None: + """The guard for the claim this ticket deletes: no `return StartSession()` + that is not inside the judged path. Any Return whose value calls + StartSession must sit under an `if` on the text being empty.""" + tree = ast.parse(inspect.getsource(host_module)) + fn = next(n for n in ast.walk(tree) if isinstance(n, ast.AsyncFunctionDef) and n.name == "derive_timebox_intent") + starts = [ + n for n in ast.walk(fn) + if isinstance(n, ast.Return) and isinstance(n.value, ast.Call) + and getattr(n.value.func, "id", None) == "StartSession" + ] + # Exactly one, and it is the empty-text start. + assert len(starts) == 1 +``` + +Adjust the last test's attribute path (`_core.model_client.calls`) to how `TimeboxingIntentInterpreter` actually holds its client (`self.model_client` per the class body: use `runtime.timeboxing_intent_interpreter.model_client.calls`). + +- [ ] **Step 2: Run them to verify they fail** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_no_session_is_judged.py -q` +Expected: FAIL — `_display_context` returns `"planning_day"`, and `derive_timebox_intent` returns `StartSession()` for the question and cancel cases. + +- [ ] **Step 3: Make the fresh session a judged state** + +In `timeboxing_intents.py`: + +(a) `InterpretedTimeboxTurn.decision` Literal: add `"start"`. + +(b) At the top of `_display_context`, before the `cancelled` check: + +```python + if snapshot.planning_day is None and not any( + artifact.kind is ArtifactKind.PLANNING_DAY for artifact in snapshot.artifacts + ): + # Before a day is even proposed there is still something to decide: + # start the session, ask about the calendar, or walk away. This used + # to be an unconditional StartSession with no model asked (#318). + return "no_session", ("start", "question", "cancel"), pending +``` + +(`pending` is computed on the function's first line; it is `None` here by construction.) + +(c) In `_intent_from_interpreted`, after the `question` branch from Task 1: + +```python + if interpreted.decision == "start": + return StartSession() +``` + +Import `StartSession` if not already imported. + +In `timeboxing_host.py`, `derive_timebox_intent` becomes: + +```python +async def derive_timebox_intent( + runtime, + snapshot: PlanningSessionSnapshot, + *, + user_text: str, +) -> TimeboxIntent: + """Turn one Slack reply into a typed intent, never by reading the words. + + Every reply with words in it is interpreted -- including the first one. + Before a day is proposed the surface offers start, question and cancel, + and the interpreter says which; an unconditional start here was what + turned "Is it planned?" into a five-stage session (2026-09-05 03:43). + Only an empty opening turn starts without asking: there is nothing to + read, and the auto-start and a bare command arrive that way. + + The schema-bound interpreter names the decision; the host binds the date, + the artifact identity, the question being answered and the user's own + words from state it already trusts. + """ + if not user_text.strip(): + fresh = snapshot.planning_day is None and not any( + artifact.kind is ArtifactKind.PLANNING_DAY for artifact in snapshot.artifacts + ) + return StartSession() if fresh else Advance() + interpreter = getattr(runtime, "timeboxing_intent_interpreter", None) + if interpreter is None: + # Falling back to a guess would give this route two behaviours, and the + # wrong one would be the silent one. + raise AdaptiveDependencyUnavailable("no intent interpreter is configured") + return await interpreter.interpret(user_text, snapshot) +``` + +- [ ] **Step 4: Run the new tests and every test that drives `derive_timebox_intent`** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/unit/test_no_session_is_judged.py tests/unit/test_timeboxing_intents.py tests/unit/test_turn_cancels_ladder.py tests/unit/test_stage_receipts_in_the_turn.py tests/unit/test_adaptive_turn_marks_timeboxing_active.py tests/unit/test_timebox_failure_card_tells_the_truth.py tests/unit/test_stage_panel_in_the_turn.py tests/e2e/test_stage1_panel_walk.py tests/integration/test_harness_timeboxing_session_route.py -q` +Expected: PASS. If a test asserted the old unconditional start with typed text on a fresh session, read it: if it drove real words through the opening turn, give its stub interpreter a `{"decision": "start", "facts": []}` response rather than deleting the assertion. + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests -m "not slow" -q` +Expected: only the three pre-existing failures. + +- [ ] **Step 5: Commit** + +```bash +git add src/fateforger/slack_bot/timeboxing_host.py src/fateforger/slack_bot/timeboxing_intents.py tests/unit/test_no_session_is_judged.py +git commit -m "feat(timeboxing): before a day is locked the reply is judged — start, question or cancel (#318) + +The unconditional StartSession before a planning day is gone, with the +docstring claim that there was nothing to decide. A fresh session is a +no_session state the interpreter reads; an empty opening turn still starts. +cancel rides along, which is #299's option 3. + +Co-Authored-By: Claude Fable 5.1 " +``` + +--- + +## Task 5: Eval — question-vs-start, question-vs-facts, break-it check (#319) + +**Files:** +- Create: `tests/integration/test_eval_timebox_question.py` +- Reference: `tests/integration/test_eval_planning_card_intent.py` (the pattern) + +**Interfaces:** +- Consumes: `QUESTION_PARAGRAPH`, `_TIMEBOX_PROMPT_FRAGMENT_BASE`, `_TIMEBOX_PROMPT_FRAGMENT` from Task 1; the `no_session` state from Task 4; `AskQuestion`, `StartSession`, `CancelSession`, `ProvidePlanningFacts`, `ReviseArtifact`. + +- [ ] **Step 1: Make the key available in the worktree** + +The worktree has no `.env` (gitignored). From the worktree root: `cp ../../.env .env`. Then, for every eval run in this task, load it into the shell first: `set -a; source .env; set +a`. Do not commit `.env` (it is ignored; `git status` must not show it). + +- [ ] **Step 2: Write the eval** + +Create `tests/integration/test_eval_timebox_question.py`: + +```python +# tests/integration/test_eval_timebox_question.py +"""Quality of the timeboxing surface's question decision against the live model. + +Unit tests stub the model and prove the plumbing; this proves the prompt. +Every case resamples -- one draw tests the model's luck -- and the rate is the +assertion. No temperature pin. The break-it check strips the discriminating +paragraph and expects the questions to stop being read as questions: a +discriminator that passes without its discriminating sentence is not one. +""" + +from __future__ import annotations + +import asyncio +import os +import traceback +from datetime import date + +import pytest + +pytestmark = [ + pytest.mark.slow, + pytest.mark.skipif(not os.environ.get("OPENROUTER_API_KEY"), reason="OPENROUTER_API_KEY not set"), +] + +SAMPLES = 8 +THRESHOLD = 7 + + +def _report(results: list) -> str: + lines = [] + for r in results: + if isinstance(r, BaseException): + lines.append("".join(traceback.format_exception(r)).rstrip()) + else: + lines.append(repr(r)) + return "\n---\n".join(lines) + + +def _fresh(): + from fateforger.agents.timeboxing.session_contracts import PlanningSessionSnapshot + return PlanningSessionSnapshot(session_key="D1:dm", revision=0, owner_user_id="U1") + + +def _committed(): + from fateforger.agents.timeboxing.session_contracts import ( + ArtifactKind, PlanningArtifact, PlanningDay, PlanningSessionSnapshot, + ) + receipt = PlanningArtifact.create( + kind=ArtifactKind.COMMIT_RECEIPT, revision=1, + payload={"tx_id": "tx_eval", "calendar_id": "primary", "calendar_backend": "google", "durable": True}, + dependency_revisions={}, + ) + return PlanningSessionSnapshot( + session_key="C1:1.0", revision=9, owner_user_id="U1", status="committed", + planning_day=PlanningDay.lock_default(value=date(2026, 9, 5), timezone="Europe/Amsterdam", lock_revision=1), + artifacts=[receipt], + ) + + +async def _intents(text: str, snapshot) -> list: + from fateforger.llm.factory import build_autogen_chat_client + from fateforger.slack_bot.timeboxing_intents import TimeboxingIntentInterpreter + + interpreter = TimeboxingIntentInterpreter(build_autogen_chat_client("planner_agent")) + + async def one(): + return await interpreter.interpret(text, snapshot) + + return await asyncio.gather(*(one() for _ in range(SAMPLES)), return_exceptions=True) + + +def _count(results: list, kind: type) -> int: + return sum(1 for r in results if not isinstance(r, BaseException) and isinstance(r, kind)) + + +QUESTIONS_FRESH = ["Is it planned?", "did you add the gym?", "what's on my calendar tomorrow?", "is there a planning session today?"] +STARTS = ["plan tomorrow", "let's timebox saturday", "start", "ok let's go"] +CANCELS = ["cancel this", "never mind, not today"] +QUESTIONS_COMMITTED = ["what did we settle on for lunch?", "when is deep work?"] +FACTS_COMMITTED = ["I sleep 00:30–08:30"] +REVISIONS_COMMITTED = ["move the work two hours later"] + + +@pytest.mark.parametrize("text", QUESTIONS_FRESH) +async def test_a_question_before_a_day_is_asked(text): + from fateforger.agents.timeboxing.session_contracts import AskQuestion + results = await _intents(text, _fresh()) + assert _count(results, AskQuestion) >= THRESHOLD, _report(results) + + +@pytest.mark.parametrize("text", STARTS) +async def test_a_start_before_a_day_starts(text): + from fateforger.agents.timeboxing.session_contracts import StartSession + results = await _intents(text, _fresh()) + assert _count(results, StartSession) >= THRESHOLD, _report(results) + + +@pytest.mark.parametrize("text", CANCELS) +async def test_a_cancel_before_a_day_cancels(text): + from fateforger.agents.timeboxing.session_contracts import CancelSession + results = await _intents(text, _fresh()) + assert _count(results, CancelSession) >= THRESHOLD, _report(results) + + +@pytest.mark.parametrize("text", QUESTIONS_COMMITTED) +async def test_a_question_after_commit_is_asked_not_revised(text): + from fateforger.agents.timeboxing.session_contracts import AskQuestion + results = await _intents(text, _committed()) + assert _count(results, AskQuestion) >= THRESHOLD, _report(results) + + +@pytest.mark.parametrize("text", FACTS_COMMITTED) +async def test_a_fact_after_commit_is_still_a_fact(text): + from fateforger.agents.timeboxing.session_contracts import ProvidePlanningFacts + results = await _intents(text, _committed()) + assert _count(results, ProvidePlanningFacts) >= THRESHOLD, _report(results) + + +@pytest.mark.parametrize("text", REVISIONS_COMMITTED) +async def test_a_revision_after_commit_is_still_a_revision(text): + from fateforger.agents.timeboxing.session_contracts import ReviseArtifact + results = await _intents(text, _committed()) + assert _count(results, ReviseArtifact) >= THRESHOLD, _report(results) + + +@pytest.mark.parametrize("text", QUESTIONS_FRESH[:2] + QUESTIONS_COMMITTED[:1]) +async def test_break_it_without_the_question_paragraph_questions_are_not_questions(text, monkeypatch): + """The paragraph is load-bearing. Strip it and the model has only a label.""" + import fateforger.slack_bot.timeboxing_intents as intents + from fateforger.agents.timeboxing.session_contracts import AskQuestion + monkeypatch.setattr(intents, "_TIMEBOX_PROMPT_FRAGMENT", intents._TIMEBOX_PROMPT_FRAGMENT_BASE) + snapshot = _fresh() if text in QUESTIONS_FRESH else _committed() + results = await _intents(text, snapshot) + assert _count(results, AskQuestion) < THRESHOLD, _report(results) +``` + +Check: does `TimeboxingIntentInterpreter.interpret` read `_TIMEBOX_PROMPT_FRAGMENT` at call time (module global lookup) so the monkeypatch takes effect? If it was bound as a default argument or captured at import, change the interpreter to read the module global at call time — that is a one-line change in `timeboxing_intents.py`, and it is in scope here. `ReviseArtifact` on a committed session needs a pending artifact: check `_intent_from_interpreted`'s `revise` branch against the committed snapshot's `_latest_artifact(COMMIT_RECEIPT)` and give `_committed()` whatever pending artifact the binding requires, mirroring the committed `_display_context` branch. + +- [ ] **Step 3: Run the eval** + +Run: `set -a; source .env; set +a; PYTHONPATH=src ../../.venv/bin/python -m pytest tests/integration/test_eval_timebox_question.py -m slow -q -p no:cacheprovider 2>&1 | tail -40` +Expected: every case ≥ 7/8; every break-it case < 7/8. Record the per-case counts in your report, including the close ones. + +If a case fails: the fix is to `QUESTION_PARAGRAPH` (Task 1's paragraph in `timeboxing_intents.py`), resampled until the case passes at ≥ 7/8 **and** the break-it cases still fail — not to the case list. Say in the report what you changed and the before/after counts. + +- [ ] **Step 4: Confirm the unit suite still passes and `.env` is untracked** + +Run: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests -m "not slow" -q` — only the three pre-existing failures. +Run: `git status --short` — `.env` must not appear. + +- [ ] **Step 5: Commit** + +```bash +git add tests/integration/test_eval_timebox_question.py +# plus src/fateforger/slack_bot/timeboxing_intents.py if the paragraph or the fragment lookup changed +git commit -m "test(timeboxing): the question decision is measured at n=8, and breaks when its paragraph is stripped (#319) + +Question-vs-start-vs-cancel on a fresh session and question-vs-fact-vs-revise +on a committed one, eight draws each, seven to pass. Without the question +paragraph the questions stop being read as questions, which is how we know +the paragraph and not luck is doing the work. + +Co-Authored-By: Claude Fable 5.1 " +``` + +--- + +## Self-review + +**Spec coverage.** §1 rule → Task 2. §2 intent/outcome/interpreter/fragment/cancelled → Task 1. §3 host answers, describe, failure loud → Task 3. §4 no_session with start/question/cancel, empty text → Task 4. §5 unit list → Tasks 1–4; eval list and break-it → Task 5. "What this does not do" → nothing planned for it. F1 not F2 → Task 2 writes both and ships F1. + +**Placeholder scan.** Every step names its file, its command and its expected result. Two places tell the implementer to *check* an existing signature before matching it (`PlanningArtifact.create`, `_stage_cards.shown`) — that is verification against real code, not a placeholder. + +**Type consistency.** `AskQuestion(question=)` / `Asked(question=)` in Tasks 1, 3, 4, 5. `describe_session(snapshot, card)` in Task 3 only. `_intent_from_interpreted(..., user_text=)` introduced in Task 1, used in Task 4. `QUESTION_PARAGRAPH` / `_TIMEBOX_PROMPT_FRAGMENT_BASE` introduced in Task 1, consumed in Task 5. `record_error(component="surface_intent", error_type="answer_failure")` in Task 3's code and test. diff --git a/docs/superpowers/plans/2026-09-12-asked-not-started-to-flash.md b/docs/superpowers/plans/2026-09-12-asked-not-started-to-flash.md new file mode 100644 index 00000000..1333e38f --- /dev/null +++ b/docs/superpowers/plans/2026-09-12-asked-not-started-to-flash.md @@ -0,0 +1,346 @@ +# Asked ≠ Started to the Flash Flip — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax. **Tasks 1–3 are one PR (#328); Tasks 4–7 are a second PR, started only after the first merges.** + +**Goal:** Land PR #328 (asked ≠ started) on today's `main`, then finish #406 — fit the timebox interpreter's prompt to the flash pin, bench flash against the row's current default, and put the flip to Hugo. + +**Architecture:** Two PRs in sequence. The first is a rebase of an existing, reviewed branch onto a `main` that moved 168 commits under it, followed by a re-run of its own evals on the interpreter row as it now stands. The second follows the shape that already worked for the planning card (`docs/superpowers/plans/2026-09-09-interpreter-fit-flash.md`): measure the loss, supply what is missing, resample, prove the clause is load-bearing, then bench the pins head-to-head with the serving host recorded per draw. + +**Tech Stack:** Python 3.11, Pydantic v2, AutoGen `OpenAIChatCompletionClient`, pytest (+ the bench plugin), OpenRouter, `scripts/bench/interpreter_tier.py`. + +**Tickets:** #328 (closes #316–#320) · #406 · map #333. **Principle (Hugo):** the cheapest, fastest configuration wherever it works; escalate only when empirical testing justifies it. + +## State this plan starts from (2026-09-12) + +- `main` is at `01b4ecc` (PR #409 merged 2026-09-11). The interpreter row `intent_interpreter` defaults to the **pro pin at reasoning `low`, cap 1024**; `narrow_schema` narrows fields and the decision `Literal` per state; the planning card carries `now` and two clauses; the bench records the serving host per draw. +- `feat/asked-not-started` (PR #328) is at `5c4b4ca`: **168 behind, 16 ahead**. It adds `AskQuestion`/`Asked`, `question` in every open state, the judged `no_session` state (`start`/`question`/`cancel`), `describe_session`, the host's `_answer_question`, and `tests/integration/test_eval_timebox_question.py`. Its eval numbers were taken on pro/`high`; that row no longer exists. +- Worktree `.worktrees/asked-not-started` is clean at `5c4b4ca`, tracks origin, and has `.env` (gitignored). +- A dry-run merge of `main` into #328 shows **four content conflicts** (resolutions in Task 1) and **eight more files touched by both sides** that auto-merge: `handlers.py`, `stage_cards.py`, `timeboxing_host.py`, `timeboxing_intents.py`, `test_adaptive_timeboxing.py`, `test_stage_receipts_in_the_turn.py`, `test_timebox_failure_card_tells_the_truth.py`, `test_timeboxing_intents.py`. Auto-merged is not verified: the suite and the evals are. +- No session named `admonish-1-8b` (the branch's previous driver) is live. The claim is a comment on #328 dated 2026-09-12. + +## Global Constraints + +- **No keyword/string/regex matching on user content, ever.** Decision names, field names and agent-type strings are identifiers this system minted and are exempt. +- **An agent never changes a model pin.** `.env` untouched. The `intent_interpreter` row's defaults change only on a bench record plus Hugo's word (Task 7), and never `.env`. +- **Evals sample n=8 and assert on the rate; never pin `temperature`; never assert an exact model output string in a unit test.** A prompt fix validated by one passing call has not been validated. Every prompt change is resampled, and its break-it check must still fail. +- **Never trade one case for another.** A regression means the change is too broad: narrow and resample. +- **Transport, length and judgement stay separate.** The bench enforces it; the reading must not fold them. +- **Worktree discipline.** PR A in `.worktrees/asked-not-started`; PR B in a fresh worktree from `main` (Task 4 creates it). Every pytest run is `PYTHONPATH=src ../../.venv/bin/python -m pytest …` from the worktree root. `.env` is gitignored: `set -a; source .env; set +a` before any eval; `git status` must never show it. +- **Long commands run in the foreground.** A backgrounded bench with nobody waiting for it stalled a previous session. +- **Suite before done:** `PYTHONPATH=src ../../.venv/bin/python -m pytest tests -m "not slow" -q`, expected all green (3,423 on `main` today). +- **Commits:** `(): (#NNN)` ending `Co-Authored-By: Claude Fable 5.1 `. Name files; never `git add -A`. Subagents commit in the worktree; **the controller pushes** (Hugo's permission gates force-pushes; the lease is stated per task). +- **Bench budget:** stop and report at $1.50 cumulative per task; never lower `SAMPLES`; re-run a rate-limited configuration rather than shrinking it. +- **Every PR body** carries the problem, the rubric proof (the actual counts), and a `## Before merging (Hugo)` checklist. + +--- + +## File Structure + +| File | Responsibility | Task | +|---|---|---| +| the four conflicting files (see Task 1) | rebase resolution | 1 | +| `tests/integration/test_eval_timebox_question.py` | builds on `build_intent_interpreter_client()`; the re-baseline | 2 | +| `scripts/bench/interpreter_tier.py` | `timebox_question` runs from this tree, not a peer worktree | 4 | +| `src/fateforger/slack_bot/timeboxing_intents.py` | the timebox prompt's discriminator (`_TIMEBOX_PROMPT_FRAGMENT_BASE`, `QUESTION_PARAGRAPH`, a new clause) | 5 | +| `tests/integration/test_eval_timebox_question.py` | break-it case for the new clause | 5 | +| `scripts/bench/results-interpreter-tier-.{json,md,reading.md}` | the flip record | 6 | +| `src/fateforger/llm/factory.py`, `core/config.py`, `tests/unit/test_intent_interpreter_client.py`, `docs/reference/setup/llm.md` | Hugo's ruling applied | 7 | + +--- + +## PR A — land #328 + +### Task 1: Rebase `feat/asked-not-started` onto `main` and resolve the four conflicts (#328) + +**Files:** +- Modify (conflict resolution only): `src/fateforger/agents/timeboxing/session_contracts.py`, `src/fateforger/agents/timeboxing/adaptive_timeboxing.py`, `src/fateforger/slack_bot/timeboxing_cards.py`, `tests/integration/test_harness_timeboxing_session_route.py` +- Modify (only if the suite says so): any of the eight auto-merged files + +**Interfaces:** +- Produces: `feat/asked-not-started` rebased onto `origin/main` at `01b4ecc` or later, suite green, ready for Task 2. +- Consumes: nothing. + +- [ ] **Step 1: Confirm the starting state and record the lease** + +From `/Users/hugoevers/VScode-projects/admonish-1/.worktrees/asked-not-started`: + +```bash +git fetch origin +git status --short # must be empty +git rev-parse --short HEAD # 5c4b4ca — this is the force-push lease value for the controller +git rev-list --left-right --count origin/main...HEAD # expect roughly 168 16 +``` + +If HEAD is not `5c4b4ca`, stop and report: someone moved the branch. + +- [ ] **Step 2: Rebase, expecting exactly four conflicting files** + +```bash +git rebase origin/main +``` + +Git stops at the first conflicting commit. Resolve each file as below, `git add` it, `git rebase --continue`, and repeat until the rebase completes. Conflicts may surface across several of the 16 commits; the same four rules apply wherever the same hunks appear. **If a conflict appears in a file not listed below, stop and report it with the hunk** — it is a change on `main` this plan did not anticipate. + +**(a) `session_contracts.py`, the `__all__` list** — `main` added `"Asking"` (an unrelated class from #259, a non-blocking question that rides beside an artifact) exactly where this branch added `"Asked"` and `"AskQuestion"`. Keep all three, alphabetical: + +```python + "Asked", + "Asking", + "AskQuestion", +``` + +**(b) `adaptive_timeboxing.py`, the import block** — same adjacency. Keep all three names in the import. + +**(c) `timeboxing_cards.py`, the failure-copy dict** — `main` added two entries (`"unknown_rule_uid"` and `"unpresentable_artifact"`, each with a comment) where this branch added `"nothing_to_cancel"`. Keep all three entries with their comments; order does not matter to the code. + +**(d) `test_harness_timeboxing_session_route.py`, three `ScriptedModel(...)` scripts** — `main` collapsed the canned replies to `ScriptedModel({"decision": "confirm_planning_day"})` (and one with `"day_type": "vacation"`), dropping the `"facts": []` padding that #440's narrowing made unnecessary. This branch made the first typed turn on a fresh session a **judged** one (`no_session` offers `start`/`question`/`cancel`), so those scripts need a `start` reply before the `confirm`. Resolve to this branch's two-step sequence in `main`'s padding-free shape: + +```python + ScriptedModel( + {"decision": "start"}, + {"decision": "confirm_planning_day"}, + ) +``` + +and, for the third hunk: + +```python + ScriptedModel( + {"decision": "start"}, + {"decision": "confirm_planning_day", "day_type": "vacation"}, + ) +``` + +(`main`'s narrowed schema for the `no_session` state carries only `decision`, and #440's `_tolerate_padding` would accept `"facts": []` anyway — but the padding-free form is what `main` writes now, so match it.) + +- [ ] **Step 3: Verify the auto-merged files semantically, with the suite** + +```bash +PYTHONPATH=src ../../.venv/bin/python -m pytest tests -m "not slow" -q +``` + +Expected: green. If red, the failure is in one of the eight auto-merged files. The two most likely shapes, and their fixes: + +- **`timeboxing_intents.py`** — `main`'s `_display_context` gained field narrowing per state via `narrow_schema(..., allowed_decisions=...)` and `_FIELDS_BY_DECISION`; this branch added the `no_session` branch (`("start", "question", "cancel")`) at the top and `"question"` to every open state's tuple. Both must survive: `no_session` must sit **after** the `cancelled` check (a session cancelled at zero artifacts stays closed — Task 4 of the 2026-09-09 plan ruled this), and `"question"`/`"start"` need no entry in `_FIELDS_BY_DECISION` because they carry no fields. `tests/unit/test_surface_intent_schema_narrowing.py` and `tests/unit/test_no_session_is_judged.py` (this branch) both must pass. +- **`timeboxing_host.py`** — this branch replaced the unconditional `return StartSession()` in `derive_timebox_intent` with the judged path (empty text on a fresh session still starts; typed words are interpreted). `main` did not touch that function, so this branch's version should win cleanly; `tests/unit/test_no_session_is_judged.py`'s AST guard pins it. +- **`handlers.py` / `stage_cards.py`** — this branch added `_answer_question` and the `Asked` branch in `_run_adaptive_timebox_turn`, and `describe_session` in `stage_cards.py`. `main` reshaped both files around them (#213, #259, #409). If a test in `tests/unit/test_asked_is_answered_in_the_turn.py` or `test_describe_session.py` fails, the fix is to re-seat the branch's code against `main`'s current structure — not to change what it asserts. + +Fix only what the suite names. Do not refactor. + +- [ ] **Step 4: Commit the resolution, if the rebase produced one** + +A clean rebase rewrites the 16 commits in place and needs no extra commit. If Step 3 required edits, commit them as one: + +```bash +git add +git commit -m "fix(timeboxing): the asked-not-started branch re-seated on main after #213, #259 and #409 (#328) + + + +Co-Authored-By: Claude Fable 5.1 " +``` + +- [ ] **Step 5: Report the rebase shape** + +In the report: the four resolutions, any Step 3 edits with file:line, the suite count, and `git log --oneline origin/main..HEAD` (expect 16 or 17 commits). Do not push. + +--- + +### Task 2: The timebox eval runs on the interpreter row, and is re-baselined there (#328, #319) + +**Files:** +- Modify: `tests/integration/test_eval_timebox_question.py` (the client, ~line 158–166) + +**Interfaces:** +- Consumes: Task 1's rebased branch; `build_intent_interpreter_client()` from `fateforger.llm.factory` (on `main`). +- Produces: per-case counts on the row's current default (pro/`low`/1024) — #328's rubric proof, and #406's baseline for the two cases flash lost. + +- [ ] **Step 1: Switch the client** + +At the eval's client construction (currently `build_autogen_chat_client("timeboxing_agent")` — the pin production stopped using for this interpreter when #336 merged), use the row: + +```python + from fateforger.llm.factory import build_intent_interpreter_client + interpreter = TimeboxingIntentInterpreter(build_intent_interpreter_client()) +``` + +Keep the import function-local, as the file's other imports are. Also correct the eval's docstring or comments if they name `timeboxing_agent` as the production client. + +- [ ] **Step 2: Confirm the guard sees it** + +`tests/unit/test_intent_interpreter_client.py` guards `src/fateforger/`, not `tests/`, so nothing enforces this in the suite. Run the file's collection to prove it imports: `PYTHONPATH=src ../../.venv/bin/python -m pytest tests/integration/test_eval_timebox_question.py --collect-only -q` → 19 items. + +- [ ] **Step 3: Run the eval on the row's default — the re-baseline** + +```bash +set -a; source .env; set +a +PYTHONPATH=src ../../.venv/bin/python -m pytest tests/integration/test_eval_timebox_question.py -m slow -q -s -p no:cacheprovider +``` + +Record every case's `[eval]` line. Expected: every positive case ≥ 7/8 (on 2026-09-06, at pro/`high`, they were 8/8 with one 7/8), and every break-it case flips. **This is the first measurement of these cases on the narrowed schema and at `low` effort.** If a positive case falls below 7/8, stop and report with the breakdown — do not touch the prompt; a regression here is either the schema narrowing or the effort drop, and both are Hugo's rulings to revisit. + +- [ ] **Step 4: Run it once more on flash, for #406's baseline** + +```bash +LLM_MODEL_INTENT_INTERPRETER="$OPENROUTER_DEFAULT_MODEL_FLASH" \ +LLM_REASONING_EFFORT_INTENT_INTERPRETER=minimal \ +PYTHONPATH=src ../../.venv/bin/python -m pytest tests/integration/test_eval_timebox_question.py -m slow -q -s -p no:cacheprovider +``` + +Record every case. Expected: `test_a_revision_after_commit_is_still_a_revision` and `test_a_fact_after_commit_is_still_a_fact` below the bar (on 2026-09-06 they were 1/8 and 6/8 on flash, against pro/`high`). These two counts, on this schema, are what Task 5 starts from. Nothing is changed on this evidence — it is recorded. + +- [ ] **Step 5: Package suite, then commit** + +```bash +PYTHONPATH=src ../../.venv/bin/python -m pytest tests -m "not slow" -q +git add tests/integration/test_eval_timebox_question.py +git commit -m "test(timeboxing): the question eval runs on the interpreter row, and is re-baselined at pro/low (#328, #319) + +The eval built its own client on timeboxing_agent — the pin production stopped +using for this interpreter when #336 landed. It now builds the row. Re-run on +the row's default (pro, low, 1024): . On flash: , which is #406's starting point. + +Co-Authored-By: Claude Fable 5.1 " +``` + +--- + +### Task 3: PR #328 refreshed, reviewed, and merged (controller) + +Not a subagent task. The controller: + +- [ ] Rewrites #328's body: the problem (unchanged), **the rubric proof from Task 2 on the row as it now stands** (the old body's 19/19 was on pro/`high`), the flash baseline as a forward pointer to #406, the rebase note (what `main` brought: #213, #259, #409, #414), and a `## Before merging (Hugo)` checklist (accept the two-step harness scripts; accept the eval's new client; confirm the `no_session`-after-`cancelled` ordering; note the two flash losses are #406's). +- [ ] Dispatches one whole-branch review over `origin/main..HEAD` (16–17 commits) on the most capable model, with the 2026-09-05 spec (`docs/superpowers/specs/2026-09-05-asked-not-started-design.md`) as the requirements and the rebase resolutions as named risks. One fix wave if needed, one scoped re-review. +- [ ] Hands Hugo the push: `git -C .worktrees/asked-not-started push --force-with-lease=feat/asked-not-started:5c4b4ca origin feat/asked-not-started` (the lease is Task 1 Step 1's HEAD; if Hugo allows the command, the controller runs it). +- [ ] After Hugo merges: fast-forward the shared checkout's `main`, `demo.py restart slack-bot` (and `tmbx` if `demo.py status` says it is stale), check the bot log for the identity line and zero errors, close #316–#320 by hand if the merge did not (GitHub's closing keyword covers only the first issue in a comma list). + +--- + +## PR B — #406's timebox half, and the flip + +### Task 4: The bench runs the timebox eval from this tree, and measures flash's losses on it (#406) + +**Files:** +- Create worktree: `.worktrees/flash-flip` on branch `feat/406-flash-flip` from `origin/main` (after Task 3's merge) +- Modify: `scripts/bench/interpreter_tier.py` (`PEER_WORKTREE` and the `timebox_question` entry in `EVALS`) +- Create: `scripts/bench/results-interpreter-tier-.{json,md}` + `.reading.md` (measurements only) + +**Interfaces:** +- Consumes: `main` with #328 merged (the eval, `AskQuestion`, `no_session` all present). +- Produces: a record naming which timebox cases flash loses on the current schema and row; the `timebox_question` eval runnable from any checkout. + +- [ ] **Step 1: Worktree** + +From the repo root: `git fetch origin && git worktree add .worktrees/flash-flip -b feat/406-flash-flip origin/main && cp .env .worktrees/flash-flip/.env`. Work there from now on. + +- [ ] **Step 2: Drop the peer-worktree indirection** + +In `scripts/bench/interpreter_tier.py`, `EVALS["timebox_question"]` points at `PEER_WORKTREE` (`WORKTREE.parent / "asked-not-started"`) because the eval only existed there. It is on `main` now. Change the entry to `(WORKTREE, "tests/integration/test_eval_timebox_question.py", "interpreter")` — note the kind changes from `"timeboxing"` to `"interpreter"` too, because the eval now builds the row (Task 2), so the row's env overrides are the right knobs. Delete `PEER_WORKTREE`. Update the module docstring where it explains the peer worktree. + +Unit-level check: `PYTHONPATH=src ../../.venv/bin/python -c "import runpy; m=runpy.run_path('scripts/bench/interpreter_tier.py', run_name='x'); print(m['EVALS']['timebox_question'][0])"` prints this worktree's path. + +- [ ] **Step 3: Run the two configurations, interleaved, twice** + +```bash +set -a; source .env; set +a +PYTHONPATH=src ../../.venv/bin/python scripts/bench/interpreter_tier.py --date --configs pro-low-1024,flash-minimal-1024 +``` + +Check the runner's repeat/interleave support as it stands after #409 (it ran `pro-high-1024, pro-low-1024, pro-high-1024, pro-low-1024` on 2026-09-11; use the same mechanism). Both configurations exist in `CONFIGS` already. Provider is recorded per draw by the plugin. + +- [ ] **Step 4: The reading — measurements only** + +Hand-write `scripts/bench/results-interpreter-tier-.reading.md`: which `timebox_question` cases flash loses that pro/`low` holds, with counts from both runs; the same for the other two evals (expected: none new — the planning card was fitted on 2026-09-09); truncation by provider; cost. **No ruling and no prompt change in this task.** End with "Ruling: none — this record is Task 5's starting point." + +- [ ] **Step 5: Commit** + +```bash +git add scripts/bench/interpreter_tier.py scripts/bench/results-interpreter-tier-.json scripts/bench/results-interpreter-tier-.md scripts/bench/results-interpreter-tier-.reading.md +git commit -m "bench(llm): the question eval runs from this tree, and flash's losses on it are measured against pro/low (#406) + +Co-Authored-By: Claude Fable 5.1 " +``` + +--- + +### Task 5: The timebox prompt gets the discriminator flash is missing (#406) + +**Files:** +- Modify: `src/fateforger/slack_bot/timeboxing_intents.py` (`_TIMEBOX_PROMPT_FRAGMENT_BASE`, `QUESTION_PARAGRAPH`, and a new named clause) +- Modify: `tests/integration/test_eval_timebox_question.py` (break-it case(s)) + +**Interfaces:** +- Consumes: Task 4's record — the list of cases flash loses. +- Produces: a prompt on which flash holds those cases at ≥ 7/8, with break-it cases proving each new clause is load-bearing, and pro/`low` unchanged. + +**The known loss, and the shape of the fix.** On 2026-09-06, flash read `"move the work two hours later"` on a committed day as `provide_facts` 6 times in 8 instead of `revise` (pro: 8/8). The committed state's schema now offers exactly `("provide_facts", "revise", "question")` (after #440), which removes some of the noise — Task 4 says how much. What is missing is a discriminator between *a fact about the day* and *an instruction against the plan*: the model has nothing to key off, which is the `project`/`permanent` shape from CLAUDE.md. The planning card's fix on 2026-09-09 followed this exact sequence and it is the sequence here: + +- [ ] **Step 1: Confirm the loss on the current schema** — from Task 4's record, not a new run. If flash already holds every case at ≥ 7/8 there, **this task is a no-op**: skip to Task 6 and say so. + +- [ ] **Step 2: Check what the request already carries before writing prose.** The committed state's payload includes `display_state="committed"`, the allowed decisions, and `open_question`/`pending_artifact_kind` context. Read `_display_context`'s committed branch and the `SurfaceView` context it produces. If a *fact* the model would need is absent (as `now` was for the planning card), supply it and re-measure before any clause. Record the result either way. + +- [ ] **Step 3: The smallest clause, named and stripped-able.** Add a module constant beside `QUESTION_PARAGRAPH` — e.g. `REVISE_PARAGRAPH` — and compose `_TIMEBOX_PROMPT_FRAGMENT = _TIMEBOX_PROMPT_FRAGMENT_BASE + QUESTION_PARAGRAPH + REVISE_PARAGRAPH`. Content: what distinguishes an instruction against the plan (a change the user wants made to something already on the day: move, shift, swap, drop, extend) from a fact about the day (a boundary or activity the day must hold, stated as true). Say it as a relation over what the surface shows — the committed receipt — not as a phrase list. Keep it under five sentences. + +- [ ] **Step 4: Resample on flash, n=8** with the flash env overrides (Task 2 Step 4's command). The lost cases must reach ≥ 7/8; **every other case in the file must hold its Task 4 count**. A regression means the clause is too broad — narrow and resample. + +- [ ] **Step 5: Break it on purpose.** Add a break-it case per new clause in the eval, following the file's existing `test_break_it_*` pattern (monkeypatch `_TIMEBOX_PROMPT_FRAGMENT` to the composition without the new clause; assert the flip — the lost decision reappears, not merely that the right one drops). Run it on flash: the clause must be shown load-bearing. Under the bench, `INTERPRETER_TIER_CONFIG` buckets these as "break-it unbroken" on pins where they do not break; follow the xfail-off-flash-except-under-the-bench pattern the planning-card eval uses if a plain run on pro would otherwise go red. + +- [ ] **Step 6: Pro/`low` did not regress.** Re-run the file on the row's default. Every case holds Task 2's count. + +- [ ] **Step 7: Package suite, commit** + +```bash +git add src/fateforger/slack_bot/timeboxing_intents.py tests/integration/test_eval_timebox_question.py +git commit -m "feat(timeboxing): the prompt says what an instruction against the plan is, so the cheap pin can tell it from a fact — measured (#406) + + + +Co-Authored-By: Claude Fable 5.1 " +``` + +--- + +### Task 6: The flip bench — flash/`minimal` against pro/`low`, all three evals, cap re-read on flash (#406, #325) + +**Files:** +- Create: `scripts/bench/results-interpreter-tier-.{json,md,reading.md}` (a second record for the day if Task 4's date is the same; use `--date -flip` or the next day) + +**Interfaces:** +- Consumes: Task 5's prompt. +- Produces: the record Hugo rules on. + +- [ ] **Step 1: The matrix.** `flash-minimal-1024`, `pro-low-1024`, and — because the 2026-09-06 record found one correct 8/8 flash case that carried a **4,839-token completed draw** which 1024 would have cut — `flash-minimal` (uncapped) and `flash-minimal-2048` as the cap re-read. Interleave flash and pro; two runs per configuration. Provider per draw. + +- [ ] **Step 2: The reading — measurements, then "Ruling: pending Hugo."** It must answer: does flash/`minimal` hold every judgement pro/`low` holds, across all three evals, with counts from both runs? Truncation at 1024 on flash by provider, and whether any *correct* flash answer was cut (compare the uncapped run's completion-token maxima per case against 1024). Latency median/p90 and cost per configuration. The routing caveat if the host mix moved between runs. Draw-level p-values only with the independence caveat the 2026-09-11 reading carries, or not at all. + +- [ ] **Step 3: Commit the record.** + +--- + +### Task 7: Hugo's ruling applied (#406, controller + one small task) + +The controller puts the record to Hugo with three options: flip to flash/`minimal` at the cap the reading supports; keep pro/`low`; or a narrower ask (a specific case to fit first). Then one subagent task applies the ruling: + +- [ ] `src/fateforger/llm/factory.py`: the `intent_interpreter` row's model and effort defaults (and the cap constant, if the reading moved it); comments cite the record and the ruling. +- [ ] `tests/unit/test_intent_interpreter_client.py`: the default-row test asserts the new defaults, renamed to say so, red first. +- [ ] `docs/reference/setup/llm.md`'s "Surface interpreter model" section and `src/fateforger/slack_bot/README.md`: the defaults and the site table; the 2026-09-11 reading gets a "Superseded " note beside its ruling, appended, not rewritten. +- [ ] If the flip lands: `CLAUDE.md`'s role table already names the flash pin for routing — no edit; the row now matches it. `.env` lines are Hugo's; the PR checklist asks. +- [ ] A docs ticket for the round (CLAUDE.md rule), landed into the PR by a sonnet agent. +- [ ] Whole-branch review, one fix wave, one scoped re-review, PR under Hugo's rule, Hugo pushes and merges, restart the bot, close #406 and #319 by hand if needed. + +--- + +## What this plan does not do + +- Touch #321 (the receptionist's `"?"` heuristic), #337 (focus never outranks a surface, the general rule), or #350 (`confirm_planning_day` drops facts) — filed, sequenced after this chain, not planned here. +- Revisit the planning card's prompt or `now` block — measured and landed in #409. +- Change what the seam does when a runaway fires (#325's remaining question). + +## Self-review + +**Coverage.** #328: rebase (1), eval on the row + re-baseline (2), PR/merge (3). #406: measure flash's timebox losses on the current schema (4), fit the prompt with the measured sequence (5), flip bench incl. the cap re-read (6), ruling applied (7). #319: Task 2 Step 1. The 4,839-token cap concern from #409's checklist: Task 6 Step 1. + +**Placeholders.** Task 1 cannot pre-write conflict resolutions as diffs; it gives the exact resolution per hunk and the tests that pin each semantic risk. Task 5's clause text is deliberately not pre-written: its content depends on Task 4's measurement, and the plan says what it must express and how it is proven. `` and `` are filled by the implementer from the calendar; `` in commit bodies is filled from the run. + +**Type consistency.** `build_intent_interpreter_client()` in Tasks 2 and 4; `_TIMEBOX_PROMPT_FRAGMENT_BASE` / `QUESTION_PARAGRAPH` / `REVISE_PARAGRAPH` in Task 5 and its break-it; `EVALS["timebox_question"]` kind `"interpreter"` in Task 4 consistent with Task 2's client switch; config names `pro-low-1024`, `flash-minimal-1024`, `flash-minimal`, `flash-minimal-2048` all exist in `CONFIGS` on `main`. diff --git a/docs/superpowers/specs/2026-09-05-asked-not-started-design.md b/docs/superpowers/specs/2026-09-05-asked-not-started-design.md new file mode 100644 index 00000000..0a842644 --- /dev/null +++ b/docs/superpowers/specs/2026-09-05-asked-not-started-design.md @@ -0,0 +1,242 @@ +# Asked ≠ started — a question to the Schedular is answered, never turned into a session + +**Date:** 2026-09-05 +**Status:** Approved (design, Hugo, 2026-09-05); tickets charted on map #157 +**Extends:** `2026-09-03-planning-card-reply-seam-design.md` (the seam), `docs/architecture/proposal_object_contract.md` (the contract) +**Map:** #157 *Rebuild FateForger on the four-move architecture* + +## The incident + +03:43, 2026-09-05. Under the Admonisher's planning card (draft, "Not added yet"), Hugo typed +*"Is it planned?"*. The card's interpreter read it correctly — `decision: none`, a question, not a +press (`logs/llm_io_20260905_010643_9295.jsonl`, record 3). The reply was then routed to +`timeboxing_agent`, which opened a fresh five-stage session and posted the Stage-1 day-confirm +card. No model was consulted for that: the card is minted deterministically. + +Two faults, one visible and one underneath. + +1. **Routing.** Every DM turn pins `set_user_focus`, and the resolver only asked + `planning.owns_thread` when focus had *not* already chosen timeboxing. Planning was the last + resolver, not the first. Fixed in #310. +2. **The Schedular has no door that does not start a session.** `derive_timebox_intent` returns + `StartSession()` for any text before a day is locked, *by design, with no model asked* — its + docstring says "there is nothing to decide about". A committed session offers only + `provide_facts` and `revise`, and `InterpretedTimeboxTurn` has no `none`, so a question to + the agent that just planned the day is coerced into a fact or a revision. #287 (one-off + instructions filed as durable MUST rules) is the memory-side echo of that coercion. + +Hugo's expectation, stated: *either the timeboxing agent that just did the session answers, or +the Schedular that can pull my calendar sees there is no planning session and says so.* Both +wear the same "The Schedular" persona (`planner_agent` and `timeboxing_agent`, +`workspace.py:62-71`). + +## What already exists, and is kept + +- The receptionist prompt already routes *"is a session planned?"* to `planner_agent` for + calendar inspection (`receptionist/agent.py:31`). With #310 the planning-card thread reaches + it. **No change to the receptionist's routing.** +- `planner_agent` (`agents/schedular/agent.py`) holds the calendar MCP tools and answers in + prose (`reflect_on_tool_use=True`, #180). It is the answerer. +- The shared surface interpreter (`surface_intents.py`) and per-state `allowed_decisions` in + `timeboxing_intents._display_context`. The change is one more decision per state, not a new + interpreter. + +## Decisions (Hugo, 2026-09-05) + +- **S2 — a question is a kernel outcome.** `AskQuestion` joins `TimeboxIntent`; `kernel.turn` + returns `Asked` with the snapshot untouched; the host renders `Asked` by asking + `planner_agent`. The kernel stays pure and the single dispatcher; no host-side second + dispatcher. +- **N1 — the no-day short-circuit becomes a judged state.** `_display_context` gains + `no_session` with `("start", "question", "cancel")`. `StartSession` becomes an interpreter + decision like every other. `cancel` rides along: it is #299's option 3 for one tuple entry. +- **A1 — `planner_agent` answers**, given the session described plus the user's words. Not a + harness turn (revisit under #302), not a raw LLM call (a second calendar answerer that never + compounds). +- **D1 — `describe(snapshot)` is derived from `StageCard`**, the model `stage_cards.py` already + builds from a snapshot. One derivation, two renderers (Block Kit, prose). +- **F1 — focus demotes to `receptionist_agent`**, not the channel default, closing #310's + review note. The general rule (F2: focus never applies inside a bot-posted thread) is written + into the contract; F1 is what ships. + +## Section 1 — the rule, in the contract + +Landed as the last two bullets of item 7 under `docs/architecture/proposal_object_contract.md`'s +`## Contract` section (`proposal_object_contract.md:60-71`) — there is no separate "§7" heading; +7 is that section's seventh numbered entry, "A reply on a proposal thread has three outcomes, +never two." Both bullets have since grown past the two sentences drafted below: the first now +names the shipped F1 case (#310, #320) and what still waits on #302 (`:60-68`); the second links +back to this spec (`:69-71`). What was two sentences when this was written: + +> A thread whose root a surface posted belongs to that surface. User focus — the DM-wide +> memory of who last answered — never outranks that ownership; a new surface registers its +> root in the resolver chain (`handlers.route_slack_event`, the ordered resolvers before agent +> selection) or its threads will be routed by focus. + +> An agent that owns a workflow exposes `question` in every state its surface allows. Asked is +> not started, and asked is not revised: a question changes nothing in the session it is asked +> of. + +And the F1 change in `handlers.py`: when planning owns the thread and `timeboxing_agent` came +only from sticky `user_focus`, `agent_type` falls back to `receptionist_agent` (today: the +channel default, which is a no-op when that default is itself `timeboxing_agent`). + +## Section 2 — `AskQuestion` → `Asked` + +**Intent.** `AskQuestion(kind="ask_question", question: str)` in `session_contracts.py`, added +to the `TimeboxIntent` union. `question` is the user's words verbatim — the host binds it from +the Slack text, never from the model's output, so nothing the model wrote reaches the +answerer as if the user said it. + +**Outcome.** `Asked(kind="asked", question: str)` in the `TurnOutcome` union. The kernel returns +it without touching the snapshot: **no artifact, no fact, no assumption, no invalidation, and +the revision does not advance.** Whether the outcome is recorded in the session's outcome +envelope is the implementer's call; the invariant is that a subsequent `load` sees the same +snapshot revision as before the question. + +**Interpreter.** `InterpretedTimeboxTurn` gains `"question"` (and, for the `no_session` state, +`"start"`). `_display_context` adds `"question"` to every state's tuple — `no_session`, +`planning_day`, `skeleton`, `candidate`, `refine`, `committed`. The binding in `timeboxing_intents` +maps `question` → `AskQuestion(question=user_text)` and `start` → `StartSession()`. + +**Prompt fragment.** One paragraph on the timeboxing surface's fragment: a reply that asks about +the day, the plan, the calendar, or what was decided is `question`; a reply that supplies a +fact, a correction, or an instruction against the plan is what it was before. A question that +also carries a fact is a fact — the fact changes the day, the question does not. + +**Cancelled sessions.** `_display_context` returns `()` for `cancelled` and the interpreter +raises "does not accept another intent". That stays: a cancelled thread is closed. `question` +is not added there. + +## Section 3 — the host answers + +In `_run_adaptive_timebox_turn`, an `Asked` outcome is rendered by: + +1. `describe(snapshot)` — a prose rendering of the `StageCard` for the current snapshot: the + planning day and its type, the stage, what is decided (facts and assumptions, by owner), the + open question if any, and for a committed session the receipt (block count, calendar, + `tx_id`). Same fields the card shows; nothing the card does not show. +2. `runtime.send_message(TextMessage(content=f"{describe}\n\nThe user's question:\n{question}"), + recipient=AgentId("planner_agent", key=session_key))` — the same call shape the handoff path + uses. `planner_agent` reads the calendar if it needs to and answers in prose. +3. The progress card ("thinking…") becomes the answer, in-thread, under the Schedular persona. + No stage card is re-rendered; the session's card is exactly as it was. +4. Failure stays loud: the answerer raising or timing out is reported in-thread as the + `TurnFailed` copy is, and metered `record_error(component="surface_intent", + error_type="answer_failure")`. Never degraded to "no answer" silently; never retried into a + session start. + +Reversibility (map #157's second constraint): a question writes nothing, so there is nothing to +revert. The one delivery — the answer message — is a Slack post, edit-in-place only, like every +other reply. + +## Section 4 — the `no_session` state + +`derive_timebox_intent` no longer returns `StartSession()` unconditionally when no planning day +exists and no `PLANNING_DAY` artifact is pending. Instead `_display_context` returns +`("no_session", ("start", "question", "cancel"), None)` for that case and the interpreter is +asked, with `display_state="no_session"` and no offered options. The docstring's claim that +"there is nothing to decide about" is deleted with the code that made it true. + +`start` → `StartSession()`, the same object as before: the opening turn is unchanged. +`question` → `AskQuestion` → `Asked` → the host answers; **no session row is created at all: the +kernel answers a question over an in-memory snapshot, and a cancel with nothing to cancel is +refused before any row exists**, and revision stays 0. +`cancel` → `CancelSession()` before a day is locked (#299 option 3). A cancel that has nothing to +cancel — no row, or a row with no locked day and no artifacts — is refused as `TurnFailed` with +code `nothing_to_cancel`. + +The empty-text case (`Advance()`) is unchanged. + +## Section 5 — tests and evals + +**Unit — model stubbed, prove the plumbing.** + +- Interpreter stub returns `question` in each state → `kernel.turn` returns `Asked`; the + snapshot revision is unchanged after the turn; no artifact or fact was added. +- `Asked` → the host sends `planner_agent` one message containing the described snapshot and + the user's verbatim words; the reply text is posted in-thread; no stage card re-render. +- `no_session` + `question` → revision stays 0, no `StartSession` turn ran. +- `no_session` + `start` → identical outcome to today's short-circuit (pin by snapshot equality). +- `no_session` + `cancel` → `CancelSession` reaches the kernel. +- Answerer raises → in-thread failure line, `record_error` called, no session started. +- `describe(snapshot)` for a committed session names the receipt; for stage 3 names the + skeleton's decided items. Assert fields, not sentences. +- F1: planning owns the thread, `user_focus` is `timeboxing_agent`, channel default is + `timeboxing_agent` → routed to `receptionist_agent`. +- An AST guard: `derive_timebox_intent` contains no unconditional `StartSession()` return. + +**Eval — real model, `@slow`, n=8, threshold 7, no temperature pin** (the seam spec's pattern, +`tests/integration/test_eval_planning_card_intent.py`'s `SAMPLES`/`gather` shape): + +- **No case text appears verbatim in `QUESTION_PARAGRAPH`.** A case whose exact words are quoted + in the prompt measures recall of the prompt, not the judgement the prompt is meant to produce, + so every text the paragraph quotes is reworded to the same intent in other words (#319). +- `no_session`: question-vs-start-vs-cancel. Questions: *"has it been scheduled?"*, *"did you put + the gym in?"*, *"what's on my calendar tomorrow?"*, *"is there a planning session today?"*. + Starts: *"plan my day tomorrow"*, *"let's timebox saturday"*, *"kick it off"*, *"right, let's + begin"*. Cancels: *"cancel this"*, *"never mind, not today"*. +- `committed`: question-vs-facts-vs-revise. Questions: *"what did we decide about lunch?"*, + *"when's the deep-work block?"*. Facts: *"I sleep 00:30–08:30"* plus the two mixed + ask-and-supply texts below, which carry the positive half of the break-it check. Revise: + *"move the work two hours later"*. +- **A draw is retried once, and only when its exception carries a transport cause.** A draw that + reached a *wrong* decision — one outside `allowed_decisions`, output that does not fit the + narrowed schema, a binder refusal — is never retried: it is the measurement. A blind + `except Exception` here re-rolls the exact degenerate answer a stripped paragraph is supposed + to produce, which would hide the break-it result (#319). The endpoint's own failure rate is + #325's problem, reported per case and never asserted on. +- Break-it check, as the seam eval does: strip the prompt fragment's question paragraph and + confirm the discrimination collapses. **Measured 2026-09-05 (#319), it takes two families, and + a plain interrogative is neither of them:** stripping the paragraph does not move a pure + question at all — *"is it planned?"* and *"what did we settle on for lunch?"* still answer + `question` at 7/8 and 8/8 without it, because the `question` label in `allowed_decisions` + already carries them, so asserting on those tested the label. What does move: + - **fresh session, asked becomes started** — *"what's on my calendar tomorrow?"* answers + `AskQuestion` 8/8 with the paragraph and `StartSession` 8/8 without it (6/8, 6/8 and 7/8 to + `StartSession` on three earlier stripped draws, and `StartSession` never once in an + unstripped run). That is the regression this branch is named for, reproduced on demand. + - **committed session, the fact lost to the question** — *"did you move lunch? I sleep + 00:30–08:30"* answers `ProvidePlanningFacts` 8/8 with the paragraph and 1/8 without it + (*"is deep work still at 9? also I get up at 07:00"*: 8/8 against 0/8). + + Both assert the **flip** — the wrong decision outnumbering the right one — not the absence of + the right one. An absence-based bar is cleared by two lost calls with the paragraph doing + nothing, which is how the first version of this check "passed"; and a bar of + `StartSession >= 7` would have failed two of three honest stripped runs at 6/8. A lost draw + subtracts from both counts and can never manufacture a flip. A discriminator that passes + without its discriminating sentence is not one — and it has to be aimed at a case that needs + discriminating. + +Each case is sampled 8 times concurrently and asserted on the count. A prompt fix validated by +one passing call has not been validated (CLAUDE.md). + +## What this does not do + +- The admonisher's own router — #164, with the harness port. +- Re-keying the DM session under one-session-per-thread — #302. The `{channel}:dm` key stays; + a question in a DM asks the DM's session, which is the one that just ran. +- Renaming the two Schedulars. Which one answered is logged, not shown. +- F2 in code. The rule is written; the code change waits until DM session threads are + verifiable. +- The receptionist's `"?" in message.content` follow-up heuristic — a CLAUDE.md violation on + this path, filed as its own issue, not folded in. + +## Tickets (map #157, all `wayfinder:task`, AFK) + +| | Ticket | Blocked by | +|---|---|---| +| A | `AskQuestion` intent, `Asked` outcome, `question` in every `_display_context` state, prompt fragment | — | +| B | `describe(snapshot)` from `StageCard`; host renders `Asked` via `planner_agent`; failure loud | A | +| C | `no_session` state replaces the `StartSession()` short-circuit; `cancel` rides along | A | +| D | Eval: question-vs-start, question-vs-facts, break-it check | A, C | +| E | F1 focus demotion; the two rules in the contract doc | — | + +Waves: A ∥ E → B ∥ C → D. One worktree, one branch (`feat/asked-not-started`), subagents do not +commit; the controller commits between waves. + +## Related + +#310 (the routing half), #281 (the seam), #299 (cancel before a day), #287 (revision +instructions as rules), #302 (one session per thread), #164 (admonisher router), #180 +(planner answers in prose), #88 (surface contract rollout). diff --git a/scripts/bench/interpreter_tier.py b/scripts/bench/interpreter_tier.py index f5cd7014..603d5e38 100644 --- a/scripts/bench/interpreter_tier.py +++ b/scripts/bench/interpreter_tier.py @@ -82,15 +82,20 @@ #: eval name -> (worktree it lives in, path relative to it, which knobs it takes). #: ``timebox_question`` is #328's, unmerged; the bench runs it from that -#: worktree read-only, under the timeboxing agent's env overrides, because that -#: is the client it builds until #328 merges and its one line changes. -#: ``day_frame`` takes the interpreter's knobs: since #336 its interpreter -#: cases build on the interpreter row, and its judge cases stay on the judge -#: row at production default as a control (``CONTROL_ROWS``). +#: worktree read-only. It takes the interpreter's knobs like the other two: +#: since 353ce9d it builds ``build_intent_interpreter_client()``, so the +#: timeboxing agent's overrides reached nothing it built -- every configuration +#: would have run on the `intent_interpreter` row's own `.env` default and been +#: labelled as the configuration, and with ``INTERPRETER_TIER_CONFIG`` set the +#: break-it xfails are off, so those misfiled outcomes land in the ``unbroken`` +#: column. +#: ``day_frame`` takes the interpreter's knobs for the same row: since #336 its +#: interpreter cases build on the interpreter row, and its judge cases stay on +#: the judge row at production default as a control (``CONTROL_ROWS``). EVALS = { "planning_card": (WORKTREE, "tests/integration/test_eval_planning_card_intent.py", "interpreter"), "day_frame": (WORKTREE, "tests/integration/test_eval_day_frame.py", "interpreter"), - "timebox_question": (PEER_WORKTREE, "tests/integration/test_eval_timebox_question.py", "timeboxing"), + "timebox_question": (PEER_WORKTREE, "tests/integration/test_eval_timebox_question.py", "interpreter"), } #: configuration -> (which .env pin, reasoning effort, max_tokens or None for uncapped) @@ -178,7 +183,7 @@ def _env_for(config: str, kind: str, base: dict) -> dict: env["LLM_MODEL_TIMEBOXING_JUDGE"] = model env["LLM_REASONING_EFFORT_TIMEBOXING_JUDGE"] = effort env["LLM_MAX_TOKENS"] = str(cap if cap else 0) - else: # the #328 eval, on its own worktree, takes the timeboxing agent's knobs + else: # the timeboxing host agent's own row; no eval in EVALS builds on it today env["LLM_MODEL_TIMEBOXING"] = model env["LLM_REASONING_EFFORT_TIMEBOXING"] = effort env["LLM_MAX_TOKENS"] = str(cap if cap else 0) diff --git a/src/fateforger/agents/timeboxing/README.md b/src/fateforger/agents/timeboxing/README.md index 99af840d..d1d66fdb 100644 --- a/src/fateforger/agents/timeboxing/README.md +++ b/src/fateforger/agents/timeboxing/README.md @@ -90,6 +90,24 @@ Stage-gated timeboxing workflow that builds daily schedules via conversational r | `state.py` | Session persistence helpers. | | `flow.py` | Legacy flow logic (being replaced by GraphFlow). | +### Session Kernel + +| File | Responsibility | +|------|---------------| +| `session_contracts.py` | The typed `TimeboxIntent` / `TurnOutcome` unions and the `PlanningSessionSnapshot` the kernel persists. `AskQuestion` -> `Asked` is the one intent/outcome pair that changes nothing: no artifact, no fact, no assumption, no invalidation, and the outcome carries the question back exactly as the host received it -- never a model's paraphrase. | +| `adaptive_timeboxing.py` | `AdaptiveTimeboxing.turn`: one typed intent in, one replayable `TurnOutcome` out, over a snapshot a `PlanningSessionRepository` persists. Row creation is intent-gated, not text-gated -- see below. | + +`Asked` is returned before the run loop applies or resolves anything (`adaptive_timeboxing.py:602-607`), +so a `load` right after sees the same revision as before the question. Getting there does not +require a session to exist: `_turn_guarded`'s first branch is "no session, and the intent is +`AskQuestion`" -- answered over an unsaved in-memory snapshot, never a written row +(`adaptive_timeboxing.py:531-543`). The sibling branch is why a row is not created by default: +`CancelSession` against no row refuses as `TurnFailed(code="nothing_to_cancel")` +(`adaptive_timeboxing.py:544-548`), and only every other intent may `load_or_create` one +(`:549-552`). The rule matters because a written row is a session as far as the host is +concerned -- the nudge suppressor reads the session store -- so the first word typed into a DM +must not silence it by existing. + ### Stage 1 Elicitation | File | Responsibility | diff --git a/src/fateforger/agents/timeboxing/adaptive_timeboxing.py b/src/fateforger/agents/timeboxing/adaptive_timeboxing.py index 2f14c750..0925de2c 100644 --- a/src/fateforger/agents/timeboxing/adaptive_timeboxing.py +++ b/src/fateforger/agents/timeboxing/adaptive_timeboxing.py @@ -27,7 +27,9 @@ ArtifactApproval, ArtifactKind, ArtifactSnapshot, + Asked, Asking, + AskQuestion, AwaitingApproval, AwaitingUser, BlockerOption, @@ -526,9 +528,28 @@ async def _turn_guarded( ) -> TurnOutcome: """Execute after acquiring the repository's session exclusion seam.""" - snapshot = await self._repository.load_or_create( - request.session_key, owner_user_id=request.actor_user_id - ) + snapshot = await self._repository.load(request.session_key) + if snapshot is None: + # No session exists. Only an intent that starts one may create the + # row: a question is answered over an in-memory snapshot and a + # cancel has nothing to cancel. Asked is not started, one layer + # down. A row written here would be a session as far as the host + # is concerned -- the nudge suppressor reads the store -- so the + # first word typed into a DM must not mint one. + if isinstance(request.intent, AskQuestion): + snapshot = PlanningSessionSnapshot.new( + session_key=request.session_key, + owner_user_id=request.actor_user_id, + ) + elif isinstance(request.intent, CancelSession): + return TurnFailed( + code="nothing_to_cancel", + message="There is no planning session to cancel yet.", + ) + else: + snapshot = await self._repository.load_or_create( + request.session_key, owner_user_id=request.actor_user_id + ) if request.actor_user_id != snapshot.owner_user_id: return TurnFailed( code="session_owner_mismatch", @@ -563,6 +584,28 @@ async def _turn_guarded( ), ) + if ( + isinstance(request.intent, CancelSession) + and snapshot.planning_day is None + and not snapshot.artifacts + ): + # A row exists but nothing was ever decided in it -- an envelope an + # earlier code path left behind, or a session that proposed a day + # and never locked one (#299). Writing `cancelled` over it closes a + # session that never opened, and the key it closes is the one the + # user's next message would arrive on. + return TurnFailed( + code="nothing_to_cancel", + message="There is no planning session to cancel yet.", + ) + + if isinstance(request.intent, AskQuestion): + # Asked is not started and not revised. Nothing is applied and + # nothing is saved: the revision the next load sees is the one + # this turn loaded. The host answers from the snapshot and the + # calendar; the kernel's whole job here is to say so. + return Asked(question=request.intent.question) + base_revision = snapshot.revision progress_sink = _BestEffortProgress(progress) applied, early_outcome = self._apply_intent(snapshot, request) diff --git a/src/fateforger/agents/timeboxing/session_contracts.py b/src/fateforger/agents/timeboxing/session_contracts.py index 23a4e12f..c8902705 100644 --- a/src/fateforger/agents/timeboxing/session_contracts.py +++ b/src/fateforger/agents/timeboxing/session_contracts.py @@ -511,6 +511,19 @@ class CancelSession(_StrictModel): kind: Literal["cancel_session"] = "cancel_session" +class AskQuestion(_StrictModel): + """A question about the day, the plan or the calendar. + + The one intent that changes nothing: the kernel returns `Asked` before it + applies or saves anything. `question` is the user's words as the host + received them -- never a model's paraphrase, so nothing the model wrote + reaches the answerer as if the user said it. + """ + + kind: Literal["ask_question"] = "ask_question" + question: str = Field(min_length=1) + + TimeboxIntent = Annotated[ Union[ StartSession, @@ -525,6 +538,7 @@ class CancelSession(_StrictModel): RestoreConstraint, GoBack, CancelSession, + AskQuestion, ], Field(discriminator="kind"), ] @@ -635,6 +649,13 @@ class Cancelled(_StrictModel): kind: Literal["cancelled"] = "cancelled" +class Asked(_StrictModel): + """The turn was a question. Nothing in the session moved; the host answers.""" + + kind: Literal["asked"] = "asked" + question: str = Field(min_length=1) + + class PlannerContinuation(_StrictModel): """The planner saying it needs another turn, and why. @@ -691,6 +712,7 @@ class TurnFailed(_StrictModel): AwaitingApproval, Committed, Cancelled, + Asked, TurnFailed, ], Field(discriminator="kind"), @@ -847,7 +869,9 @@ class PlanningResult(_StrictModel): "ArtifactKind", "ArtifactReady", "ArtifactSnapshot", + "Asked", "Asking", + "AskQuestion", "AwaitingApproval", "AwaitingUser", "BlockerOption", diff --git a/src/fateforger/slack_bot/README.md b/src/fateforger/slack_bot/README.md index 631ce0cf..da91117e 100644 --- a/src/fateforger/slack_bot/README.md +++ b/src/fateforger/slack_bot/README.md @@ -215,8 +215,29 @@ very session a Thursday card is already proposing. A day-naming clause added on whenever a surface's replies are time-relative ("later", "tonight", "saturday"): supply the fact first and measure, then add prose only against a gap the fact alone doesn't close. +Second implementation: the timeboxing surface (`timeboxing_intents.py`, `timeboxing_host.py`). +`_display_context(snapshot)` is the one place its states and their offered decisions are +declared -- `no_session`, `planning_day`, `skeleton`, `review_commit`, `capture`, `refine`, +`committed`, and `cancelled` (an empty tuple: a cancelled thread is closed for good). The typed +decision is `InterpretedTimeboxTurn`; `_intent_from_interpreted` binds it to a `TimeboxIntent` +(`session_contracts.py`) from host-trusted state -- the artifact identity, the pending question, +the day the host proposed -- never from anything the model itself asserts. + +`question` is offered in every open state. It binds to `AskQuestion`, whose kernel outcome is +`Asked`: the session moves nothing, and `handlers._answer_question` sends `planner_agent` the +described session plus the user's words verbatim, never a paraphrase. Before this, `no_session` +did not exist: any text before a day was locked returned `StartSession()` unconditionally with +no model asked, so a question such as "is it planned?" started a five-stage session (the +2026-09-05 03:43 incident). `no_session` now offers `("start", "question", "cancel")` instead, +and only an empty opening turn -- nothing typed, the auto-start and a bare command -- still +starts without asking (`derive_timebox_intent` in `timeboxing_host.py`). + +Eval: `tests/integration/test_eval_timebox_question.py`, alongside the planning card's own eval +and the day-frame eval, all three listed in `scripts/bench/interpreter_tier.py`'s `EVALS`. + Reference spec: - `docs/architecture/proposal_object_contract.md` +- `docs/superpowers/specs/2026-09-05-asked-not-started-design.md` ### Narrowing a surface's schema to what its state can express diff --git a/src/fateforger/slack_bot/handlers.py b/src/fateforger/slack_bot/handlers.py index 8af62583..ba83c5f4 100644 --- a/src/fateforger/slack_bot/handlers.py +++ b/src/fateforger/slack_bot/handlers.py @@ -42,6 +42,8 @@ from fateforger.agents.timeboxing.session_contracts import ( ApproveArtifact, ArtifactKind, + Asked, + AskQuestion, Cancelled, Committed, ConfirmPlanningDay, @@ -112,7 +114,11 @@ from fateforger.slack_bot.reply_guard import agent_reply_text from fateforger.slack_bot.stage_card_registry import StageCardRegistry, receipt_body, receipt_label from fateforger.slack_bot.stage_context import context_fold -from fateforger.slack_bot.stage_cards import date_stage_card +from fateforger.slack_bot.stage_cards import ( + StageCard, + date_stage_card, + describe_session, +) from fateforger.slack_bot.task_cards import ( FF_TASK_DETAILS_ACTION_ID, FF_TASK_EDIT_MODAL_CALLBACK_ID, @@ -1602,6 +1608,76 @@ def _timeboxing_kernel( +async def _answer_question( + *, + runtime, + session_key: str, + actor_user_id: str, + snapshot: PlanningSessionSnapshot, + card: StageCard | None, + question: str, + logger, +) -> SlackBlockMessage: + """Answer an `Asked` outcome through planner_agent, the calendar's answerer. + + The session is described from the card the user is looking at plus the + snapshot, and the question travels verbatim after it. One ask; a failure + is one failure line and one metered error, never a second ask and never + a session start. + """ + + content = ( + f"{describe_session(snapshot, card)}\n\n" + f"The user's question:\n{question}" + ) + try: + result = await runtime.send_message( + TextMessage(content=content, source=actor_user_id), + recipient=AgentId("planner_agent", key=session_key), + ) + except Exception as exc: # noqa: BLE001 - one failure shape reaches Slack + logger.error( + "question answer failed session_key=%s error_type=%s error=%s", + session_key, + type(exc).__name__, + exc, + exc_info=True, + ) + record_error(component="surface_intent", error_type="answer_failure") + return timebox_failure_message(snapshot=snapshot) + payload = _compact_slack_payload(**_slack_payload_from_result(result)) + text = payload.get("text", "") or "" + # No synthesised section block for a blockless answer. Every caller runs + # this message back through `_compact_slack_payload`, which clips block + # text at `SLACK_MAX_BLOCK_TEXT_CHARS` (1600) while plain `text` keeps + # `SLACK_MAX_TEXT_CHARS` (3900) -- and Slack renders `blocks` whenever they + # are there. Wrapping the answer in a block therefore delivered it cut at + # 1600 characters with the rest sitting unread in the fallback text. An + # empty list is dropped by `_compact_slack_payload`, so the answer takes + # the same text-only path every other agent reply takes. + return SlackBlockMessage(text=text, blocks=payload.get("blocks") or []) + + +async def _load_or_new( + repository, session_key: str, *, owner_user_id: str +) -> PlanningSessionSnapshot: + """The stored session, or an unsaved empty one -- never a written row. + + Creating the row here is how a question became a session: the Admonisher's + nudge suppressor reads the session store, so a revision-0 `open` row with + `updated_at=now` silences the planning reminder for an hour. The kernel is + the only thing that may create a session, and only for an intent that + starts one; the host reads. + """ + + stored = await repository.load(session_key) + if stored is not None: + return stored + return PlanningSessionSnapshot.new( + session_key=session_key, owner_user_id=owner_user_id + ) + + async def _run_adaptive_timebox_turn( *, runtime, @@ -1642,37 +1718,15 @@ async def _run_adaptive_timebox_turn( ) return timebox_failure_message() - # The activity tracker no longer decides whether the Admonisher nudges -- - # `dispatch_planning_reminder` reads the session store for that (#256). - # What it still owns is the idle timer: ten quiet minutes after the last - # turn it asks the guardian to reconcile, which is how an abandoned - # session earns its nudge back. - timeboxing_activity.mark_active( - user_id=actor_user_id, - channel_id=card_channel, - thread_ts=card_thread_ts, - ) - - # Any turn is activity on this session: it cancels a pending Admonisher - # ladder (the planning session that started itself, #164 increment A). - # Best-effort: a haunt failure must never block the turn. haunting_service = getattr(runtime, "haunting_service", None) - if haunting_service is not None: - try: - await haunting_service.record_user_activity( - topic_id=session_key, task_id=None, user_id=actor_user_id - ) - except Exception: - logger.exception("activity record failed session_key=%s", session_key) - record_error(component="session_start", error_type="cancel_failure") progress_card = HarnessProgressCard( client, channel=progress_channel, message_ts=progress_ts ) snapshot: PlanningSessionSnapshot | None = None try: - snapshot = await repository.load_or_create( - session_key, owner_user_id=actor_user_id + snapshot = await _load_or_new( + repository, session_key, owner_user_id=actor_user_id ) if action is not None: intent: TimeboxIntent = action.intent @@ -1682,6 +1736,37 @@ async def _run_adaptive_timebox_turn( runtime, snapshot, user_text=user_text ) expected_revision = snapshot.revision + if not isinstance(intent, AskQuestion): + # Asked is not started up here either. Both of these say "the user + # is planning": the tracker owns the idle timer that earns an + # abandoned session its nudge back, and `record_user_activity` + # cancels the pending Admonisher ladder (#164 increment A). A + # question is the user asking whether they have planned anything, + # and silencing the reminder that would answer it is the opposite + # of what they asked for. So both wait until the intent is known. + # The button path never carries a question, so it is unaffected. + # + # The activity tracker no longer decides whether the Admonisher + # nudges -- `dispatch_planning_reminder` reads the session store + # for that (#256). + timeboxing_activity.mark_active( + user_id=actor_user_id, + channel_id=card_channel, + thread_ts=card_thread_ts, + ) + # Best-effort: a haunt failure must never block the turn. + if haunting_service is not None: + try: + await haunting_service.record_user_activity( + topic_id=session_key, task_id=None, user_id=actor_user_id + ) + except Exception: + logger.exception( + "activity record failed session_key=%s", session_key + ) + record_error( + component="session_start", error_type="cancel_failure" + ) outcome = await kernel.turn( TurnRequest( session_key=session_key, @@ -1692,8 +1777,8 @@ async def _run_adaptive_timebox_turn( ), progress=KernelProgressSink(progress_card, session_key=session_key), ) - current = await repository.load_or_create( - session_key, owner_user_id=actor_user_id + current = await _load_or_new( + repository, session_key, owner_user_id=actor_user_id ) observer = getattr(runtime, "timeboxing_feedback_observer", None) if observer is not None: @@ -1707,9 +1792,17 @@ async def _run_adaptive_timebox_turn( type(exc).__name__, len(new_feedback), ) - if current.status != "open": + if current.status != "open" and not isinstance(intent, AskQuestion): # Committed or cancelled: the session is over, so the idle timer # has nothing left to watch. + # + # A question is exempt for the same reason it is exempt on the way + # in, and for a sharper one: `mark_inactive` is keyed by *user*, + # not by session, and `cancel_followups` drops the whole topic's + # ladder. A question typed into an already-committed thread ends + # nothing -- it did not move this session, and tearing down the + # user's idle timer here would drop the one watching whatever open + # session they have running elsewhere. timeboxing_activity.mark_inactive(user_id=actor_user_id) if haunting_service is not None: try: @@ -1747,6 +1840,20 @@ async def _run_adaptive_timebox_turn( finally: await progress_card.close() + if isinstance(outcome, Asked): + # Asked is not started and not revised: no card transition, no panel + # sync, no relabel. The thinking card becomes the answer. + shown = _stage_cards.shown(session_key) + return await _answer_question( + runtime=runtime, + session_key=session_key, + actor_user_id=actor_user_id, + snapshot=current, + card=shown.card if shown is not None else None, + question=outcome.question, + logger=logger, + ) + try: message, card = present_outcome( outcome, @@ -2785,7 +2892,11 @@ async def _update_constraints(thread_key: str) -> None: # An explicit per-thread binding does not lose here -- that one the # user asked for by name. if binding is None and agent_type == "timeboxing_agent": - agent_type = channel_default_agent or default_agent + # Not the channel default: when that default is itself + # timeboxing_agent the demotion was a no-op (#310's review). + # The receptionist is the one agent that refers rather than + # starts, which is what a card's thread needs. + agent_type = "receptionist_agent" else: # Asked whatever `agent_type` already says. It used to be asked only # when `agent_type` was not already `timeboxing_agent`, which in the @@ -3163,27 +3274,44 @@ async def _begin_timeboxing_session_surface( except Exception: pass - if channel != target_channel and payload.get("blocks"): - try: - permalink = await _permalink(target_channel, root_ts) - except Exception: - permalink = None + # The echo fires whenever the turn produced something to show, not + # only a card. `_answer_question` is the one result that carries + # no blocks -- its answer is plain text on purpose, because a + # section block clips at 1600 characters while `text` keeps 3900 -- + # so gating on blocks alone answered the question in the session + # thread and left the DM that asked it on a spinner forever. + if channel != target_channel and ( + payload.get("blocks") or payload.get("text") + ): try: dm_channel = channel if is_dm else "" if not dm_channel: dm = await client.conversations_open(users=[user]) dm_channel = (dm.get("channel") or {}).get("id") or "" if dm_channel: - dm_blocks = list(payload["blocks"]) - if permalink: - dm_blocks.extend( - open_link_blocks( - text="Progress is tracked in the session thread:", - url=permalink, - button_text="Go to Session Thread", - action_id="ff_open_thread", + dm_blocks = None + if payload.get("blocks"): + # Only a card gets the link block. With any block + # present Slack hides `text`, so wrapping an + # answer to carry the link would deliver it cut at + # 1600 -- and a thread permalink is not what + # someone who asked a question came for. + try: + permalink = await _permalink( + target_channel, root_ts + ) + except Exception: + permalink = None + dm_blocks = list(payload["blocks"]) + if permalink: + dm_blocks.extend( + open_link_blocks( + text="Progress is tracked in the session thread:", + url=permalink, + button_text="Go to Session Thread", + action_id="ff_open_thread", + ) ) - ) if is_dm: # DM origin: the DM's own "thinking..." message # becomes the card, so the DM stays the control @@ -3192,16 +3320,17 @@ async def _begin_timeboxing_session_surface( text=update["text"], blocks=dm_blocks ) else: - dm_payload = { + dm_payload: dict[str, object] = { "channel": dm_channel, "text": update["text"], - "blocks": dm_blocks, } + if dm_blocks: + dm_payload["blocks"] = dm_blocks dm_payload.update(_persona_payload(persona)) await client.chat_postMessage(**dm_payload) except Exception: logger.debug( - "Failed to DM timeboxing commit prompt", exc_info=True + "Failed to echo the timeboxing turn to the DM", exc_info=True ) await _maybe_update_timeboxing_thread_header( diff --git a/src/fateforger/slack_bot/stage_cards.py b/src/fateforger/slack_bot/stage_cards.py index 7da27392..ffb71ed5 100644 --- a/src/fateforger/slack_bot/stage_cards.py +++ b/src/fateforger/slack_bot/stage_cards.py @@ -682,6 +682,78 @@ def map_outcome( return None +def describe_session( + snapshot: PlanningSessionSnapshot, card: StageCard | None +) -> str: + """What the user is looking at, in prose, for an agent that cannot see it. + + The same fields the card renders and nothing the card does not show: the + prose renderer beside the Block Kit one, over the same `StageCard`. The + snapshot supplies what a card does not carry -- the day, the status, the + receipt -- so a session with no card on screen (a DM after a restart) + still describes itself. + """ + + lines: list[str] = [] + day = snapshot.planning_day + if day is None: + lines.append( + "The user is talking to their timeboxing session; no planning day " + "has been locked yet (the session has not started)." + ) + else: + lines.append( + f"The user is talking to their timeboxing session for " + f"{day.date.isoformat()} ({day.date.strftime('%A')}, " + f"{day.day_type.value} day, {day.timezone})." + ) + lines.append(f"Session status: {snapshot.status}.") + + if card is not None: + lines.append( + f"Stage {card.stage.index}/5 \u00b7 {card.stage.name}" + + (f" \u2014 {card.done}" if card.done else "") + ) + if card.context: + lines.append("Context in use: " + "; ".join( + f"{item.text} (from {item.source})" for item in card.context + )) + if card.decided: + lines.append("Decided so far: " + "; ".join( + f"{item.text} ({item.kind}" + + (f", filed by {item.filed_by}" if item.filed_by else "") + + ")" + for item in card.decided + )) + if card.asking is not None: + lines.append(f"Open question to the user: {card.asking.question}") + lines.append(f"Why it is needed: {card.asking.why_needed}") + if card.asking.options: + # The buttons the user is looking at, and what each would do. + # Without them an agent asked "what are my choices?" answers + # from a description of the question alone. + lines.append("Offered answers: " + "; ".join( + f"{option.label} ({option.effect})" + for option in card.asking.options + )) + if card.gate: + lines.append(f"Gate: {card.gate}") + if card.body: + lines.append("The card's body:\n" + card.body) + + receipt = next( + (a for a in reversed(snapshot.artifacts) if a.kind is ArtifactKind.COMMIT_RECEIPT), + None, + ) + if receipt is not None: + payload = receipt.payload if isinstance(receipt.payload, dict) else {} + lines.append( + "Commit receipt: " + + ", ".join(f"{k}={v}" for k, v in payload.items() if v is not None) + ) + return "\n".join(lines) + + __all__ = [ "STAGES", "ApproveControl", @@ -701,6 +773,7 @@ def map_outcome( "UndoControl", "date_stage_card", "commit_basis_notice", + "describe_session", "map_outcome", "stage", ] diff --git a/src/fateforger/slack_bot/timeboxing_cards.py b/src/fateforger/slack_bot/timeboxing_cards.py index a0ed3741..190777f8 100644 --- a/src/fateforger/slack_bot/timeboxing_cards.py +++ b/src/fateforger/slack_bot/timeboxing_cards.py @@ -228,6 +228,7 @@ def _undo_outcome_text(payload: dict) -> str: "This is the first step of the session, so there is nothing to go " "back to. Pick the day, or cancel." ), + "nothing_to_cancel": "There is no planning session to cancel yet.", # A cited rule that is not one of the day's. Retrying is the right move # and the sentence says so, because the citation is the planner's and the # next draft may well not repeat it -- but it must not read as "your plan diff --git a/src/fateforger/slack_bot/timeboxing_host.py b/src/fateforger/slack_bot/timeboxing_host.py index 864ec079..e70615d9 100644 --- a/src/fateforger/slack_bot/timeboxing_host.py +++ b/src/fateforger/slack_bot/timeboxing_host.py @@ -930,22 +930,24 @@ async def derive_timebox_intent( ) -> TimeboxIntent: """Turn one Slack reply into a typed intent, never by reading the words. - Until a day has even been proposed there is nothing to decide about, so no - model is asked: the session starts and the host puts its own date on screen. - From the moment that card exists the reply is interpreted -- including the - reply that confirms it. Skipping the interpreter there was what left the - date card answerable only by a press, and a session an agent drives by - typing could not get past it. + Every reply with words in it is interpreted -- including the first one. + Before a day is proposed the surface offers start, question and cancel, + and the interpreter says which; an unconditional start here was what + turned "Is it planned?" into a five-stage session (2026-09-05 03:43). + Only an empty opening turn starts without asking: there is nothing to + read, and the auto-start and a bare command arrive that way. The schema-bound interpreter names the decision; the host binds the date, - the artifact identity and the question being answered from state it already - trusts. + the artifact identity, the question being answered and the user's own + words from state it already trusts. """ - if snapshot.planning_day is None and not any( - artifact.kind is ArtifactKind.PLANNING_DAY for artifact in snapshot.artifacts - ): - return StartSession() if not user_text.strip(): + fresh = snapshot.planning_day is None and not any( + artifact.kind is ArtifactKind.PLANNING_DAY + for artifact in snapshot.artifacts + ) + if fresh: + return StartSession() return Advance() interpreter = getattr(runtime, "timeboxing_intent_interpreter", None) if interpreter is None: diff --git a/src/fateforger/slack_bot/timeboxing_intents.py b/src/fateforger/slack_bot/timeboxing_intents.py index 2c8de58a..d2a20911 100644 --- a/src/fateforger/slack_bot/timeboxing_intents.py +++ b/src/fateforger/slack_bot/timeboxing_intents.py @@ -22,6 +22,7 @@ Advance, ApproveArtifact, ArtifactKind, + AskQuestion, BlockerOption, CancelSession, ChooseBlockerOption, @@ -38,6 +39,7 @@ ProvidePlanningFacts, RestoreConstraint, ReviseArtifact, + StartSession, TimeboxIntent, elicited_fact_id, suspension_fact_id, @@ -106,6 +108,8 @@ class InterpretedTimeboxTurn(_StrictModel): "restore", "assume", "deny", + "question", + "start", ] facts: list[PlanningFactDraft] = Field(default_factory=list) revision_instruction: str | None = Field(default=None, min_length=1) @@ -213,7 +217,7 @@ class TimeboxActionEnvelope(_StrictModel): intent: TimeboxIntent -_TIMEBOX_PROMPT_FRAGMENT = """Extract facts only when the user actually supplies them. +_TIMEBOX_PROMPT_FRAGMENT_BASE = """Extract facts only when the user actually supplies them. Fact kinds you may extract: requested_activity (one per thing the user wants the day to hold, value a short description in their words) and day_frame (when they get up and when they go to sleep on the planning day, value @@ -240,6 +244,18 @@ class TimeboxActionEnvelope(_StrictModel): an elicited_statement fact in their words. """ +QUESTION_PARAGRAPH = """A reply that asks about the day, the plan, the calendar, or what was +decided -- "is it planned?", "did you add the gym?", "what did we settle on +for lunch?", "when is deep work?" -- is question. A reply that supplies a +fact, a correction, or an instruction against the plan is what it was +before. A reply that asks and also supplies a fact is that fact: the fact +changes the day and the question does not. A reply that asks to plan, to +timebox, or to get going -- "plan tomorrow", "let's do saturday", "start", +"ok let's go" -- is start, wherever the surface offers it. +""" + +_TIMEBOX_PROMPT_FRAGMENT = _TIMEBOX_PROMPT_FRAGMENT_BASE + QUESTION_PARAGRAPH + def _is_approved( snapshot: PlanningSessionSnapshot, artifact: PlanningArtifact @@ -306,6 +322,16 @@ def _display_context( pending = _pending_artifact(snapshot) if snapshot.status == "cancelled": return "cancelled", (), pending + # After the cancelled check, never before it: cancel is on offer below, so + # a fresh session can now be cancelled at zero artifacts, and a state that + # read "no day yet" first would hand that closed thread a start. + if snapshot.planning_day is None and not any( + artifact.kind is ArtifactKind.PLANNING_DAY for artifact in snapshot.artifacts + ): + # Before a day is even proposed there is still something to decide: + # start the session, ask about the calendar, or walk away. This used + # to be an unconditional StartSession with no model asked (#318). + return "no_session", ("start", "question", "cancel"), pending if snapshot.status == "committed": # The day is on the calendar and the user is still talking, which on # 2026-09-02 meant "move the work two hours later, I sleep 00:30-08:30" @@ -316,22 +342,30 @@ def _display_context( # not approve, which has nothing left to approve. return ( "committed", - ("provide_facts", "revise"), + ("provide_facts", "revise", "question"), _latest_artifact(snapshot, ArtifactKind.COMMIT_RECEIPT), ) if snapshot.planning_day is None: # Confirming belongs here as much as cancelling does. Without it a # session driven by typing cannot get past stage 0 at all, and the two # surfaces this project deliberately converged have diverged again. - return "planning_day", ("confirm_planning_day", "cancel"), pending + return ( + "planning_day", + ("confirm_planning_day", "cancel", "question"), + pending, + ) if pending is not None and pending.kind is ArtifactKind.SKELETON: return ( "skeleton", - ("provide_facts", "approve", "revise", "back", "cancel"), + ("provide_facts", "approve", "revise", "back", "cancel", "question"), pending, ) if pending is not None and pending.kind is ArtifactKind.VALIDATED_CANDIDATE: - return "review_commit", ("approve", "revise", "back", "cancel"), pending + return ( + "review_commit", + ("approve", "revise", "back", "cancel", "question"), + pending, + ) # Choosing is offered exactly while a question with options is open. It is # absent everywhere else because there would be no question to answer, and a # decision the session cannot honour is one the model can only waste a turn @@ -380,12 +414,13 @@ def _display_context( *choose, "back", "cancel", + "question", ), None, ) return ( "refine", - ("provide_facts", "advance", *choose, "back", "cancel"), + ("provide_facts", "advance", *choose, "back", "cancel", "question"), None, ) @@ -430,7 +465,7 @@ async def interpret( ), ) return _intent_from_interpreted( - interpreted, snapshot=snapshot, pending=pending + interpreted, snapshot=snapshot, pending=pending, user_text=user_text ) @@ -477,8 +512,22 @@ def _intent_from_interpreted( *, snapshot: PlanningSessionSnapshot, pending: PlanningArtifact | None, + user_text: str, ) -> TimeboxIntent: """Bind one schema decision to trusted host state.""" + if interpreted.decision == "question": + # The host's copy of the words, verbatim. The schema carries no text + # field for this decision on purpose: a paraphrase is the model's + # words reaching the answerer as if the user said them. + # + # Any `facts` the reading attached are discarded, deliberately. The + # prompt says a question that carries a fact is a fact, and the eval + # measures that at 8/8, so a `question` arriving with facts is the + # model contradicting itself -- and the words the user typed win over + # the structure the model wrapped them in. + return AskQuestion(question=user_text) + if interpreted.decision == "start": + return StartSession() if interpreted.decision == "confirm_planning_day": if pending is None or pending.kind is not ArtifactKind.PLANNING_DAY: raise ValueError("confirm_planning_day requires a proposed planning day") diff --git a/tests/e2e/test_stage1_panel_walk.py b/tests/e2e/test_stage1_panel_walk.py index 0c3f5964..9f8da472 100644 --- a/tests/e2e/test_stage1_panel_walk.py +++ b/tests/e2e/test_stage1_panel_walk.py @@ -104,7 +104,7 @@ class Repo: def __init__(self) -> None: self._loads = 0 - async def load_or_create(self, key, owner_user_id): + async def load(self, key): turn_index = min(self._loads // 2, len(snapshots) - 1) self._loads += 1 return snapshots[turn_index] diff --git a/tests/integration/test_eval_timebox_question.py b/tests/integration/test_eval_timebox_question.py new file mode 100644 index 00000000..a6a12e1e --- /dev/null +++ b/tests/integration/test_eval_timebox_question.py @@ -0,0 +1,538 @@ +# tests/integration/test_eval_timebox_question.py +"""Quality of the timeboxing surface's question decision against the live model. + +Unit tests stub the model and prove the plumbing; this proves the prompt. +Every case resamples -- one draw tests the model's luck -- and the rate is the +assertion. No temperature pin. + +Run it with `-s`; that is what makes the per-case counts visible: + + set -a; source .env; set +a + PYTHONPATH=src ../../.venv/bin/python -m pytest \ + tests/integration/test_eval_timebox_question.py -m slow -q -s \ + -p no:cacheprovider + +Two break-it families strip `QUESTION_PARAGRAPH`, because the paragraph does +two separable jobs and a plain interrogative exercises neither. With the +paragraph gone, "is it planned?" and "what did we settle on for lunch?" still +answer `question` at 7/8 and 8/8 -- `question` is already in +`allowed_decisions` and `GENERIC_PREAMBLE` says to choose from that list, so +asserting on those measured the label. What does move, measured 2026-09-05: + +* on a fresh session, "what's on my calendar tomorrow?" answers `start` at + 6/8, 6/8 and 7/8 across three stripped runs, against `question` 8/8 with + the paragraph and `start` never once -- asked becomes started, which is the + regression this branch is named for; +* on a committed session, "did you move lunch? I sleep 00:30-08:30" answers + with the fact 8/8 with the paragraph and 1/8 without -- the clause about a + reply that asks *and* supplies one. + +Both assert the *flip* -- the wrong decision outnumbering the right one -- +and not the absence of the right one. An absence-based bar is cleared by two +lost calls, which is how the first version of this check "passed" while the +paragraph was doing nothing at all. + +Whether the flip can be *read* is a property of the pin, and both families +were re-measured on 2026-09-12 at n=8 per case. Where it cannot be read the +case is an expected failure off the bench rather than a skip, so the bench -- +which sets `INTERPRETER_TIER_CONFIG` and buckets a `test_break_it_` failure +into its own `unbroken` column -- still sees the raw outcome: + +* on the flash pin at `minimal` nothing flips. The fresh question answers + `question` 7/8 with the paragraph stripped and `start` once; the mixed + texts keep the fact 8/8 and 8/8 on one run and 7/8 and 8/8 on the next, + which is the bar they clear *with* the paragraph. The paragraph is not + load-bearing there, and `_expected_unbroken_here` xfails both families; +* on the pro pin at `low` the fresh question flips cleanly -- 0 asked against + 7 and 8 started on two runs -- and so does "is deep work still at 9? also I + get up at 07:00", kept 0, 1, 1, 0 across four runs against asked 7, 6, 4, + 6. Those stay strict. "did you move lunch? I sleep 00:30-08:30" does not: + the stripped prompt runs past the `intent_interpreter` row's 1024-token cap + -- a cap that arrived with the row in #336, the old `timeboxing_agent` + client having had none -- and two to five of its eight draws die on + `LengthFinishReasonError`, their retries with them. That leaves three to + six decisions to read a flip off, and the four runs went kept 2 / asked 1, + kept 2 / asked 3, kept 3 / asked 2, kept 1 / asked 5: the flip read twice. + What is stable there is the truncation, not the judgement, so + `_cap_eats_the_sample_here` xfails that one parametrisation on that pin and + nothing else. Non-strict, so the runs that do flip report `xpass` rather + than turning a working break-it into a red one. + +A draw whose exception carries a transport cause reached no decision and is +retried once; the retry count is reported, never asserted (#325). A draw that +reached a *wrong* decision is never retried -- see `_is_transport`. +""" + +from __future__ import annotations + +import asyncio +import os +import traceback +from collections import Counter +from datetime import date + +import pytest + +pytestmark = [ + pytest.mark.slow, + pytest.mark.skipif(not os.environ.get("OPENROUTER_API_KEY"), reason="OPENROUTER_API_KEY not set"), +] + +SAMPLES = 8 +THRESHOLD = 7 + + +def _report(results: list, retries: int = 0) -> str: + lines = [f"{retries} draw(s) retried after reaching no decision"] + for r in results: + if isinstance(r, BaseException): + lines.append("".join(traceback.format_exception(r)).rstrip()) + else: + lines.append(repr(r)) + return "\n---\n".join(lines) + + +def _fresh(): + from fateforger.agents.timeboxing.session_contracts import PlanningSessionSnapshot + + return PlanningSessionSnapshot(session_key="D1:dm", revision=0, owner_user_id="U1") + + +def _committed(): + from fateforger.agents.timeboxing.session_contracts import ( + ArtifactKind, + PlanningArtifact, + PlanningDay, + PlanningSessionSnapshot, + ) + + # The receipt is what the committed state hands the binder as its pending + # artifact, so `revise` has something to name. Payload keys are the ones + # `timeboxing_host` actually writes -- a made-up shape here would prove + # the model reads a receipt this system never mints. + receipt = PlanningArtifact.create( + kind=ArtifactKind.COMMIT_RECEIPT, + revision=1, + payload={ + "committed": True, + "tx_id": "tx_eval", + "reason": None, + "candidate_digest": "d" * 64, + "calendar_backend": "google", + "durable": True, + }, + dependency_revisions={"validated_candidate": 1}, + ) + return PlanningSessionSnapshot( + session_key="C1:1.0", + revision=9, + owner_user_id="U1", + status="committed", + planning_day=PlanningDay.lock_default( + value=date(2026, 9, 5), timezone="Europe/Amsterdam", lock_revision=1 + ), + artifacts=[receipt], + ) + + +def _is_transport(exc: BaseException) -> bool: + """Whether a raise means "the endpoint gave us nothing", not "wrong answer". + + Exactly one raise site on the interpreter's path wraps a cause -- + `surface_intents.py`'s ``except Exception as exc: raise SurfaceIntentError( + ...) from exc``, which is the transport and JSON-parse layer. Every other + raise is a judgement the model actually made and lost, and none of them + sets ``__cause__``: + + * `SurfaceIntentError` for a decision outside `allowed_decisions` + (`surface_intents.py:205`), raised after the call returned; + * `ValidationError` for output that does not fit the narrowed schema + (`surface_intents.py:193`), re-raised as itself; + * `SurfaceIntentError` for a response carrying no string content + (`surface_intents.py:191`); + * the binder's own `ValueError`s -- `provide_facts` with no facts, + `revise` with no instruction, a `steer_not_today` naming a row that is + not on the card (`timeboxing_intents.py:559,602,606,615`). + + Retrying any of those would be the eval re-rolling until the run agreed + with it. The first is the exact degenerate answer a stripped paragraph is + *supposed* to produce, so swallowing it would hide the break-it result + this file exists to measure. + """ + + from fateforger.slack_bot.surface_intents import SurfaceIntentError + + return isinstance(exc, SurfaceIntentError) and exc.__cause__ is not None + + +async def _intents(text: str, snapshot) -> tuple[list, int]: + """`SAMPLES` concurrent draws, and how many of them had to be retried. + + A draw lost to the transport reached *no* decision, and folding that into + the rate reads a broken call as a misjudged one -- which is exactly how + this eval's first run made a working paragraph look like a prompt bug. So + a draw that failed in transport is redrawn once, and the retry count + travels back with the results to be reported. It is not asserted on: the + endpoint's error rate is #325's problem, not this eval's. + + A draw that reached a wrong decision is *not* redrawn. `_is_transport` + draws that line, and it is drawn narrowly on purpose: a blind + ``except Exception`` here would retry a disallowed decision, which is the + one outcome the break-it families are trying to observe. + """ + + from fateforger.llm.factory import build_intent_interpreter_client + from fateforger.slack_bot.timeboxing_intents import TimeboxingIntentInterpreter + + # The client production builds for this interpreter: + # `core.runtime._build_timeboxing_intent_interpreter` calls exactly this + # function, which is the `intent_interpreter` row -- the row this + # interpreter moved to when #336 landed, off the `timeboxing_agent` host + # agent's client. Naming a model here instead would measure a model nothing + # runs on -- the `.env` pins decide, and this eval reports whichever they + # name. + interpreter = TimeboxingIntentInterpreter(build_intent_interpreter_client()) + retries = 0 + + async def one(): + nonlocal retries + try: + return await interpreter.interpret(text, snapshot) + except Exception as exc: + if not _is_transport(exc): + # A decision was reached and it was the wrong one. That is the + # measurement, not an error to paper over. + raise + retries += 1 + # Exactly once. A second failure is the run's answer, and it + # reaches `_count` as the miss it is rather than being retried + # until the endpoint happens to agree. + return await interpreter.interpret(text, snapshot) + + results = await asyncio.gather( + *(one() for _ in range(SAMPLES)), return_exceptions=True + ) + return results, retries + + +def _outcome(result: object) -> str: + """One word for what a draw produced. + + The decision it reached, or -- when it reached none -- the failure that + stopped it. `SurfaceIntentError` is the wrapper the seam raises over + anything the transport did, so the cause is the part worth naming: a draw + that never got an answer out of the endpoint is not the same finding as a + draw that answered the wrong thing, and a rate that prints them as one + number reads a broken call as a misjudged one. + """ + if isinstance(result, BaseException): + cause = result.__cause__ or result + return type(cause).__name__ + return type(result).__name__ + + +def _count(results: list, kind: type, case: str = "", retries: int = 0) -> int: + """How many of the draws landed on `kind` -- and say it out loud. + + The rate is the assertion, so the rate is what a run has to report, the + cases that cleared the bar by one included: a 7/8 and an 8/8 are the same + green and very different evidence. The breakdown and the retry count ride + along so the misses can be read and the endpoint's flakiness stays visible + without the rate absorbing it. All of it needs `-s`. + """ + count = sum( + 1 for r in results if not isinstance(r, BaseException) and isinstance(r, kind) + ) + breakdown = dict(Counter(_outcome(r) for r in results)) + print( + f"[eval] {kind.__name__} {count}/{SAMPLES} <- {case!r} " + f":: {breakdown} retries={retries}" + ) + return count + + +# Not one of these texts appears inside `QUESTION_PARAGRAPH`. A case whose +# exact words are quoted in the prompt measures recall of the prompt rather +# than the judgement the prompt is meant to produce, so every text the +# paragraph quotes -- "is it planned?", "did you add the gym?", "what did we +# settle on for lunch?", "when is deep work?", "plan tomorrow", "start", "ok +# let's go" -- is reworded here to the same intent in different words. + +#: Named once because the fresh-session break-it family strips the paragraph +#: from this exact text, and the two halves have to be provably the same one. +CALENDAR_QUESTION = "what's on my calendar tomorrow?" + +QUESTIONS_FRESH = [ + "has it been scheduled?", + "did you put the gym in?", + CALENDAR_QUESTION, + "is there a planning session today?", +] +STARTS = [ + "plan my day tomorrow", + "let's timebox saturday", + "kick it off", + "right, let's begin", +] +CANCELS = ["cancel this", "never mind, not today"] +QUESTIONS_COMMITTED = [ + "what did we decide about lunch?", + "when's the deep-work block?", +] +# A reply that asks *and* supplies a fact is the fact -- the clause the plain +# interrogatives never exercise, and the one the paragraph actually carries. +# These two are the positive half of the break-it check below; the same texts +# appear there with the paragraph stripped. +#: The parametrisation id the break-it xfail below points at. The xfail covers +#: one case of that family and not the other, so it has to name the case -- +#: and it names it by an id minted here, never by reading the words. An +#: identity check against a module constant (`text is LUNCH_AND_SLEEP`) did the +#: same job but fails open: inline the literal into the list and the comparison +#: quietly stops matching, taking the xfail with it and turning a known cap +#: artefact into a red run nobody asked for. A missing id fails loudly instead +#: -- `callspec.id` is what pytest collected, and it is in the node name the +#: bench files the case under. +CAP_BITES = "cap_bites" +LUNCH_AND_SLEEP = "did you move lunch? I sleep 00:30-08:30" +MIXED_COMMITTED = [ + pytest.param(LUNCH_AND_SLEEP, id=CAP_BITES), + pytest.param("is deep work still at 9? also I get up at 07:00", id="flips_clean"), +] +FACTS_COMMITTED = ["I sleep 00:30–08:30", *MIXED_COMMITTED] +REVISIONS_COMMITTED = ["move the work two hours later"] +#: The fresh-session half of the break-it check. One text, and it is the same +#: object `QUESTIONS_FRESH` asserts the positive on. +BREAK_IT_FRESH = [CALENDAR_QUESTION] + + +@pytest.mark.parametrize("text", QUESTIONS_FRESH) +async def test_a_question_before_a_day_is_asked(text): + from fateforger.agents.timeboxing.session_contracts import AskQuestion + + results, retries = await _intents(text, _fresh()) + assert _count(results, AskQuestion, text, retries) >= THRESHOLD, _report(results, retries) + + +@pytest.mark.parametrize("text", STARTS) +async def test_a_start_before_a_day_starts(text): + from fateforger.agents.timeboxing.session_contracts import StartSession + + results, retries = await _intents(text, _fresh()) + assert _count(results, StartSession, text, retries) >= THRESHOLD, _report(results, retries) + + +@pytest.mark.parametrize("text", CANCELS) +async def test_a_cancel_before_a_day_cancels(text): + from fateforger.agents.timeboxing.session_contracts import CancelSession + + results, retries = await _intents(text, _fresh()) + assert _count(results, CancelSession, text, retries) >= THRESHOLD, _report(results, retries) + + +@pytest.mark.parametrize("text", QUESTIONS_COMMITTED) +async def test_a_question_after_commit_is_asked_not_revised(text): + from fateforger.agents.timeboxing.session_contracts import AskQuestion + + results, retries = await _intents(text, _committed()) + assert _count(results, AskQuestion, text, retries) >= THRESHOLD, _report(results, retries) + + +@pytest.mark.parametrize("text", FACTS_COMMITTED) +async def test_a_fact_after_commit_is_still_a_fact(text): + from fateforger.agents.timeboxing.session_contracts import ProvidePlanningFacts + + results, retries = await _intents(text, _committed()) + assert _count(results, ProvidePlanningFacts, text, retries) >= THRESHOLD, _report(results, retries) + + +@pytest.mark.parametrize("text", REVISIONS_COMMITTED) +async def test_a_revision_after_commit_is_still_a_revision(text): + from fateforger.agents.timeboxing.session_contracts import ReviseArtifact + + results, retries = await _intents(text, _committed()) + assert _count(results, ReviseArtifact, text, retries) >= THRESHOLD, _report(results, retries) + + +def _interpreter_model() -> str: + """The model the `intent_interpreter` row resolves to, at test time. + + Resolved on call and never at import: a module-level production import + would load settings before the key-less skip above could decide anything. + """ + + from fateforger.llm.factory import INTENT_INTERPRETER, _model_for_agent + + return _model_for_agent(INTENT_INTERPRETER) + + +def _expected_unbroken_here() -> bool: + """Whether this is a plain run on the pin where the paragraph does nothing. + + The mirror of the planning card's helper of the same name: there the + stripped clause is load-bearing on flash and inert on pro, here it is the + other way round. Both sides of the comparison are model ids this project + pinned, not user text. + """ + + if os.environ.get("INTERPRETER_TIER_CONFIG"): + # The bench reads the raw outcome and buckets a failure as `unbroken` + # itself; an xfail here would move the case out of that column. + return False + from fateforger.core.config import settings + + return _interpreter_model() == settings.openrouter_default_model_flash + + +def _cap_eats_the_sample_here(request) -> bool: + """Whether the row's 1024-token cap leaves this case too few decisions. + + One case, one pin. On the pro pin at `low` the stripped prompt runs long + enough that two to five of the eight draws for the `cap_bites` + parametrisation are truncated into `LengthFinishReasonError` -- retries + included -- so the flip is read off three to six decisions and landed twice + in four runs (2026-09-12). That is the cap eating the sample, not the + paragraph proving inert, and the other mixed text flips cleanly on the same + pin in the same runs, so this narrows to the one parametrisation. + + It narrows by the parametrisation id this file minted, read off the node + pytest collected. Both sides of every comparison here are identifiers -- + an id and a model id -- and the pin is resolved at test time, never at + import. + """ + + if os.environ.get("INTERPRETER_TIER_CONFIG"): + return False + from fateforger.core.config import settings + + return ( + request.node.callspec.id == CAP_BITES + and _interpreter_model() == settings.openrouter_default_model_pro + ) + + +@pytest.mark.parametrize("text", BREAK_IT_FRESH) +async def test_break_it_without_the_question_paragraph_a_question_starts_a_session( + text, monkeypatch, request +): + """Strip the paragraph and a question about the day *starts* the day. + + This is the regression the branch is named for, reproduced on demand. + "what's on my calendar tomorrow?" names a day, and with nothing in the + prompt to say that asking about a day is not asking for one, the model + reads it as `start`: 6/8, 6/8 and 7/8 to `StartSession` across three + stripped runs on 2026-09-05, against `AskQuestion` 8/8 with the paragraph + and `StartSession` not once in any unstripped run. In production that is a + planning session opened by somebody who only wanted to know what was on. + + Asserted as a flip rather than a bar. `StartSession >= THRESHOLD` would + have failed two of those three runs at 6/8 while the paragraph was plainly + doing its job; and the mirror-image bar, `AskQuestion < THRESHOLD` alone, + is cleared by two lost calls with the paragraph doing nothing. Only the + two together say the decision moved: a transport failure subtracts from + both counts and can never manufacture `started > asked`. + + It flips on the pro pin the row resolves to -- 0 asked against 8 and 7 + started on two runs of 2026-09-12 -- and not on flash at `minimal`, where + the same stripped prompt answers `question` 7/8 and `start` once. So on + flash it is an expected failure off the bench, and strict nowhere: these + are rates, and a draw can still land on either side. + """ + if _expected_unbroken_here(): + request.applymarker( + pytest.mark.xfail( + reason=( + "on the flash pin the stripped paragraph is not load-bearing: " + "asked 7/8 and started 1/8 with it gone (2026-09-12, n=8)" + ), + strict=False, + ) + ) + + import fateforger.slack_bot.timeboxing_intents as intents + from fateforger.agents.timeboxing.session_contracts import ( + AskQuestion, + StartSession, + ) + + monkeypatch.setattr( + intents, "_TIMEBOX_PROMPT_FRAGMENT", intents._TIMEBOX_PROMPT_FRAGMENT_BASE + ) + results, retries = await _intents(text, _fresh()) + asked = _count(results, AskQuestion, text, retries) + started = _count(results, StartSession, text, retries) + assert asked < THRESHOLD, _report(results, retries) + assert started > asked, _report(results, retries) + + +@pytest.mark.parametrize("text", MIXED_COMMITTED) +async def test_break_it_without_the_question_paragraph_the_fact_is_lost_to_the_question( + text, monkeypatch, request +): + """Strip the paragraph and a question carrying a fact stops being the fact. + + The break-it check used to strip the paragraph and expect plain + interrogatives -- "is it planned?", "what did we settle on for lunch?" -- + to stop reading as questions. Measured on the pro pin 2026-09-05, they do + not: they answer `question` at 7/8 and 8/8 with the paragraph gone, because + `question` is already in `allowed_decisions` and `GENERIC_PREAMBLE` says to + choose from that list. The label alone carries a pure interrogative, so + that assertion was measuring the label, not the paragraph. + + What the paragraph carries is the harder half: *"A reply that asks and also + supplies a fact is that fact: the fact changes the day and the question + does not."* Strip it and the fact is dropped -- the user gets an answer to + their question while the sleep boundary they just stated goes nowhere, + which is the silent-wrong-answer shape the whole ban exists to stop. The + same two texts assert the positive above, in + `test_a_fact_after_commit_is_still_a_fact`. + + The flip, not the absence, for the reason given on the fresh-session check: + `ProvidePlanningFacts < THRESHOLD` on its own is satisfied by two lost + draws. `asked > kept` is not. + + Two pins, two reasons for an expected failure, and they are different + findings. On flash at `minimal` both texts keep the fact 8/8 with the + paragraph stripped -- the same 8/8 they score with it -- so the paragraph + is inert there. On pro at `low` only `LUNCH_AND_SLEEP` is unreadable, and + not because the judgement held: the row's 1024-token cap truncates two to + five of its eight draws, so the flip is read off three to six decisions + and landed twice across four runs. Neither xfail is strict. + """ + if _expected_unbroken_here(): + request.applymarker( + pytest.mark.xfail( + reason=( + "on the flash pin the stripped paragraph is not load-bearing: the " + "mixed texts keep the fact 7-8/8 with it gone, across two runs of " + "each (2026-09-12, n=8)" + ), + strict=False, + ) + ) + elif _cap_eats_the_sample_here(request): + request.applymarker( + pytest.mark.xfail( + reason=( + "on the pro pin the stripped prompt runs past the row's 1024-token " + "cap: 2-5 of 8 draws die on LengthFinishReasonError, and the four " + "runs of 2026-09-12 went kept 2 / asked 1, kept 2 / asked 3, kept 3 " + "/ asked 2, kept 1 / asked 5 -- too few decisions to read the flip off" + ), + strict=False, + ) + ) + + import fateforger.slack_bot.timeboxing_intents as intents + from fateforger.agents.timeboxing.session_contracts import ( + AskQuestion, + ProvidePlanningFacts, + ) + + monkeypatch.setattr( + intents, "_TIMEBOX_PROMPT_FRAGMENT", intents._TIMEBOX_PROMPT_FRAGMENT_BASE + ) + results, retries = await _intents(text, _committed()) + kept = _count(results, ProvidePlanningFacts, text, retries) + asked = _count(results, AskQuestion, text, retries) + assert kept < THRESHOLD, _report(results, retries) + assert asked > kept, _report(results, retries) diff --git a/tests/integration/test_harness_timeboxing_session_route.py b/tests/integration/test_harness_timeboxing_session_route.py index fba82d80..052e7a84 100644 --- a/tests/integration/test_harness_timeboxing_session_route.py +++ b/tests/integration/test_harness_timeboxing_session_route.py @@ -38,6 +38,7 @@ PlanningFact, PlanningResult, ProvidePlanningFacts, + StartSession, UserBlockerDraft, coverage_fact_id, ) @@ -287,10 +288,14 @@ async def test_timebox_start_renders_the_date_card_and_starts_no_planner( """ planner = ExplodingPlanner() + runtime = Runtime(repository=repository, planner=planner) + # The opening turn is judged since #318, so even a session that only + # renders the date card needs an interpreter to answer the start. + runtime.timeboxing_intent_interpreter = ScriptedInterpreter([StartSession()]) client = Client() await handlers.route_slack_event( - runtime=Runtime(repository=repository, planner=planner), + runtime=runtime, focus=_focus(), default_agent="timeboxing_agent", event={"channel": "C1", "user": "U1", "text": "plan my day", "ts": "111"}, @@ -410,7 +415,12 @@ async def test_confirming_the_card_locks_saturday_as_a_weekend_the_host_derived( planner = RecordedPlanner() runtime = Runtime(repository=repository, planner=planner) runtime.timeboxing_intent_interpreter = ScriptedInterpreter( - [ProvidePlanningFacts(facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")])] + [ + StartSession(), + ProvidePlanningFacts( + facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")] + ), + ] ) client = Client() @@ -469,6 +479,7 @@ async def test_a_fresh_repository_rehydrates_the_session_without_the_transcript( runtime = Runtime(repository=repository, planner=planner) runtime.timeboxing_intent_interpreter = ScriptedInterpreter( [ + StartSession(), ProvidePlanningFacts( facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")] ), @@ -638,7 +649,12 @@ def _forbidden_apply(*_args: Any, **_kwargs: Any) -> int: planner = RecordedPlanner() runtime = Runtime(repository=repository, planner=planner) runtime.timeboxing_intent_interpreter = ScriptedInterpreter( - [ProvidePlanningFacts(facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")])] + [ + StartSession(), + ProvidePlanningFacts( + facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")] + ), + ] ) client = Client() @@ -706,7 +722,12 @@ async def count_suspended(self, planned_day: str, day_type: str | None) -> int: planner = RecordedPlanner([_skeleton_result(), _candidate_result()]) runtime = Runtime(repository=repository, planner=planner) runtime.timeboxing_intent_interpreter = ScriptedInterpreter( - [ProvidePlanningFacts(facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")])] + [ + StartSession(), + ProvidePlanningFacts( + facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")] + ), + ] ) runtime.timeboxing_calendar_id = "cal" runtime.timeboxing_constraint_store = _ConstraintStore() @@ -772,7 +793,12 @@ async def produce( runtime = Runtime(repository=repository, planner=_ExplodingPlanner()) runtime.timeboxing_intent_interpreter = ScriptedInterpreter( - [ProvidePlanningFacts(facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")])] + [ + StartSession(), + ProvidePlanningFacts( + facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")] + ), + ] ) client = Client() @@ -817,6 +843,9 @@ async def test_a_stale_card_click_changes_nothing_and_says_so_safely( planner = RecordedPlanner() runtime = Runtime(repository=repository, planner=planner) + # The opening turn is judged since #318, so even a session that only + # renders the date card needs an interpreter to answer the start. + runtime.timeboxing_intent_interpreter = ScriptedInterpreter([StartSession()]) client = Client() await handlers.route_slack_event( @@ -888,6 +917,9 @@ async def test_pressing_vacation_on_a_saturday_locks_vacation_not_the_weekend( planner = RecordedPlanner() runtime = Runtime(repository=repository, planner=planner) + # The opening turn is judged since #318, so even a session that only + # renders the date card needs an interpreter to answer the start. + runtime.timeboxing_intent_interpreter = ScriptedInterpreter([StartSession()]) client = Client() await handlers.route_slack_event( @@ -938,6 +970,9 @@ async def test_picking_another_day_keeps_the_day_type_row_on_the_card( planner = RecordedPlanner() runtime = Runtime(repository=repository, planner=planner) + # The opening turn is judged since #318, so even a session that only + # renders the date card needs an interpreter to answer the start. + runtime.timeboxing_intent_interpreter = ScriptedInterpreter([StartSession()]) client = Client() await handlers.route_slack_event( @@ -1064,7 +1099,12 @@ async def test_a_failed_turn_offers_retry_and_cancel_bound_to_the_session( planner = FlakyPlanner() runtime = Runtime(repository=repository, planner=planner) runtime.timeboxing_intent_interpreter = ScriptedInterpreter( - [ProvidePlanningFacts(facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")])] + [ + StartSession(), + ProvidePlanningFacts( + facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")] + ), + ] ) client = Client() @@ -1127,7 +1167,12 @@ async def spy_run(**kwargs: Any) -> Any: pressed_planner = FlakyPlanner() pressed_runtime = Runtime(repository=repository, planner=pressed_planner) pressed_runtime.timeboxing_intent_interpreter = ScriptedInterpreter( - [ProvidePlanningFacts(facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")])] + [ + StartSession(), + ProvidePlanningFacts( + facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")] + ), + ] ) pressed_client = Client() pressed_key = await _drive_to_a_failed_turn( @@ -1152,6 +1197,7 @@ async def spy_run(**kwargs: Any) -> Any: typed_runtime = Runtime(repository=repository, planner=typed_planner) typed_runtime.timeboxing_intent_interpreter = ScriptedInterpreter( [ + StartSession(), ProvidePlanningFacts( facts=[_fact("b1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("b1-frame")] ), @@ -1209,6 +1255,9 @@ async def test_an_open_question_is_asked_without_a_button_row( planner = RecordedPlanner() runtime = Runtime(repository=repository, planner=planner) + # The opening turn is judged since #318, so even a session that only + # renders the date card needs an interpreter to answer the start. + runtime.timeboxing_intent_interpreter = ScriptedInterpreter([StartSession()]) client = Client() session_key = await _start_and_confirm_saturday( @@ -1256,6 +1305,7 @@ async def test_approving_a_superseded_skeleton_plans_nothing( runtime = Runtime(repository=repository, planner=planner) runtime.timeboxing_intent_interpreter = ScriptedInterpreter( [ + StartSession(), ProvidePlanningFacts( facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")] ), @@ -1442,7 +1492,12 @@ async def count_suspended(self, planned_day: str, day_type: str | None) -> int: planner = RecordedPlanner([_skeleton_result(), _candidate_result()]) runtime = Runtime(repository=repository, planner=planner) runtime.timeboxing_intent_interpreter = ScriptedInterpreter( - [ProvidePlanningFacts(facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")])] + [ + StartSession(), + ProvidePlanningFacts( + facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")] + ), + ] ) runtime.timeboxing_calendar_id = "cal" runtime.timeboxing_constraint_store = _ConstraintStore() @@ -1579,7 +1634,12 @@ async def count_suspended(self, planned_day: str, day_type: str | None) -> int: planner = RecordedPlanner([_skeleton_result(), _candidate_result()]) runtime = Runtime(repository=repository, planner=planner) runtime.timeboxing_intent_interpreter = ScriptedInterpreter( - [ProvidePlanningFacts(facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")])] + [ + StartSession(), + ProvidePlanningFacts( + facts=[_fact("a1", FactKind.REQUESTED_ACTIVITY, "gym"), _frame("a1-frame")] + ), + ] ) runtime.timeboxing_calendar_id = "cal" runtime.timeboxing_constraint_store = _ConstraintStore() @@ -1681,7 +1741,10 @@ async def test_a_typed_vacation_gets_past_the_date_card_without_a_press( planner = RecordedPlanner() runtime = Runtime(repository=repository, planner=planner) model = ScriptedModel( - {"decision": "confirm_planning_day", "day_type": "vacation", "facts": []} + # The opening turn is judged like any other since #318: typed words on + # a fresh session are a start, not an assumption. + {"decision": "start", "facts": []}, + {"decision": "confirm_planning_day", "day_type": "vacation", "facts": []}, ) runtime.timeboxing_intent_interpreter = TimeboxingIntentInterpreter(model) client = Client() @@ -1695,8 +1758,9 @@ async def test_a_typed_vacation_gets_past_the_date_card_without_a_press( say=None, client=client, ) - # The card is on screen and nobody pressed it. - assert not model.prompts + # The card is on screen and nobody pressed it: the one interpretation so + # far is the start the opening words were judged to be (#318). + assert len(model.prompts) == 1 await handlers.route_slack_event( runtime=runtime, @@ -1723,7 +1787,8 @@ async def test_a_typed_vacation_gets_past_the_date_card_without_a_press( # basis that disagrees with the weekday, so a chat override that skipped it # would raise rather than quietly claim the calendar said so. assert snapshot.planning_day.classification_basis == "user_override" - assert len(model.prompts) == 1 + # The start, and then the sentence that got past the card. + assert len(model.prompts) == 2 def _closed_choice_requirements() -> type: @@ -1858,7 +1923,10 @@ async def test_a_closed_question_arrives_as_buttons_carrying_what_they_answer( planner = ShapePlanner() runtime = Runtime(repository=repository, planner=planner) runtime.timeboxing_intent_interpreter = TimeboxingIntentInterpreter( - ScriptedModel({"decision": "confirm_planning_day"}) + ScriptedModel( + {"decision": "start"}, + {"decision": "confirm_planning_day"}, + ) ) client = Client() @@ -1907,7 +1975,10 @@ async def test_an_option_question_keeps_its_buttons_when_a_press_is_refused( planner = ShapePlanner() runtime = Runtime(repository=repository, planner=planner) runtime.timeboxing_intent_interpreter = TimeboxingIntentInterpreter( - ScriptedModel({"decision": "confirm_planning_day"}) + ScriptedModel( + {"decision": "start"}, + {"decision": "confirm_planning_day"}, + ) ) client = Client() @@ -2068,6 +2139,7 @@ async def spy_run(**kwargs: Any) -> Any: typed_runtime, typed_planner, typed_model = _shape_runtime( repository, responses=[ + {"decision": "start"}, {"decision": "confirm_planning_day", "day_type": "vacation"}, facts_reply, {"decision": "advance", "facts": []}, @@ -2100,7 +2172,8 @@ async def spy_run(**kwargs: Any) -> Any: text=text, ts=ts, ) - assert len(typed_model.prompts) == step + 1 + # One interpretation per reply, after the opening start (#318). + assert len(typed_model.prompts) == step + 2 typed_snapshot = await repository.load_or_create("C1:p1", owner_user_id="U1") assert typed_snapshot.planning_day is not None @@ -2114,7 +2187,11 @@ async def spy_run(**kwargs: Any) -> Any: # -- pressed, wherever a control exists ------------------------------- pressed_runtime, pressed_planner, pressed_model = _shape_runtime( repository, - responses=[facts_reply, {"decision": "advance", "facts": []}], + responses=[ + {"decision": "start", "facts": []}, + facts_reply, + {"decision": "advance", "facts": []}, + ], ) pressed_client = Client() @@ -2209,15 +2286,16 @@ async def spy_run(**kwargs: Any) -> Any: # The typed choice was made from the offer rather than from a memory of it: # the turn that answered had the ids, the labels and the effects in front of # it, which is the whole difference between choosing and guessing. - choice_prompt = typed_model.prompts[3] + choice_prompt = typed_model.prompts[4] for option in _shape_options(): assert option.option_id in choice_prompt assert option.label in choice_prompt assert option.effect in choice_prompt - # Three of the pressed run's five transitions needed no model at all. That - # is the case for buttons where the answer set is closed, and the case for - # keeping both doors: the typed run needed five. - assert len(pressed_model.prompts) == 2 - assert len(typed_model.prompts) == 5 + # Three of the pressed run's transitions needed no model at all. That is + # the case for buttons where the answer set is closed, and the case for + # keeping both doors: the typed run judged every turn it had, the opening + # one included (#318). + assert len(pressed_model.prompts) == 3 + assert len(typed_model.prompts) == 6 for planner in (typed_planner, pressed_planner): assert planner.briefs[-1].target_artifact is ArtifactKind.VALIDATED_CANDIDATE diff --git a/tests/unit/test_adaptive_timeboxing.py b/tests/unit/test_adaptive_timeboxing.py index 6ea161ec..96252379 100644 --- a/tests/unit/test_adaptive_timeboxing.py +++ b/tests/unit/test_adaptive_timeboxing.py @@ -30,9 +30,13 @@ ArtifactApproval, ArtifactDraft, ArtifactKind, + Asked, + AskQuestion, AwaitingApproval, AwaitingUser, BlockerOption, + Cancelled, + CancelSession, ChooseBlockerOption, Committed, ConfirmPlanningDay, @@ -45,6 +49,7 @@ PlanningResult, PlanningSessionSnapshot, ProvidePlanningFacts, + StartSession, TurnFailed, UserBlockerDraft, ) @@ -2489,3 +2494,192 @@ async def test_a_required_kind_already_on_the_day_satisfies_it_through_the_rows( request, progress=RecordingProgressSink() ) assert isinstance(outcome, AwaitingApproval), outcome + + +class _ForbiddenDependency: + def __getattr__(self, name: str): + raise AssertionError(f"kernel dependency must not be used: {name}") + + +@pytest.mark.asyncio +async def test_a_question_is_asked_and_changes_nothing() -> None: + """Asked is not started and not revised: the snapshot the next load sees + is the one this turn loaded, field for field.""" + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=3, owner_user_id="U1", + planning_day=_locked_day(), status="open", + ) + repo = InMemoryPlanningSessionRepository([snapshot]) + kernel = AdaptiveTimeboxing( + repository=repo, requirements=TimeboxRequirements(), + planner=_ForbiddenDependency(), context=_ForbiddenDependency(), + commit=_ForbiddenDependency(), + ) + before = (await repo.load_or_create("C1:1.0", owner_user_id="U1")).model_dump() + + outcome = await kernel.turn( + TurnRequest( + session_key="C1:1.0", interaction_id="q1", actor_user_id="U1", + expected_revision=3, intent=AskQuestion(question="Is it planned?"), + ), + progress=RecordingProgressSink(), + ) + + assert isinstance(outcome, Asked) + assert outcome.question == "Is it planned?" + after = (await repo.load_or_create("C1:1.0", owner_user_id="U1")).model_dump() + assert after == before + assert after["revision"] == 3 + + +@pytest.mark.asyncio +async def test_a_question_to_a_committed_session_is_still_just_asked() -> None: + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=9, owner_user_id="U1", + planning_day=_locked_day(), status="committed", + ) + repo = InMemoryPlanningSessionRepository([snapshot]) + kernel = AdaptiveTimeboxing( + repository=repo, requirements=TimeboxRequirements(), + planner=_ForbiddenDependency(), context=_ForbiddenDependency(), + commit=_ForbiddenDependency(), + ) + outcome = await kernel.turn( + TurnRequest( + session_key="C1:1.0", interaction_id="q2", actor_user_id="U1", + expected_revision=9, intent=AskQuestion(question="when is deep work?"), + ), + progress=RecordingProgressSink(), + ) + assert isinstance(outcome, Asked) + assert (await repo.load_or_create("C1:1.0", owner_user_id="U1")).revision == 9 + + +# --- a session row is created only by an intent that starts one (#318) ------ +# +# `cancel` at revision 0 used to brick a DM in one message: the first thing a +# user typed ("never mind, not today") wrote `cancelled` on the thread-blind +# `{channel}:dm` key, and every later turn hit a cancelled session. Asking used +# to leave a revision-0 `open` row behind, which the host reads as a session +# under way. Neither intent may reach the store now. + + +def _fresh_kernel( + repo: InMemoryPlanningSessionRepository, + *, + context: PlanningContextPort | None = None, +) -> AdaptiveTimeboxing: + return AdaptiveTimeboxing( + repository=repo, + requirements=TimeboxRequirements(), + planner=_ForbiddenDependency(), + context=context or _ForbiddenDependency(), + commit=_ForbiddenDependency(), + ) + + +def _fresh_request(intent: object, *, interaction_id: str) -> TurnRequest: + return TurnRequest( + session_key="D1:dm", + interaction_id=interaction_id, + actor_user_id="U1", + expected_revision=0, + intent=intent, + ) + + +@pytest.mark.asyncio +async def test_a_question_to_a_session_that_does_not_exist_creates_no_row() -> None: + repo = InMemoryPlanningSessionRepository() + + outcome = await _fresh_kernel(repo).turn( + _fresh_request(AskQuestion(question="is tomorrow planned?"), interaction_id="q1"), + progress=RecordingProgressSink(), + ) + + assert isinstance(outcome, Asked) + assert outcome.question == "is tomorrow planned?" + assert await repo.load("D1:dm") is None + + +@pytest.mark.asyncio +async def test_cancelling_a_session_that_does_not_exist_is_refused_and_writes_nothing() -> None: + repo = InMemoryPlanningSessionRepository() + + outcome = await _fresh_kernel(repo).turn( + _fresh_request(CancelSession(), interaction_id="c1"), + progress=RecordingProgressSink(), + ) + + assert isinstance(outcome, TurnFailed) + assert outcome.code == "nothing_to_cancel" + assert await repo.load("D1:dm") is None + + +@pytest.mark.asyncio +async def test_cancelling_an_empty_revision_zero_row_is_refused_and_leaves_it_open() -> None: + """A row an earlier code path left behind is still nothing to cancel.""" + + repo = InMemoryPlanningSessionRepository( + [PlanningSessionSnapshot.new(session_key="D1:dm", owner_user_id="U1")] + ) + + outcome = await _fresh_kernel(repo).turn( + _fresh_request(CancelSession(), interaction_id="c2"), + progress=RecordingProgressSink(), + ) + + assert isinstance(outcome, TurnFailed) + assert outcome.code == "nothing_to_cancel" + stored = await repo.load("D1:dm") + assert stored is not None + assert stored.status == "open" + assert stored.revision == 0 + + +@pytest.mark.asyncio +async def test_cancelling_a_session_that_locked_a_day_still_cancels() -> None: + """The refusal is about having nothing to cancel, not about cancelling.""" + + repo = InMemoryPlanningSessionRepository( + [ + PlanningSessionSnapshot( + session_key="C1:1.0", + revision=3, + owner_user_id="U1", + planning_day=_locked_day(), + status="open", + ) + ] + ) + + outcome = await _fresh_kernel(repo).turn( + TurnRequest( + session_key="C1:1.0", + interaction_id="c3", + actor_user_id="U1", + expected_revision=3, + intent=CancelSession(), + ), + progress=RecordingProgressSink(), + ) + + assert isinstance(outcome, Cancelled) + stored = await repo.load("C1:1.0") + assert stored is not None + assert stored.status == "cancelled" + + +@pytest.mark.asyncio +async def test_starting_a_session_is_the_intent_that_creates_the_row() -> None: + repo = InMemoryPlanningSessionRepository() + context = RecordedContextPort() + + await _fresh_kernel(repo, context=context).turn( + _fresh_request(StartSession(), interaction_id="s1"), + progress=RecordingProgressSink(), + ) + + stored = await repo.load("D1:dm") + assert stored is not None + assert stored.owner_user_id == "U1" diff --git a/tests/unit/test_adaptive_turn_marks_timeboxing_active.py b/tests/unit/test_adaptive_turn_marks_timeboxing_active.py index 4af41da3..16247bc7 100644 --- a/tests/unit/test_adaptive_turn_marks_timeboxing_active.py +++ b/tests/unit/test_adaptive_turn_marks_timeboxing_active.py @@ -42,9 +42,9 @@ async def turn(self, request, progress): return TurnFailed(code="x", message="x") class Repo: - async def load_or_create(self, key, owner_user_id): + async def load(self, key): return PlanningSessionSnapshot( - session_key=key, revision=1, owner_user_id=owner_user_id + session_key=key, revision=1, owner_user_id="U1" ) class Runtime: @@ -114,9 +114,9 @@ async def turn(self, request, progress): return TurnFailed(code="x", message="x") class Repo: - async def load_or_create(self, key, owner_user_id): + async def load(self, key): return PlanningSessionSnapshot( - session_key=key, revision=1, owner_user_id=owner_user_id, + session_key=key, revision=1, owner_user_id="U1", status="committed", ) diff --git a/tests/unit/test_asked_is_answered_in_the_turn.py b/tests/unit/test_asked_is_answered_in_the_turn.py new file mode 100644 index 00000000..7d9ea752 --- /dev/null +++ b/tests/unit/test_asked_is_answered_in_the_turn.py @@ -0,0 +1,282 @@ +"""An `Asked` outcome is answered by planner_agent with the session described, +in the turn's own reply. No stage card is drawn, no session state moves, and +an answerer that fails is reported, never swallowed and never retried into a +session start. + +Asked is not started on the host either: a question marks no activity, cancels +no Admonisher ladder, and persists no session row. The Admonisher reads the +session store to decide whether to nudge, so a row written by a question is a +planning reminder silenced for an hour by the user asking whether they have +planned anything. + +Fixture shape copied from `test_turn_cancels_ladder.py`: Kernel/Repo/Runtime +fakes plus the four monkeypatches that let `_run_adaptive_timebox_turn` run +end to end without Slack, a planner or a store. +""" + +from __future__ import annotations + +import logging +from types import SimpleNamespace + +import pytest +from autogen_agentchat.messages import TextMessage + +import fateforger.slack_bot.handlers as handlers +from fateforger.agents.timeboxing.session_contracts import ( + Advance, + ArtifactKind, + Asked, + AskQuestion, + PlanningArtifact, + PlanningSessionSnapshot, +) +from fateforger.slack_bot.timeboxing_cards import timebox_failure_message + + +class _StubCard: + def __init__(self, *a, **k): + pass + + async def close(self): + pass + + +class _Repo: + """Loads without creating; `load_or_create` is the only thing that writes.""" + + def __init__(self, snapshot: PlanningSessionSnapshot | None = None) -> None: + self.rows: dict[str, PlanningSessionSnapshot] = ( + {} if snapshot is None else {snapshot.session_key: snapshot} + ) + + async def load(self, key): + return self.rows.get(key) + + async def load_or_create(self, key, owner_user_id): + row = self.rows.get(key) + if row is None: + row = PlanningSessionSnapshot.new( + session_key=key, owner_user_id=owner_user_id + ) + self.rows[key] = row + return row + + +class _Haunting: + def __init__(self) -> None: + self.activity: list[str] = [] + self.cancelled: list[str] = [] + + async def record_user_activity(self, *, topic_id, task_id, user_id): + self.activity.append(topic_id) + + async def cancel_followups(self, *, topic_id): + self.cancelled.append(topic_id) + + +class _ActivityRecorder: + def __init__(self) -> None: + self.active: list[str] = [] + self.inactive: list[str] = [] + + def mark_active(self, *, user_id, channel_id, thread_ts): + self.active.append(user_id) + + def mark_inactive(self, *, user_id): + self.inactive.append(user_id) + + +def _committed_snapshot(session_key: str = "D1:dm") -> PlanningSessionSnapshot: + receipt = PlanningArtifact.create( + kind=ArtifactKind.COMMIT_RECEIPT, + revision=1, + payload={"committed": True, "tx_id": "tx_7", "candidate_digest": "d" * 64}, + dependency_revisions={}, + ) + return PlanningSessionSnapshot( + session_key=session_key, + revision=8, + owner_user_id="U1", + status="committed", + artifacts=[receipt], + ) + + +def _fixture( + monkeypatch, + *, + reply=None, + raise_=None, + existing: PlanningSessionSnapshot | None = None, + intent=None, + kernel_raises: Exception | None = None, +): + sent: list[tuple[object, object]] = [] + turn_intent = AskQuestion(question="Is it planned?") if intent is None else intent + + async def _intent(*a, **k): + return turn_intent + + class Kernel: + async def turn(self, request, progress): + if kernel_raises is not None: + raise kernel_raises + assert isinstance(request.intent, AskQuestion) + return Asked(question=request.intent.question) + + class Runtime: + timeboxing_session_store = _Repo(existing) + haunting_service = _Haunting() + + async def send_message(self, message, recipient): + sent.append((message, recipient)) + if raise_ is not None: + raise raise_ + return SimpleNamespace( + chat_message=TextMessage(content=reply, source="planner_agent") + ) + + def _no_card(*a, **k): + raise AssertionError("present_outcome must not run for Asked") + + activity = _ActivityRecorder() + monkeypatch.setattr(handlers, "_timeboxing_kernel", lambda *a, **k: Kernel()) + monkeypatch.setattr(handlers, "derive_timebox_intent", _intent) + monkeypatch.setattr(handlers, "HarnessProgressCard", _StubCard) + monkeypatch.setattr(handlers, "present_outcome", _no_card) + monkeypatch.setattr(handlers, "timeboxing_activity", activity) + runtime = Runtime() + runtime.activity = activity + return runtime, sent + + +async def _turn(runtime): + return await handlers._run_adaptive_timebox_turn( + runtime=runtime, client=object(), logger=logging.getLogger(__name__), + session_key="D1:dm", actor_user_id="U1", interaction_id="1.1", + progress_channel="D1", progress_ts="1.0", + card_channel="D1", card_thread_ts="dm", user_text="Is it planned?", + ) + + +@pytest.mark.asyncio +async def test_a_question_is_answered_by_planner_agent_with_the_session_described(monkeypatch): + runtime, sent = _fixture(monkeypatch, reply="No — nothing on the calendar today.") + message = await _turn(runtime) + assert len(sent) == 1 + msg, recipient = sent[0] + assert recipient.type == "planner_agent" + assert msg.source == "U1" + assert "Is it planned?" in msg.content + assert "timeboxing session" in msg.content # the description came along + assert message.text == "No — nothing on the calendar today." + # A blockless answer stays blockless: see + # `test_a_long_answer_reaches_the_caller_whole` for what a synthesised + # section block costs. + assert message.blocks == [] + + +@pytest.mark.asyncio +async def test_a_long_answer_reaches_the_caller_whole(monkeypatch): + """The answer travels as text, so the block cap never touches it. + + Every caller runs the returned message back through + `_compact_slack_payload`, which clips block text at + `SLACK_MAX_BLOCK_TEXT_CHARS` (1600) while plain `text` keeps + `SLACK_MAX_TEXT_CHARS` (3900) -- and Slack renders `blocks` whenever they + are present. An answer wrapped in a synthesised section block was therefore + delivered clipped at 1600 characters while the whole of it sat unused in + the fallback text. + """ + + long_reply = "Lunch is still at 12:30, and here is why. " * 60 + assert len(long_reply) > handlers.SLACK_MAX_BLOCK_TEXT_CHARS + assert len(long_reply) < handlers.SLACK_MAX_TEXT_CHARS + + runtime, _ = _fixture(monkeypatch, reply=long_reply) + message = await _turn(runtime) + + payload = handlers._compact_slack_payload(text=message.text, blocks=message.blocks) + assert "blocks" not in payload + assert payload["text"] == long_reply + + +@pytest.mark.asyncio +async def test_a_question_leaves_no_session_row_and_no_activity(monkeypatch): + """The Admonisher decides by the store and the idle timer; a question must + move neither, or asking "is it planned?" silences the reminder that would + have answered it.""" + + runtime, _ = _fixture(monkeypatch, reply="Nothing on the calendar today.") + await _turn(runtime) + + assert runtime.activity.active == [] + assert runtime.haunting_service.activity == [] + assert runtime.timeboxing_session_store.rows == {} + + +@pytest.mark.asyncio +async def test_a_question_in_a_committed_thread_ends_no_session(monkeypatch): + """The other half of the same exemption, on the way out of the turn. + + `mark_inactive` is keyed by *user*, not by session, and `cancel_followups` + drops the whole topic's ladder. A question typed into a thread whose + session is already committed reaches that branch -- `current.status` is + `committed`, so the turn concludes "the session is over" -- and tears down + the idle timer of whatever open session the same user has running + elsewhere. Asking what was decided yesterday must not end today's planning. + """ + + runtime, _ = _fixture( + monkeypatch, reply="Lunch stayed at 12:30.", existing=_committed_snapshot() + ) + await _turn(runtime) + + assert runtime.activity.inactive == [] + assert runtime.haunting_service.cancelled == [] + + +@pytest.mark.asyncio +async def test_a_turn_that_is_not_a_question_still_records_activity(monkeypatch): + """The other side of the same seam: only a question is exempt.""" + + runtime, _ = _fixture( + monkeypatch, intent=Advance(), kernel_raises=RuntimeError("kernel down") + ) + await _turn(runtime) + + assert runtime.haunting_service.activity == ["D1:dm"] + assert runtime.activity.active == ["U1"] + + +@pytest.mark.asyncio +async def test_an_answerer_that_fails_is_reported_and_never_starts_a_session(monkeypatch): + errors: list[dict] = [] + monkeypatch.setattr(handlers, "record_error", lambda **kw: errors.append(kw)) + runtime, sent = _fixture(monkeypatch, raise_=RuntimeError("planner down")) + message = await _turn(runtime) + assert len(sent) == 1 # asked once, not retried + assert errors == [{"component": "surface_intent", "error_type": "answer_failure"}] + # The snapshot the answer was described from is the one the failure is + # reported against; on a session with no row that is a fresh one. + assert message.text == timebox_failure_message( + snapshot=PlanningSessionSnapshot.new(session_key="D1:dm", owner_user_id="U1") + ).text + assert runtime.timeboxing_session_store.rows == {} + + +@pytest.mark.asyncio +async def test_an_answerer_that_fails_on_a_committed_day_says_the_day_still_stands(monkeypatch): + """The two failure sentences differ, and the code picks by the receipt. A + test that passed `snapshot=None` could not tell the branch apart from one + that passed nothing at all.""" + + monkeypatch.setattr(handlers, "record_error", lambda **kw: None) + runtime, sent = _fixture( + monkeypatch, raise_=RuntimeError("planner down"), existing=_committed_snapshot() + ) + message = await _turn(runtime) + assert len(sent) == 1 + assert message.text == timebox_failure_message(snapshot=_committed_snapshot()).text + assert message.text != timebox_failure_message(snapshot=None).text diff --git a/tests/unit/test_describe_session.py b/tests/unit/test_describe_session.py new file mode 100644 index 00000000..03923fad --- /dev/null +++ b/tests/unit/test_describe_session.py @@ -0,0 +1,110 @@ +"""`describe_session` says what the card says, in prose, for an agent that +cannot see the card. Fields, not sentences: the wording is free to move.""" + +from __future__ import annotations + +from datetime import date + +from fateforger.agents.timeboxing.session_contracts import ( + ArtifactKind, + BlockerOption, + PlanningArtifact, + PlanningDay, + PlanningSessionSnapshot, +) +from fateforger.slack_bot.stage_cards import ( + Asking, + ContextItem, + DecidedItem, + StageCard, + describe_session, + stage, +) + + +def _planning_day() -> PlanningDay: + return PlanningDay.lock_default( + value=date(2026, 9, 5), timezone="Europe/Amsterdam", lock_revision=1 + ) + + +def test_a_stage_three_card_names_its_decided_items_and_the_day() -> None: + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=5, owner_user_id="U1", + planning_day=_planning_day(), status="open", + ) + card = StageCard( + stage=stage(3), session_key="C1:1.0", expected_revision=5, + context=[ContextItem(text="Oats two hours before gym", source="memory")], + decided=[ + DecidedItem(text="Gym at 18:00", kind="fact", ref="f1"), + DecidedItem(text="Lunch at 13:00", kind="assumption", ref="a1", filed_by="planner"), + ], + body="07:00 wake · 09:00 deep work · 18:00 gym", + ) + text = describe_session(snapshot, card) + assert "2026-09-05" in text + assert "Saturday" in text + assert "3/5" in text and "Sketch" in text + assert "Gym at 18:00" in text + assert "Lunch at 13:00" in text and "assumption" in text and "planner" in text + assert "Oats two hours before gym" in text + assert "deep work" in text + + +def test_a_committed_session_names_the_receipt() -> None: + # The payload keys are the ones `PendingCandidateCommitPort.commit` writes + # (`timeboxing_host.py`): there is no calendar id and no applied count on a + # real receipt, so the identifying fields are the transaction and what it + # reached. + receipt = PlanningArtifact.create( + kind=ArtifactKind.COMMIT_RECEIPT, revision=1, + payload={"committed": True, "tx_id": "tx_42", "reason": None, + "candidate_digest": "d" * 64, + "calendar_backend": "google", "durable": True}, + dependency_revisions={}, + ) + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=9, owner_user_id="U1", + planning_day=_planning_day(), status="committed", artifacts=[receipt], + ) + text = describe_session(snapshot, card=None) + assert "committed" in text + assert "tx_42" in text + assert "d" * 64 in text + assert "google" in text + + +def test_a_fresh_session_says_so() -> None: + snapshot = PlanningSessionSnapshot(session_key="D1:dm", revision=0, owner_user_id="U1") + text = describe_session(snapshot, card=None) + assert "no planning day" in text.lower() or "not started" in text.lower() + + +def test_the_open_question_is_described_with_its_options_and_their_effects() -> None: + """A user staring at two buttons and typing "what are my choices?" gets an + answer from this text. Dropping the options leaves the answerer describing + a question whose answers it cannot see.""" + + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=5, owner_user_id="U1", + planning_day=_planning_day(), status="open", + ) + asking = Asking( + requirement_id="skeleton.dinner_anchor", + question="When is dinner?", + why_needed="the gym block has to sit around it", + options=[ + BlockerOption(option_id="o1", label="18:30", effect="gym moves to 16:45"), + BlockerOption(option_id="o2", label="20:00", effect="gym stays at 18:00"), + ], + ) + card = StageCard( + stage=stage(3), session_key="C1:1.0", expected_revision=5, asking=asking, + ) + text = describe_session(snapshot, card) + assert asking.question in text + assert asking.why_needed in text + for option in asking.options: + assert option.label in text + assert option.effect in text diff --git a/tests/unit/test_no_session_is_judged.py b/tests/unit/test_no_session_is_judged.py new file mode 100644 index 00000000..7e2f70df --- /dev/null +++ b/tests/unit/test_no_session_is_judged.py @@ -0,0 +1,148 @@ +"""Before a day is locked there *is* something to decide: start, ask, or +cancel. The interpreter decides it; nothing here reads the words.""" + +from __future__ import annotations + +import ast +import inspect +import json +from types import SimpleNamespace + +import pytest + +import fateforger.slack_bot.timeboxing_host as host_module +from fateforger.agents.timeboxing.session_contracts import ( + Advance, + AskQuestion, + CancelSession, + PlanningSessionSnapshot, + StartSession, +) +from fateforger.slack_bot.timeboxing_host import derive_timebox_intent +from fateforger.slack_bot.timeboxing_intents import ( + TimeboxingIntentInterpreter, + _display_context, +) + + +class _SchemaOutputClient: + def __init__(self, *responses): + self._responses = list(responses) + self.calls = [] + + async def create(self, messages, *, json_output): # noqa: ANN001 + self.calls.append((messages, json_output)) + return SimpleNamespace(content=json.dumps(self._responses.pop(0))) + + +def _fresh() -> PlanningSessionSnapshot: + return PlanningSessionSnapshot(session_key="D1:dm", revision=0, owner_user_id="U1") + + +def _runtime(*responses): + return SimpleNamespace( + timeboxing_intent_interpreter=TimeboxingIntentInterpreter( + _SchemaOutputClient(*responses) + ) + ) + + +def test_a_fresh_session_offers_start_question_and_cancel() -> None: + state, allowed, pending = _display_context(_fresh()) + assert state == "no_session" + assert set(allowed) == {"start", "question", "cancel"} + assert pending is None + + +@pytest.mark.asyncio +async def test_start_opens_the_session_exactly_as_before() -> None: + intent = await derive_timebox_intent( + _runtime({"decision": "start", "facts": []}), + _fresh(), + user_text="plan tomorrow", + ) + assert intent == StartSession() + + +@pytest.mark.asyncio +async def test_a_question_before_a_day_is_asked_not_started() -> None: + intent = await derive_timebox_intent( + _runtime({"decision": "question", "facts": []}), + _fresh(), + user_text="Is it planned?", + ) + assert intent == AskQuestion(question="Is it planned?") + + +@pytest.mark.asyncio +async def test_a_cancel_before_a_day_reaches_the_kernel() -> None: + intent = await derive_timebox_intent( + _runtime({"decision": "cancel", "facts": []}), + _fresh(), + user_text="never mind", + ) + assert intent == CancelSession() + + +@pytest.mark.asyncio +async def test_empty_text_on_a_fresh_session_still_opens_it() -> None: + # The opening turn arrives with no words (the auto-start, a bare command); + # that is a start, as it always was. Only typed words are judged. + runtime = _runtime() # no interpreter response: it must not be asked + intent = await derive_timebox_intent(runtime, _fresh(), user_text=" ") + assert intent == StartSession() + assert runtime.timeboxing_intent_interpreter.model_client.calls == [] + + +@pytest.mark.asyncio +async def test_empty_text_on_a_started_session_is_still_advance() -> None: + from datetime import date + + from fateforger.agents.timeboxing.session_contracts import PlanningDay + + snapshot = PlanningSessionSnapshot( + session_key="D1:dm", + revision=2, + owner_user_id="U1", + planning_day=PlanningDay.lock_default( + value=date(2026, 9, 5), timezone="Europe/Amsterdam", lock_revision=1 + ), + ) + assert await derive_timebox_intent(_runtime(), snapshot, user_text="") == Advance() + + +def test_derive_timebox_intent_has_no_unconditional_start() -> None: + """The guard for the claim this ticket deletes: no `return StartSession()` + that is not inside the judged path. Any Return whose value calls + StartSession must sit under an `if` on the text being empty. + + Counting the returns is not enough on its own -- the code this ticket + deletes had exactly one too, the unconditional one -- so the enclosing + `if` is what the guard actually asserts. + """ + tree = ast.parse(inspect.getsource(host_module)) + fn = next( + n + for n in ast.walk(tree) + if isinstance(n, ast.AsyncFunctionDef) and n.name == "derive_timebox_intent" + ) + starts = [ + n + for n in ast.walk(fn) + if isinstance(n, ast.Return) + and isinstance(n.value, ast.Call) + and getattr(n.value.func, "id", None) == "StartSession" + ] + # Exactly one, and it is the empty-text start. + assert len(starts) == 1 + guarding = [ + branch + for branch in ast.walk(fn) + if isinstance(branch, ast.If) + and any( + isinstance(node, ast.Name) and node.id == "user_text" + for node in ast.walk(branch.test) + ) + and any(node is starts[0] for node in ast.walk(branch)) + ] + assert guarding, "the one StartSession return is not under a test on user_text" diff --git a/tests/unit/test_slack_timeboxing_routing.py b/tests/unit/test_slack_timeboxing_routing.py index 3db9b8a7..9b5e03a7 100644 --- a/tests/unit/test_slack_timeboxing_routing.py +++ b/tests/unit/test_slack_timeboxing_routing.py @@ -548,6 +548,73 @@ async def test_a_planning_thread_survives_a_sticky_dm_focus_on_timeboxing(): assert "CARD CONTEXT" in runtime.calls[0][0].content +@pytest.mark.asyncio +async def test_a_planning_thread_in_a_timeboxing_channel_is_demoted_to_the_receptionist(monkeypatch): + # #310 demoted to the channel default, which is a no-op when that default + # is itself timeboxing_agent. The planning card's thread goes to the + # receptionist, whatever the channel is for. + import fateforger.slack_bot.handlers as handlers + + monkeypatch.setattr(handlers, "_agent_for_channel", lambda channel_id: "timeboxing_agent") + focus = FocusManager( + ttl_seconds=60, allowed_agents=["receptionist_agent", "timeboxing_agent"] + ) + runtime = _FakeRuntime([_FakeResult(TextMessage(content="answer", source="bot"))]) + runtime.timeboxing_session_store = _SessionStore({}) + client = _FakeClient() + planning = _PlanningReplyHandler( + ThreadReply(ThreadReplyOutcome.NO_PRESS, context="CARD CONTEXT"), owns=True + ) + + await route_slack_event( + runtime=runtime, + focus=focus, + default_agent="receptionist_agent", + event={ + "channel": "C9", + "user": "U1", + "text": "Is it planned?", + "thread_ts": "root", + "ts": "777", + }, + bot_user_id=None, + say=_unused_say, + client=client, + planning=planning, + ) + + assert planning.ownership_calls == [("C9", "root")] + assert len(runtime.calls) == 1 + assert runtime.calls[0][1].type == "receptionist_agent" + + +@pytest.mark.asyncio +async def test_an_explicit_thread_binding_beats_planning_ownership(): + # /ff-focus on this very thread is the one thing the user asked for by + # name; ownership does not take it away. #310 traced this and never pinned it. + focus = FocusManager( + ttl_seconds=60, allowed_agents=["receptionist_agent", "timeboxing_agent"] + ) + focus.set_focus("D1:root", "timeboxing_agent", by_user="U1", note="ff-focus") + runtime = _FakeRuntime([_FakeResult(TextMessage(content="ok", source="bot"))]) + runtime.timeboxing_session_store = _SessionStore({}) + client = _FakeClient() + planning = _PlanningReplyHandler( + ThreadReply(ThreadReplyOutcome.NO_PRESS, context="CARD CONTEXT"), owns=True + ) + + await _route( + runtime=runtime, + focus=focus, + client=client, + planning=planning, + event=_dm_reply_event("Is it planned?"), + ) + + assert len(runtime.calls) == 1 + assert runtime.calls[0][1].type == "timeboxing_agent" + + @pytest.mark.asyncio async def test_a_focus_manager_that_refuses_the_agent_also_refuses_the_claim(): # The claim and the focus binding are one decision. Assigning the agent diff --git a/tests/unit/test_stage_panel_in_the_turn.py b/tests/unit/test_stage_panel_in_the_turn.py index c4e15246..2b5f450a 100644 --- a/tests/unit/test_stage_panel_in_the_turn.py +++ b/tests/unit/test_stage_panel_in_the_turn.py @@ -93,7 +93,7 @@ async def turn(self, request, progress): # `_timeboxing_kernel(...)` is called once per turn (a fresh instance # each time), so the outcome advances on that call, not on `Kernel.turn` - # -- the same "once per turn" shape as `Repo.load_or_create` below. + # -- the same "once per turn" shape as `Repo.load` below. kernel_calls = {"n": 0} def make_kernel(*_a, **_k): @@ -112,7 +112,7 @@ class Repo: def __init__(self) -> None: self._loads = 0 - async def load_or_create(self, key, owner_user_id): + async def load(self, key): turn_index = min(self._loads // 2, len(snapshots) - 1) self._loads += 1 return snapshots[turn_index] diff --git a/tests/unit/test_stage_receipts_in_the_turn.py b/tests/unit/test_stage_receipts_in_the_turn.py index 2f027f9e..24794b6a 100644 --- a/tests/unit/test_stage_receipts_in_the_turn.py +++ b/tests/unit/test_stage_receipts_in_the_turn.py @@ -82,7 +82,7 @@ async def turn(self, request, progress): return outcome class Repo: - async def load_or_create(self, key, owner_user_id): + async def load(self, key): return snapshot class Runtime: diff --git a/tests/unit/test_timebox_failure_card_tells_the_truth.py b/tests/unit/test_timebox_failure_card_tells_the_truth.py index dac2c709..c232bd74 100644 --- a/tests/unit/test_timebox_failure_card_tells_the_truth.py +++ b/tests/unit/test_timebox_failure_card_tells_the_truth.py @@ -114,7 +114,7 @@ async def turn(self, request, progress): raise ValueError("the planning session does not accept another intent") class Repo: - async def load_or_create(self, key, owner_user_id): + async def load(self, key): return _snapshot(committed=True) class Runtime: diff --git a/tests/unit/test_timebox_session_surface.py b/tests/unit/test_timebox_session_surface.py index 13fb48cb..204ea990 100644 --- a/tests/unit/test_timebox_session_surface.py +++ b/tests/unit/test_timebox_session_surface.py @@ -15,11 +15,17 @@ from fateforger.slack_bot import handlers from fateforger.slack_bot.focus import FocusManager from fateforger.slack_bot.handlers import route_slack_event +from fateforger.slack_bot.messages import SlackBlockMessage from fateforger.slack_bot.timeboxing_commit import ( FF_TIMEBOX_COMMIT_START_ACTION_ID, build_timebox_date_card, ) +#: The link block the cross-channel echo appends so the DM can reach the +#: session thread. Minted by this system, so asserting on it is an identity +#: check, not a reading of prose. +FF_OPEN_THREAD_ACTION_ID = "ff_open_thread" + class _FakeRuntime: """route_slack_event demands one; the adaptive turn is stubbed past it.""" @@ -120,6 +126,31 @@ async def fake_turn(**kwargs): return calls +#: What a planner answer reads like. A fixture-minted constant: the test +#: asserts the delivery of this exact string, never anything about its words. +ANSWER_TEXT = "The 09:00 block is the one that moved; nothing else changed." + + +@pytest.fixture +def stub_answer_turn(monkeypatch): + """Replace the kernel turn with the one result that carries no blocks. + + `_answer_question` returns the planner's answer as plain text with an + empty block list -- deliberately, since a section block clips at 1600 + characters while `text` keeps 3900. Every other result the turn can + produce (cards, the failure message) synthesises blocks, so this is the + single shape the DM echo has to deliver without them. + """ + calls: list[dict] = [] + + async def fake_turn(**kwargs): + calls.append(kwargs) + return SlackBlockMessage(text=ANSWER_TEXT, blocks=[]) + + monkeypatch.setattr(handlers, "_run_adaptive_timebox_turn", fake_turn) + return calls + + @pytest.fixture def same_channel_session(monkeypatch): """Anchor the session in the origin channel (the /timebox-in-#plan-sessions case).""" @@ -304,6 +335,9 @@ async def test_a_dm_origin_handoff_still_delivers_the_card_to_the_dm( FF_TIMEBOX_COMMIT_START_ACTION_ID in _action_ids(w.get("blocks")) for w in dm_writes ), "the card never reached the DM origin message" + assert FF_OPEN_THREAD_ACTION_ID in _action_ids(dm_writes[-1].get("blocks")), ( + "a card echoed into the DM still carries the link to the session thread" + ) in_session_channel = [p for p in client.posted if p["channel"] == "C-timebox"] assert len(in_session_channel) == 2, "root and threaded card in the session channel" @@ -312,6 +346,55 @@ async def test_a_dm_origin_handoff_still_delivers_the_card_to_the_dm( assert not [p for p in client.posted if p["channel"] == "D1"][1:] +async def test_a_dm_origin_answer_without_blocks_still_resolves_the_dm_ack( + focus, stub_answer_turn, monkeypatch +): + """A blockless answer must leave the DM ack resolved, not spinning. + + The turn runs in the session channel, so the DM's "thinking..." message is + only ever resolved by the echo at the end of the surface. That echo used + to fire on `payload["blocks"]` alone, which is exactly what an answer does + not have: the question was answered in the channel thread while the DM + that asked it sat on the spinner forever. The answer is text, so the echo + is text -- and no link block, because any block at all makes Slack hide + `text` and a section block would reintroduce the 1600-char clip the + answer exists outside of. + """ + monkeypatch.setattr(handlers, "_channel_for_agent", lambda _agent: "C-timebox") + client = _CrossChannelClient() + + await route_slack_event( + runtime=_HandoffRuntime(), + focus=focus, + default_agent="receptionist_agent", + event={ + "channel": "D1", + "channel_type": "im", + "user": "U1", + "text": "what time does my day start?", + "ts": "555", + }, + bot_user_id=None, + say=_noop_say, + client=client, + ) + + assert stub_answer_turn, "the kernel turn ran" + + dm_origin_ts = client.posted[0]["ts"] + dm_writes = _writes_to(client, dm_origin_ts) + final = dm_writes[-1] + assert final["text"] == ANSWER_TEXT, ( + "the DM ack was never resolved into the answer" + ) + assert "blocks" not in final, ( + "a text-only answer must reach Slack as text; any block hides it" + ) + + # Still no second DM message: the ack is the delivery. + assert not [p for p in client.posted if p["channel"] == "D1"][1:] + + class _CardPostFailsClient(_FakeClient): """The root exists; the threaded working message never arrives.""" diff --git a/tests/unit/test_timeboxing_intents.py b/tests/unit/test_timeboxing_intents.py index f9e158a5..385f5c0b 100644 --- a/tests/unit/test_timeboxing_intents.py +++ b/tests/unit/test_timeboxing_intents.py @@ -17,7 +17,9 @@ from fateforger.agents.timeboxing.session_contracts import ( Advance, ApproveArtifact, + ArtifactApproval, ArtifactKind, + AskQuestion, BlockerOption, ChooseBlockerOption, ConfirmPlanningDay, @@ -503,7 +505,7 @@ async def test_an_offered_option_can_be_answered_in_words() -> None: assert ( # Stage 1 decision set, spec 2026-09-04 # No assume: `skeleton.day_shape` is not one of the forty-five cells, and a # `PlannerAssumption` cannot satisfy it. - '"allowed_decisions":["provide_facts","choose_option","back","cancel"]' + '"allowed_decisions":["provide_facts","choose_option","back","cancel","question"]' ) in prompt # The offer is the context the judgement needs: an id with no label beside # it asks the model to choose between two names it has never seen. @@ -563,7 +565,7 @@ async def test_an_open_question_still_has_nothing_to_choose_from() -> None: assert set(client.calls[0][1].model_fields) == {"decision", "facts"} prompt = "\n".join(message.content for message in client.calls[0][0]) assert ( # Stage 1 decision set, spec 2026-09-04 - '"allowed_decisions":["provide_facts","back","cancel"]' + '"allowed_decisions":["provide_facts","back","cancel","question"]' ) in prompt @@ -583,7 +585,7 @@ async def test_choosing_is_not_offered_when_no_question_is_open() -> None: assert set(client.calls[0][1].model_fields) == {"decision", "facts"} prompt = "\n".join(message.content for message in client.calls[0][0]) assert ( # Stage 1 decision set, spec 2026-09-04 - '"allowed_decisions":["provide_facts","back","cancel"]' + '"allowed_decisions":["provide_facts","back","cancel","question"]' ) in prompt @@ -891,7 +893,7 @@ async def test_a_committed_session_takes_a_revision_bound_to_its_receipt() -> No assert '"display_stage":"committed"' in prompt assert '"pending_artifact_kind":"commit_receipt"' in prompt allowed = json.loads(client.calls[0][0][1].content)["allowed_decisions"] - assert set(allowed) == {"provide_facts", "revise"} + assert set(allowed) == {"provide_facts", "revise", "question"} @pytest.mark.asyncio @@ -931,7 +933,11 @@ async def test_a_committed_session_does_not_offer_to_cancel_or_approve() -> None ) offered = get_args(client.calls[0][1].model_fields["decision"].annotation) - assert offered == ("provide_facts", "revise") + # `question` rides along in every open state, committed included: the + # calendar is written but the thread is still live (#316). It is neither + # of the two this test is named for, which stay absent. + assert offered == ("provide_facts", "revise", "question") + assert "cancel" not in offered and "approve" not in offered @pytest.mark.asyncio @@ -1248,7 +1254,12 @@ def test_assume_with_nothing_open_is_refused() -> None: # exercises its own defence-in-depth refusal. interpreted = InterpretedTimeboxTurn(decision="assume") with pytest.raises(ValueError, match="no open"): - _intent_from_interpreted(interpreted, snapshot=_stage1_snapshot(), pending=None) + _intent_from_interpreted( + interpreted, + snapshot=_stage1_snapshot(), + pending=None, + user_text="just move on", + ) async def test_deny_names_an_assumption_on_record() -> None: @@ -1297,3 +1308,140 @@ async def test_a_malformed_suspension_fact_makes_restore_raise_naming_the_fact() await TimeboxingIntentInterpreter(client).interpret( "put the oats rule back", _stage1_snapshot(facts=[malformed]) ) + + +@pytest.mark.asyncio +async def test_a_question_during_capture_binds_the_users_words_verbatim() -> None: + client = _SchemaOutputClient({"decision": "question", "facts": []}) + interpreter = TimeboxingIntentInterpreter(client) + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=2, owner_user_id="U1", + planning_day=_planning_day(), status="open", + ) + intent = await interpreter.interpret(" Is it planned? ", snapshot) + assert isinstance(intent, AskQuestion) + assert intent.question == " Is it planned? " # verbatim, not stripped, not paraphrased + _, json_output = client.calls[0] + assert "question" in get_args(json_output.model_fields["decision"].annotation) + + +@pytest.mark.asyncio +async def test_a_question_on_a_committed_session_is_offered_and_bound() -> None: + client = _SchemaOutputClient({"decision": "question", "facts": []}) + interpreter = TimeboxingIntentInterpreter(client) + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=9, owner_user_id="U1", + planning_day=_planning_day(), status="committed", + ) + intent = await interpreter.interpret("what did we settle on for lunch?", snapshot) + assert isinstance(intent, AskQuestion) + + +@pytest.mark.asyncio +async def test_a_cancelled_session_still_accepts_no_intent() -> None: + client = _SchemaOutputClient({"decision": "question", "facts": []}) + interpreter = TimeboxingIntentInterpreter(client) + snapshot = PlanningSessionSnapshot( + session_key="C1:1.0", revision=4, owner_user_id="U1", + planning_day=_planning_day(), status="cancelled", + ) + with pytest.raises(ValueError, match="does not accept another intent"): + await interpreter.interpret("Is it planned?", snapshot) + assert client.calls == [] + + +def _validated_candidate() -> PlanningArtifact: + return PlanningArtifact.create( + artifact_id="candidate-1", + kind=ArtifactKind.VALIDATED_CANDIDATE, + revision=1, + payload={"events": []}, + dependency_revisions={"skeleton": 2}, + ) + + +def _approval_of(artifact: PlanningArtifact) -> ArtifactApproval: + return ArtifactApproval( + artifact_id=artifact.artifact_id, + artifact_revision=artifact.revision, + artifact_digest=artifact.digest, + actor_user_id="U1", + session_revision=3, + ) + + +def _snapshot_awaiting_commit() -> PlanningSessionSnapshot: + """A validated candidate on the table and nothing approved yet.""" + return _capture_snapshot().model_copy( + update={"artifacts": [_validated_candidate()]} + ) + + +def _snapshot_past_the_skeleton() -> PlanningSessionSnapshot: + """The skeleton is approved, so nothing is pending and the day is refining.""" + skeleton = _skeleton() + return _capture_snapshot().model_copy( + update={"artifacts": [skeleton], "approvals": [_approval_of(skeleton)]} + ) + + +#: One snapshot per state `_display_context` returns, `cancelled` excepted and +#: pinned separately below -- all seven of them. The state name is asserted +#: alongside the decision set because a snapshot that quietly fell through to +#: `capture` would satisfy a membership check while testing nothing about the +#: state it was built for, which is exactly what the first version of this +#: guard did. +_OPEN_STATES: tuple[tuple[str, PlanningSessionSnapshot], ...] = ( + ( + "no_session", + PlanningSessionSnapshot(session_key="C1:1.0", revision=0, owner_user_id="U1"), + ), + ("planning_day", _date_stage_snapshot()), + ("capture", _capture_snapshot()), + ("skeleton", _snapshot_with_skeleton()), + ("review_commit", _snapshot_awaiting_commit()), + ("refine", _snapshot_past_the_skeleton()), + ("committed", _committed_snapshot()), +) + + +@pytest.mark.parametrize(("expected_stage", "snapshot"), _OPEN_STATES) +def test_every_open_state_offers_question( + expected_stage: str, snapshot: PlanningSessionSnapshot +) -> None: + """The contract: an agent that owns a workflow exposes `question` in every + state its surface allows. One case per state `_display_context` returns + today, so a miswired case cannot hide behind another state's answer. An + eighth state added later must be added here too; nothing enforces that.""" + stage, allowed, _ = _display_context(snapshot) + assert stage == expected_stage + assert "question" in allowed + + +#: Cancelled twice over: mid-session, and before a day was ever proposed. The +#: second case is only reachable since #318 put `cancel` on offer at zero +#: artifacts, and it is the one a `no_session` branch placed too early would +#: reopen -- the thread would take a start, then die on the confirmation with +#: the generic failure line. +_CANCELLED_SESSIONS: tuple[tuple[str, PlanningSessionSnapshot], ...] = ( + ("mid-session", _capture_snapshot().model_copy(update={"status": "cancelled"})), + ( + "before a day", + PlanningSessionSnapshot( + session_key="C1:1.0", + revision=1, + owner_user_id="U1", + status="cancelled", + ), + ), +) + + +@pytest.mark.parametrize(("case", "cancelled"), _CANCELLED_SESSIONS) +def test_a_cancelled_session_offers_nothing( + case: str, cancelled: PlanningSessionSnapshot +) -> None: + """The one state that must not gain `question`: the session is closed.""" + stage, allowed, _ = _display_context(cancelled) + assert stage == "cancelled" + assert allowed == () diff --git a/tests/unit/test_turn_cancels_ladder.py b/tests/unit/test_turn_cancels_ladder.py index 7a01be6a..dd9e8abd 100644 --- a/tests/unit/test_turn_cancels_ladder.py +++ b/tests/unit/test_turn_cancels_ladder.py @@ -49,6 +49,25 @@ async def cancel_followups(self, **kwargs): return 0 +class _ActivityRecorder: + """Stands in for the `timeboxing_activity` singleton. + + Shape copied from `test_asked_is_answered_in_the_turn.py`. Patching it is + not only for the assertion: without it the turn marks the real + process-wide recorder, which every other test in the run then sees. + """ + + def __init__(self) -> None: + self.active: list[str] = [] + self.inactive: list[str] = [] + + def mark_active(self, *, user_id, channel_id, thread_ts): + self.active.append(user_id) + + def mark_inactive(self, *, user_id): + self.inactive.append(user_id) + + async def test_a_turn_records_activity_on_the_session_topic(monkeypatch) -> None: haunting = _HauntingService() @@ -57,9 +76,9 @@ async def turn(self, request, progress): return TurnFailed(code="x", message="x") class Repo: - async def load_or_create(self, key, owner_user_id): + async def load(self, key): return PlanningSessionSnapshot( - session_key=key, revision=1, owner_user_id=owner_user_id + session_key=key, revision=1, owner_user_id="U1" ) class Runtime: @@ -86,15 +105,16 @@ class Runtime: async def test_a_turn_that_ends_the_session_cancels_the_ladder(monkeypatch) -> None: haunting = _HauntingService() + activity = _ActivityRecorder() class Kernel: async def turn(self, request, progress): return TurnFailed(code="x", message="x") class Repo: - async def load_or_create(self, key, owner_user_id): + async def load(self, key): return PlanningSessionSnapshot( - session_key=key, revision=1, owner_user_id=owner_user_id, + session_key=key, revision=1, owner_user_id="U1", status="committed", ) @@ -106,6 +126,7 @@ class Runtime: monkeypatch.setattr(handlers, "derive_timebox_intent", _noop_intent) monkeypatch.setattr(handlers, "HarnessProgressCard", _StubCard) monkeypatch.setattr(handlers, "present_outcome", lambda *a, **k: ("rendered", None)) + monkeypatch.setattr(handlers, "timeboxing_activity", activity) await handlers._run_adaptive_timebox_turn( runtime=Runtime(), client=object(), logger=logging.getLogger(__name__), @@ -118,3 +139,7 @@ class Runtime: assert haunting.activity == [ {"topic_id": "C1:2.0", "task_id": None, "user_id": "U2"} ] + # The ladder cancel and the idle timer are two halves of one teardown, and + # only the ladder half was ever asserted here: `mark_inactive` could stop + # firing and this file would stay green. + assert activity.inactive == ["U2"]