tasks: one port for the day's board, and the section that shows it (#401) - #436
Open
hugolytics wants to merge 11 commits into
Open
tasks: one port for the day's board, and the section that shows it (#401)#436hugolytics wants to merge 11 commits into
hugolytics wants to merge 11 commits into
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The day's candidates are read once, through one port, so the judgement that decides which tickets the person named and the surface that shows them the board see the same list. Two reads could disagree and nothing would notice. `BoardTaskSource` maps `TaskBoard`'s rows: GTD state by equality against Notion's own enums (Status first, so a finished ticket does not read as `waiting_for`), overdue by arithmetic against the day, `truncated` from the listing's cursor. Every failure -- `TaskBoardError` and the transport errors that arrive from inside the MCP client alike -- becomes `TaskSourceUnavailable` carrying its cause, never an empty list standing in for an error. Nothing is wired here: the host still calls the board directly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…at they stand for (#401) The mapping sits inside the port's guard so a `due` that will not parse fails loudly -- dropping it would turn an overdue ticket into one that is not, which is the silent wrong answer the caller's unresolved flag exists to prevent. No test held it there: moved outside the `try`, all 28 still passed and a bare `ValueError` would escape every caller that handles `TaskSourceUnavailable`. `BoardTaskSource` is passed around as a `TaskSource` and `FakeBoard` in place of a `TaskBoard`, and neither relationship is checked at runtime. Signatures are now compared whole, the way `test_task_board_mcp.py` holds its stub. Also corrects what the default limit means: the work-lookup eval runs on a frozen twelve-row snapshot and this cannot move it. The hazard is production, where a caller taking the default would show the judgement twelve of the hundred rows the host asks for. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ent share the list (#401) `work_refs_for_turn` takes a `TaskSource` in place of a board and reads it once per turn, passing `WORK_ROW_LIMIT` explicitly: the port defaults to a readable page of twelve, and a caller taking that default would show the judgement twelve of a hundred ready rows and still get a plausible answer back over a silently narrowed list. That one listing goes two places. `resolve_work` is handed it -- in the port's order, unsorted, because the prompt tells the model that order is the person's ranking -- and it comes back on `WorkRefs.candidates`, through `PlanningContext` to the snapshot the cards read. `None` there means no board was read; a board that answered with nothing is an empty `rows`, and the surface has a different sentence for each. It stops at the snapshot: `PlanningBrief` is untouched, because the planner is handed resolved refs and a sprint's rows on every brief would be tokens spent on nothing. Every failure path keeps its event name and stays non-blocking. The three that happen after a successful read keep the listing, so the person can still be shown what was on offer with nothing marked taken from it; only `work_board_unavailable` has nothing to carry. `resolve_work` now takes `TaskCandidate`, which gains `summary` and `url` -- the two board facts its consumers need. The summary is the tail of the line the prompt renders and the url is what the material store is given; without them the prompt would have changed and the material write would have had to mint a link. **The prompt is byte-identical** over all six eval cases, verified against the pre-change code, and the eval was re-run on the flash pin: 8/8 on every case across four sweeps, including the finance case that pooled 109/112 before. No rate moved. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…n from it (#401) The context panel now draws a section under the work line: a head naming the sprint, then the day's candidates with the ones this day was planned around marked. The list is the one the work-lookup judgement was handed -- the board is read once and mirrored onto the snapshot -- so what the person is shown and what was judged over cannot disagree. The chosen mark is a join on the board number, an integer the board minted, never on the ticket's name: two rows can share a name, and a wrongly marked row tells the reader the day is about a ticket it is not. A ref with no number marks nothing. Three states, three sentences, because the difference between them is the whole point: * no board was read -- nothing is drawn. Either nobody asked for work, or the mirror cleared the listing on a turn that never looked. Drawing the previous read's rows here would present them as today's offer. * a board was read and offered nothing -- one line saying so, so an empty sprint does not read as a board nobody asked. * a board was read and offered rows -- the head and the rows, capped by the work line's own count cap with a "+N more" tail. A board that could not be read keeps the unresolved sentence and adds no second vocabulary for the same fact. An unresolved turn whose read *did* succeed still shows the rows, with nothing marked, for the reason `_work` names no ticket there. `shown_with_of` gains the board's row ids and a read mark, so a turn that changed what the board offered redraws the panel rather than leaving a stale one -- and so a board that answered with nothing does not compare equal to no board at all. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…er (#401) Two deferred minors from Task 2's review. `work_lookup_failed` filed `error_type` from the exception it was handed, and since the board moved behind `TaskSource` that is always `TaskSourceUnavailable` -- so a Notion 503, a missing token and a malformed page all logged under one label. The sibling catch on `TaskBoard.from_settings` passes the real exception, so one event name was carrying two type vocabularies. It now reads the cause when there is one, which is the fault in both paths, and the traceback still carries the wrapper. The deferred `TaskBoard` import inside `_work_refs` was decorative: the module-level `task_source` import pulls `board` anyway, so the deferral saved no import and only hid where the name comes from. Hoisted. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…hing once (#401) Review round on the section. **The redraw test could pass for the wrong reason.** Its three listings differed in length as well as in identity, so a board term carrying only a count -- `board:read` plus `board:n=2` -- satisfied all three while leaving the panel stale for the change that actually happens: a ticket closed and another taking its place. Pinned by a listing of the same length with one row swapped, and confirmed by mutating the term to a count, which reddens only that test. **The cap is the section's own, and it is twelve.** Three is right for the work line -- a glance to catch a wrong resolution -- and wrong here: on the turn that prints "say which one and I'll attach it", showing three of twelve defeats the only job the section has. Twelve is the current sprint's real size and the port's own `DEFAULT_CANDIDATE_LIMIT`, and twelve rows land near 720 characters against the 1600 Slack renders, so a test now pins it from both sides. That dissolves a second finding. `shown_with` is a frozenset, so a re-read returning the same rows reordered does not redraw -- and under a cap shorter than the sprint that silently *dropped* a row, because the panel kept its old top-N while a row a Priority edit had moved into it was never drawn. At a cap of twelve every row is on the card whatever the order. The comment where the frozenset tradeoff is accepted now says the cap is load-bearing for it. Two copy fixes. A sprint titled with a space rendered `From your board — :`; it is the empty case, tested for rather than stripped off a name that has one. And an unresolved turn over an empty board printed "say which one and I'll attach it" directly above "nothing to plan around" -- an instruction with nothing to point at, reachable through `work_lookup_failed` on an empty sprint. The sentence carries the whole fact, so the section stays out of its way. `work_lookup_failed` spells the cause check `is not None` rather than leaning on an exception being truthy. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
materials-and-work-links.md: document the TaskSource port -- read once per turn, shared by the work-lookup judgement and the context panel's board section; the state mapping over Notion's own enums; why candidates=None (never read, or read failed) is a different fact from an empty rows list; and the deferred Refined widening, deferred because the judgement was measured over Ready rows only and widening the input without re-running the eval would move a measured judgement. Also documents the board section's rendering and its join on the ticket number, never the label. stage-ux-port-design.md: correct the three lines describing a "from your board" section on the stage-2 card, staying empty until a task backend exists. #401 put the section on the stage-1 context panel instead, beside the resolved work line, because the candidates and the resolved refs are two halves of one thing and splitting them across a card and a panel would make the reader assemble them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…it changes (#401) `shown_with_of`'s board term was an unordered set of external ids, and its comment argued the cap made that safe: at `BOARD_ROW_CAP` = 12, "the sprint's real size", every row lands on the card whatever the order. That is false in production. The cap bounds what is *shown*, not what is read, and the only production caller passes `WORK_ROW_LIMIT` = 100 (`timeboxing_host.py`), so the port returns up to a hundred rows and the section shows twelve. On a twenty-row Ready sprint, a Priority edit in Notion that promotes row thirteen leaves the term comparing equal, `stage_card_registry.py` skips the redraw, and the reader goes on seeing rows one to twelve in the old order while the judgement had already judged over the new one — the invariant this branch exists to establish, leaking on the ordering axis rather than the membership one. The term now carries each row's position. The comment, `BOARD_ROW_CAP`'s docstring and the architecture page say the cap is not load-bearing for `shown_with_of` any more, and the cost that is accepted instead: a panel edit on a turn where only the order moved. Also corrects the same docstring's closing claim that anything past the cap becomes "+N more". `render_context_panel` cuts the head at `SLACK_MAX_BLOCK_TEXT_CHARS`, and the cut takes the tail rows and the count line with them, silently; the count line only ever reports rows this cap dropped. Measured against the real twelve-row board, the section is 918 characters and the whole panel 1087 of 1600. The new test fails against the unfixed term (1 failed, 18 passed) and is the only test that does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…417) `session_contracts` imports `TaskCandidates`, which pulled `agents.tasks.board`, which imports `mcp.client.streamable_http` and builds a `Settings()` at module scope. Measured, that module's import went from 143ms to 1327ms — and the cost is not only latency: a pure contracts module, one the per-turn stdio children load, came to transitively require the MCP client package and a constructible config in order to import at all. `Scope`, `TaskBoard`, `TaskListing` and `TaskRow` appear only in signatures, `from __future__ import annotations` is already on, and no pydantic field is typed by one, so they move behind `TYPE_CHECKING`. `session_contracts` now imports in 160ms against 1234ms (three runs each: 1272/1189/1243 before, 181/145/155 after). Pinned by a subprocess test — the suite has already imported `board` for its own fakes, so the assertion has to run in a fresh interpreter. It fails against the eager import. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The fixture minted `https://www.notion.so/{page_id}`, which is what `TaskCandidate.url`'s own docstring forbids: a url this system made up is one nobody can follow, and here it sat in the file that documents the measurement looking exactly like a fact the board supplied. The snapshot captured the page id and not the link, so the field is empty and says why. Harmless by construction — `build_prompt` renders no url — and verified as such: its SHA-256 over these rows is identical before and after the change on all six eval messages. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #401. Implements the decision on #279. Closes #418 (docs) and #417 (the import cost, fixed here rather than deferred).
Problem
work_refs_for_turnread the board itself, so the list the judgement saw existed only inside that call. Anything showing you what is on your board would have fetched its own list, and the two could differ — a row offered to you that the judgement never saw, or the reverse — with nothing able to detect it.What this adds
TaskSourceport (agents/tasks/task_source.py) withBoardTaskSourcebehind it. Read-only, no model call, one candidate row per ticket carrying the board's own facts. Every failure isTaskSourceUnavailablewith the cause attached; an empty list never stands for an error.candidatestravels onWorkRefs,PlanningContextand the session snapshot, following the seamwork_refs_unresolvedalready uses, and deliberately not ontoPlanningBrief: the planner gets resolved refs and has no use for the candidate list, so twelve rows on every brief would be a token cost for nothing. A test fails if anyone adds them.The judgement did not change, and that is measured
The lookup's prompt is built from the new candidate type, so the risk was moving a judgement that had been measured over the old one.
build_promptoutput is byte-identical by SHA-256 on all six eval messages, before and against the change. Verified by the implementer and re-derived independently by the reviewer.Two deliberate deviations, both recorded in the plan
The candidate set is
Readyonly, notReadyorRefined. #279's decided scope is the wider one, but the judgement reading this list was measured over exactly the current sprint'sReadyrows, and widening it without re-running the eval would move a measured judgement. The widening gets its own ticket and its own eval. A consequence worth knowing: under this scope every row isnext, so thewaiting forandsomedaytags never print today.The section renders on the context panel, not the stage-2 card. The older stage-UX spec puts it on the card. Since that was written, #398 put the resolved work on the panel, and the candidates and the refs are two halves of one thing; splitting them across two surfaces would make the reader assemble them. The spec's lines are corrected rather than left contradicting the code.
Proof
tests/unit3057 passed, 2 skipped, 1 xfailed.tests/integration -m "not slow"33 passed.session_contractsimport went from about 1234 ms to 160 ms, because importing the candidate type had been pulling the MCP client and a module-scope settings object into a pure contracts module that the per-turn children import.Human checklist
Follow-ups filed, not smuggled in
#415 the chosen-mark join is number-only and would collide once a second backend exists. #416
truncatedis not surfaced, so the tail count can understate past a hundred rows. Both need conditions that do not exist today.🤖 Generated with Claude Code