Skip to content

refactor: retire the legacy timeboxing agent, and the 34 modules only it reached - #444

Merged
hugolytics merged 13 commits into
chore/prune-and-consolidate-testsfrom
chore/retire-legacy-timeboxing-agent
Sep 11, 2026
Merged

refactor: retire the legacy timeboxing agent, and the 34 modules only it reached#444
hugolytics merged 13 commits into
chore/prune-and-consolidate-testsfrom
chore/retire-legacy-timeboxing-agent

Conversation

@hugolytics

Copy link
Copy Markdown
Collaborator

Problem

TimeboxingFlowAgent — 8,936 of its own lines, plus the 34 modules only it
reached — has been unreachable from any live entry point since
_timebox_backend() started answering harness unless
FF_TIMEBOX_BACKEND=legacy, a variable set nowhere outside
tests/conftest.py. It was still registered on the AutoGen runtime
unconditionally, still instantiated in the running bot with nothing routed
to it, and four live send_message call sites still addressed it directly
— each one a bare Exception("Recipient not found") waiting for the day the
class was deleted without those sends being closed first. This PR closes
those four sends, ports the one behaviour the harness genuinely lacked (a
refine-loop no-progress cap legacy had and the harness's uncapped
NeedsAnotherTurn did not), then deletes the class, the constraint-review
surface only it wrote to, and every module reachable only through it —
across thirteen commits, each task-reviewed, with the deletion landing only after the sends were closed and the cap was ported.

Measured numbers

  • src/: 74 files changed, 442 insertions(+), 21,830 deletions(-) — 56 modules deleted
    (git diff --stat 31e88b5..HEAD -- src/)
  • tests/: 100 files changed, 902 insertions(+), 15,295 deletions(-) — 77 test files deleted (67 in the retirement commit)
    (git diff --stat 31e88b5..HEAD -- tests/)
  • Fast suite: 3,136 tests before → 2,630 after (one pre-existing
    environmental failure —
    test_the_deployed_profile_matches_the_repository — on both sides)
  • Wall-clock: ~28s, against the 31.6s baseline (well inside the 10%
    budget)
  • scripts/dev/tests/orphans.py: 0 newly orphaned modules
  • scripts/dev/tests/dangling.py 42d9eb2..HEAD: 54 deleted modules, 0
    surviving src/ importers
  • -m slow still collects 89 tests (untouched by this cut)
  • The AST honesty guard landed in Task 8 with a measured, per-file
    allowlist: 34 files, 149 offences (24 __new__, 125 private writes)
    this is the starting point Project 2 (test-suite composability) shrinks
    from, not the "few dozen" the original spec estimated

Also in the fix wave after the whole-branch review

Three constraint-review button ids from legacy thread roots had no handler once constraint_review.py went — the exact dead-button failure the retired-cards handler exists to end. Ten ids are retired now, not seven. docs/architecture/agents.md no longer calls the deleted class the primary planner.

Two spec deviations from docs/superpowers/specs/2026-09-08-test-suite-composability-design.md

  1. mcp_clients.py scope. The spec said the whole file goes. Measured:
    only McpCalendarClient and CalendarDaySnapshot leave it —
    ConstraintMemoryClient stays, because it is the default
    tasks_defaults_memory_backend and has tests of its own that have
    nothing to do with the legacy agent.
  2. settings.timeboxing_memory_backend. The spec said this setting and
    its validator go with agent.py. Measured: kept — it is also read by
    runtime.py's graphiti startup checks and by tasks' defaults memory;
    only agent.py's own branch on it was deleted.

(A third correction, not a deviation in behaviour: Project 2's "Injection —
none" section listed ConstraintMemoryClient as deleted in project 1 —
corrected to say it survives, and that its workbench= seam — measured
0.5s → 17.9s without it — is the one project 2 actually adds. The spec's
"allowlist shrinks to TBPlan" line now states the measured 34-file/
149-offence starting point it shrinks from.)

Coverage: three real regressions, named rather than explained away

Per-line coverage diff against PR #396 (covdiff.py, root-argument bug
fixed and re-verified in Task 7) found several surviving files that
lost covered lines a now-deleted test used to reach. Two of the original
five candidates are dead code, not regressions — their only consumer,
CalendarEventWorkerAgent (agents/schedular/agent.py:~901), is never
registered in runtime.py (git grep -n CalendarEventWorkerAgent returns
only its own definition), so nothing live reaches these lines at all. Most
of durable_constraint_store.py's lost lines are the same shape — callers
nothing calls. What's left is three real, live-code-now-untested lines:

  • agents/timeboxing/durable_constraint_store.py:506 — the
    if not rows and search_names re-query, an ordinary-day path (not an
    edge case): the rest of the file's lost lines (326-329, 334-336, 357,
    539, 561) are get_store_info and callable(...) fallbacks the live
    client never takes — dead code, not a regression
  • debug/diag.py:46-48 — the except Exception arm of with_timeout,
    imported by four live agents; the surviving test only covers the
    TimeoutError arm
  • contracts.py:51 — the one ordinary-day loss: an all-day event arrives
    as {"date": ...} rather than {"dateTime": ...}, and haunt/reconcile.py
    imports contracts for exactly this parse

durable_constraint_store.py:506 and contracts.py:51 are ticketed —
tickets/recover_two_lines_the_retirement_left_untested.md — rather than
fixed in this pass; debug/diag.py:46-48 is an accepted known gap.

Dead-code cleanup candidates the retirement exposed (not fixed this round)

Named but not removed — a source change, out of scope for a docs/test-
retirement PR:

  • llm/factory.py:151-158 — the calendar_submitter and timebox_patcher
    branches; their only users (submitter.py, patching.py) are deleted
  • tmbx/journal/store.py:31-34journal_sessionmaker; its only caller
    was the deleted agent.py:339
  • agents/schedular/models/calendar.py:185, 246, 351-352, 367-371, 392-402, 408-411 and agents/schedular/models/core.py:143_parse_event_type,
    _parse_time, the _check_time_combo validator, the colorId field:
    CalendarEventWorkerAgent, their only consumer, is never registered in
    runtime.py
  • agents/timeboxing/durable_constraint_store.py:325-329, 334-336, 357
    get_store_info (zero callers) and the callable(...) fallbacks the
    live client never takes

(Three more class-(a) findings from the same coverage diff —
adapters/calendar/models.py, tool_result_models.py,
constraint_review.py — were already deleted in this branch, Tasks 6–7.)

Follow-up tickets filed

  • tickets/harness_bounded_brief_says_what_it_withheld.md — new. The
    harness puts every applicable constraint row into the brief today (nothing
    withheld), but the day the brief is bounded, nothing makes the truncation
    speak — legacy rediscovered that rule three times (3dea6ae, e0c1f30,
    Journal: ACCEPTED is the fallback, so an unattended auto-commit becomes a fake approval label #177).
  • tickets/test_suite_docs_after_legacy_retirement.md — new, filed and
    dispatched to a sonnet subagent per CLAUDE.md; its commit is folded
    into this branch. Brought tests/README.md's "What's out" section, the
    two superseded root calendar docs, the timeboxing agent's README.md +
    AGENTS.md, core/README.md, and slack_bot/README.md back in line
    with what the retirement actually deleted.
  • tickets/skeleton_pre_generation.md — status line updated (not new): the
    confirm/undo wiring it describes was retired with the legacy agent on
    this branch; Undo lives on ff_harness_undo / handlers.act_harness_undo
    now.
  • tickets/recover_two_lines_the_retirement_left_untested.md — new. The
    two real coverage regressions the corrected classification leaves
    (durable_constraint_store.py:506, contracts.py:51), each with the
    one test that would cover it. Not blocking.

The live-drive checklist — for Hugo, not for an agent

This is the one gate the suite cannot stand in for: commit dda88f4
records every unit test passing while the live bot went to legacy. The
how-to is in the memory note "Driving a live Slack timeboxing session".

  • /timebox in the timeboxing channel opens a session root with the
    working card as first reply
  • A plain "plan tomorrow" in a normal channel: the receptionist hands
    off, and a session root appears in the timeboxing channel (or where I
    typed, if none is configured) — no :warning: Exception: Recipient not found
  • A second message in the open DM session continues it in the same
    thread
  • Press Undo on a committed candidate: the calendar write reverses and
    the card says so
  • Press a button on an old legacy stage card in history: it rewrites to
    the retired-flow sentence

🤖 Generated with Claude Code

hugocool and others added 13 commits September 9, 2026 12:47
Fails today on two of four handoff configurations and on the second DM
turn -- the redirect route and the handoff fall-through still send to
the agent directly. Green once the sends are closed (Task 2) and the
registration is gone (Task 5).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two sends still addressed timeboxing_agent directly: the redirect route,
which every second DM turn takes once a session is open, and the handoff
fall-through, reached whenever no session channel is configured or the
user is already in it. Both now continue on the kernel. A retired
registration would have turned each into Exception('Recipient not
found') rendered as a warning.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two sends still addressed timeboxing_agent directly: the redirect route,
which every second DM turn takes once a session is open, and the handoff
fall-through, reached whenever no session channel is configured or the
user is already in it. Both now continue on the kernel. A retired
registration would have turned each into Exception('Recipient not
found') rendered as a warning.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The stage and submit cards' dispatchers sent to timeboxing_agent inside
a broad except that rewrote the card to 'please try again'. With the
agent gone that would have been a transient glitch forever. One handler
now answers all seven action ids honestly.

Deviation from the task brief: build_text_section_block and four sibling
block-builder functions lived in the two modules this task deletes, and
TimeboxingFlowAgent (agent.py, not yet deleted -- that is Task 5) still
imported all five to render its own cards. Moved all five verbatim into
messages.py instead of just the one retired_cards.py needs, and repointed
agent.py's two import lines there, so the legacy agent keeps working
unmodified until its own deletion. Also touched the README/AGENTS docs
that named the deleted modules or the now-dead action-id dispatch, so
they do not describe code that no longer exists.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…pped

Legacy capped consecutive no-change refine passes at three (9eb333e); the
kernel's NeedsAnotherTurn had no cap, and _another_turn's own docstring
named the loop it could not see. The streak is read from the outcome
kinds the snapshot already records, so nothing new is stored.

_advance_request in tests/doubles/timeboxing.py gains an interaction_id=
keyword (default unchanged) because InMemoryPlanningSessionRepository.save
replays a repeated interaction_id instead of re-running the planner, which
would have flattened every scripted turn in the new test onto one.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Off everywhere since 2026-08-22, registered unconditionally at startup
with nothing routed to it. The harness covers all five stages, the
confirm gate (stronger: deny by default), Undo, and the migrated
constraint store; the one behaviour it lacked landed in the previous
commit. The reachability tool reports zero orphans after the cut; the
identifier 'timeboxing_agent' stays where it is a handoff target, a
persona key, or a session-key prefix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
kg_constraint_client's rows now go through the durable-store adapter the
harness actually reads; the tmbx patch-order test replays the model's own
dump, which is the only serialiser left on that path. The reachability,
dangling-import, coverage-diff and seam-taxonomy tools move into
scripts/dev/tests/ so the retirement's gates are reproducible.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…, taxonomy output path un-hardcoded

Four fixes from Task 7 review: covdiff.py's source-root argument is now
required and it refuses to run when fewer than half the before-report's
paths resolve under it, so a wrong or missing root can no longer misfile
every regression as "deleted" in silence. adapters/calendar/models.py has
zero importers and zero tests since 67489cd deleted its last caller
(test_sync_engine.py), so its orphans.py KEPT_DESPITE_UNREACHABLE entry was
false; the module and the entry are both gone. taxonomy.py's per-test JSON
path is now an optional argument defaulting beside the script instead of a
hardcoded scratchpad path from one session, and the seam summary always
prints to stdout.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Its writers were agent.py and a review modal only legacy posted. After
the retirement the table is permanently empty and the harness path
re-read it after every turn -- a no-op dragging get_constraint_store
through four handler signatures. ConstraintStore now names one class in
the repo, the memory server's.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The offence walker judges every assignment shape reaching a private
attribute (plain, tuple/list-unpacking, starred, augmented, annotated),
not just plain assignment, so the allowlist stays a true ceiling. Each
offence is keyed by its target's own line:col, not the statement's line,
so two dishonest targets sharing one line (tuple unpacking, chained
assignment) are counted separately rather than one hiding the other.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…code as live

Two root calendar docs now note McpCalendarClient's removal and tmbx/calendar/
as its replacement; core/README.md and the timeboxing package's README/AGENTS
stop describing the deleted TimeboxingFlowAgent coordinator, GraphFlow
orchestration, sync engine, patcher, and Notion-backed constraint plumbing as
live, and instead say the Stage 1 elicitation loop and durable
constraint-memory backends (now read by tasks' defaults memory and
runtime.py's startup checks) are what remains; slack_bot/README.md explains
the two rows commit 6f93212 removed instead of leaving the removal silent.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nest coverage list

The constraint-review buttons on legacy thread roots had no handler at all
once constraint_review.py went -- the exact dead-button failure the
retired-cards handler was written to end. Ten ids now, not seven.

docs/architecture still called the deleted agent the primary planner;
tests/README undercounted the deleted tests; a ticket told a reader to run
six files that do not exist; the docs ticket said Open for work already
landed. Fixed, and the docs ticket now covers docs/architecture too.

The coverage classification is corrected in the branch's favour: the
schedular calendar models are dead code (their only consumer is never
registered), most of durable_constraint_store's lost lines are callers
nothing calls, and the one ordinary-day loss is contracts.py:51 -- an
all-day event's date. Ticketed with durable_constraint_store.py:506.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@hugolytics
hugolytics merged commit 155cf43 into chore/prune-and-consolidate-tests Sep 11, 2026
@hugolytics
hugolytics deleted the chore/retire-legacy-timeboxing-agent branch September 11, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants