fix(witan): cut the context hook's cold read cost and raise its timeouts - #372
Merged
Merged
Conversation
`witan inject-context` against a deployment took 16-23s cold on a graph with 19 active projects and 184 ready tasks, so Claude Code killed the UserPromptSubmit hook at 15s and discarded the output. The user paid the full wait and got nothing, and the agent proceeded with no knowledge of active projects, in-flight branch tasks, or `claimed by` markers. Two things were behind it. `task_ready`'s repo-scoped branch scans every Task and then narrows to the candidates, but it resolved blocker statuses against the narrowed set. A blocker living in another repo is never a candidate and so was never in the lookup, which sent it back to the store one `get_task` per blocker for rows the scan had already returned. Against the deployed service that was ~3.0s of a 4.2s call, measured against a 1.2s `task_list` doing the same two scans. The hook then issued up to ten of those calls one at a time, so the cold path was their sum. They are mostly independent, so it now runs two waves: projects, ready tasks, branch tasks and held tasks together, then sessions and comment threads, which need the first wave's answers. Measured in-process against the deployment, 11.3s -> 6.5s median with byte-identical output. Threads rather than asyncio because `server` is duck-typed on plain attribute-style methods, and every call is I/O-bound. Each read keeps the per-read failure isolation the hook depends on, since a read that fails comes back as a value rather than a live exception, and a machine that cannot start threads runs the calls in line instead. The same narrowed lookup was a correctness bug on the local path, where an unknown blocker counts as closed: a task blocked by an open task in another repo was advertised as ready to work. `filter_ready` now takes the wider row set, which the hook already had in hand. Only the client-side half is live on merge. The `task_ready` fix lands for deployed users when the service is redeployed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WdyXu9F6St8F4E427gpxeP
Both hooks `witan setup` installs carried `timeout_seconds=15`, which sat inside the cost distribution of what they were timing rather than above it. A timeout there is the worst of both: the work is killed mid-flight, so the user pays the full wait and the result is discarded anyway. `witan inject-context` was measured at 16-23s cold on a large graph. The reads behind that are fixed in the preceding commit, so 45s is headroom for a graph bigger than the one measured rather than a budget anything is expected to use. `witan session-checkpoint` writes, via `workflow_session_end`, and a single write against a deployment has been measured at up to 51s - see the credential-refresh note in `witan_core.remote.proxy._invoke`. Killing that one does not drop a block, it leaves a session open with no handoff summary, which stays invisible until someone resumes and finds nothing recorded. 60s covers the recorded worst case. The pi `workflow-context` extension shells out to the same `witan inject-context` at `timeout: 5000`, below even the warm path, so it landed no block at all on a graph of any size. It now matches at 45s. `codegraph.ts` is left alone: it runs `witan-code inject-context`, which reads a local index in under a second. I have not measured the checkout path myself; the 51s figure is the repo's own recorded measurement, and issue #349 flags the Stop hook as an unverified hypothesis. Raising it costs nothing if the hypothesis is wrong. Closes #349 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WdyXu9F6St8F4E427gpxeP
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Concurrent calls on a fresh remote proxy can duplicate tool-schema discovery, undermining the intended cold-path optimization.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Optimizes Witan context injection while correcting cross-repository blocker handling.
Changes:
- Reuses scanned task rows to resolve blockers correctly.
- Runs remote context reads concurrently in two waves.
- Raises hook timeouts and adds regression coverage.
| File | Description |
|---|---|
witan/setup.py |
Raises Claude hook timeouts. |
witan/server.py |
Reuses task scans for blocker lookup. |
witan/readiness.py |
Supports wider blocker-row sets. |
witan/context.py |
Parallelizes remote context reads. |
witan/extensions/pi/workflow-context.ts |
Raises Pi timeout. |
tests/test_tasks.py |
Tests cross-repository blockers. |
tests/test_setup.py |
Tests revised timeouts. |
tests/test_readiness.py |
Tests widened blocker lookup. |
tests/test_context.py |
Tests concurrency and failure isolation. |
CHANGELOG.md |
Documents fixes. |
configs/pi/extensions/workflow-context.ts |
Updates reference Pi extension. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot review on #372. Concurrent workers each found the proxy's param-name cache unset, because none of them had finished listing yet, and so each resolved the deployment's tool surface itself. Counted against witan.ol.mit.edu on a cold proxy: four `tools/list` sequences for the four-call wave, where the sequential shape it replaced issued one. That is server load rather than latency, and the review's framing of it as undermining the cold-path win does not hold: the listings overlap, so they cost about one listing's wall time either way. But it is a 4x increase in schema discovery against a shared service, repeated by every agent session on every cold prompt, and this PR introduced it. `RemoteMCPProxy` gains `ensure_tool_schema()` and its sync form `prime_tool_schema()`, and the hook calls it before `_gather`. Verified against the deployment: 4 `tools/list` -> 1, concurrency retained. Not a lock inside `_invoke_once`, which is where it first looks like it belongs. The refresh spans an `await`, and this proxy is driven both from one loop per thread (the CLI and hook) and from several coroutines on a single shared loop (`witan.remote.serve`); a threading.Lock held across that await deadlocks the second case. Priming from the caller that knows it is about to fan out needs no cross-context locking. The feature check is on the CLASS, not the instance. `__getattr__` is a catch-all that turns any unknown attribute into a tool call, so an instance-level getattr would never return None and a witan-core predating the method would have `prime_tool_schema` invoked on the deployment as a tool. A class lookup is not intercepted, so the `witan-core>=0.37` floor stays honest: older cores skip priming and keep the previous behaviour. `just check-core-floor` passes unchanged. Also corrects the pi extension comment, which claimed 5s sat below the warm path. It did not: #349 measured warm cache hits at 0.6-0.9s. 5s cleared those and failed only the cold path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WdyXu9F6St8F4E427gpxeP
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.


What are the relevant tickets?
Closes #349.
Description (What does it do?)
witan inject-contextagainst a deployment took 16-23s cold, so Claude Code killed theUserPromptSubmithook at its 15s timeout and discarded the output. The user paid the full 15s and got nothing, and the agent ran with no knowledge of active projects, in-flight branch tasks orclaimed bymarkers.I reproduced it from this checkout against
witan.ol.mit.edu: 11.2s cold, against a budget the source documents as "~1-2s" per read.Two things account for it.
task_ready's repo-scoped branch scans every Task, then narrows to the candidates for this repo plus the unscoped ones. It built the blocker-status lookup from that narrowed set, so a blocker living in another repo was never in it and went back to the store as its ownget_task, for rows the scan had already returned. Timing the two against the deployment, on identical scans:task_list(repo=R)1.18s median,task_ready(repo=R)4.21s median. The ~3.0s difference is the per-blocker refetch.The hook then issued up to ten tool calls one at a time, so the cold path was their sum rather than the slowest of them. Most are independent, so it now runs two waves: projects, ready tasks, branch tasks and held tasks together, then sessions and comment threads, which need the first wave's answers.
The same narrowed lookup was a correctness bug on the local (non-deployed) path. There an unresolvable blocker counts as closed, so a task blocked by an open task in another repo was being advertised as ready to work, on the signal whose job is to keep two actors off the same work.
filter_readynow takes the wider row set, which the hook already had in hand.Both hook timeouts go up, because 15s sat inside the distribution of what they were timing instead of above it.
inject-contextgoes to 45s.session-checkpointgoes to 60s: it writes, and a write against a deployment has been measured at up to 51s (the credential-refresh note inwitan_core.remote.proxy._invoke). The piworkflow-contextextension shells out to the same command attimeout: 5000, below even the warm path, so it landed no block at all on a graph of any size; it now matches at 45s.Implementation details
task_readykeeps the candidate set and the blocker lookup as separate row sets.rowsis ordered last when buildingstatus_by_slug, so where a slug is in both, the candidate row decides. Theget_taskfallback stays for a blocker genuinely absent from the graph.serveris duck-typed on plain attribute-style methods (RemoteMCPProxy.__getattr__wraps each call in its ownasyncio.run), and every call is I/O-bound. This leaves the interface, and the test fakes, unchanged._gatherreturns each key's result or its exception, rather than raising. That is what preserves the per-read isolation the hook depends on (one failed read costs its own block and nothing else). It never raises, including when the pool cannot be built: a machine at its thread limit runs the calls in line, since_gatheris called outside the remote path's own try/except.held[:_HELD_TASK_LIMIT]slice_held_task_commentsuses, so nothing is read speculatively, andcomments_forre-raises a carried exception so that function keeps its own per-task isolation and debug line.RemoteMCPProxy.prime_tool_schema) before fanning out. Without it every worker finds the param-name cache unset and lists the surface itself, which measured 4tools/listcold against 1 for the sequential path. That is server load rather than latency (the listings overlap), but it is load every agent session repeats. It is feature-detected on the CLASS, not the instance, because__getattr__is a catch-all that would otherwise invokeprime_tool_schemaas a tool against a witan-core predating it; thewitan-core>=0.37floor is therefore unchanged and older cores simply skip priming.codegraph.tskeeps its 5s timeout: it runswitan-code inject-context, which reads a local index and was measured in witan inject-context: cold graph read takes ~20s, exceeding the 15s hook timeout it installs #349 at 0.78-1.25s cold.How can this be tested?
just test-witan-council: 1207 passed, 1 skipped.just test-witan-core: 643 passed, 3 skipped.just check-core-floorandjust check-versionspass.prekclean on the changed files.test_ready_resolves_a_cross_repo_blocker_without_re_reading_itbuilds a task blocked by one in another repo, counts the queriestask_readyissues, and asserts noget_taskamong them, in both directions, blocked and then released by closing the blocker.test_inject_context_remote_issues_its_independent_reads_concurrentlyholds every first-wave read on athreading.Barriersized to all four. A sequential implementation deadlocks on it and fails rather than quietly passing slower.test_inject_context_remote_one_failed_read_costs_only_its_own_blockandtest_gather_falls_back_to_serial_when_the_pool_cannot_startcover the isolation and the no-threads path.witan.ol.mit.edu, A/B back to back on the same graph, swapping only_gatherfor a sequential version. Before the schema-priming commit, sequential/concurrent medians over three sittings: 13.36/7.37, 13.19/7.61, 11.31/6.50. After it, which adds one round trip: 11.33/8.95, 14.23/9.05, 13.63/6.88. The absolute numbers move a lot with deployment load (concurrency feels it more than sequential does), so read the ratio, which holds at roughly 1.6-2x.tools/list, sequential 1, concurrent-primed 1.Additional Context
Only the client-side half is live on merge. The
task_readyfix runs on the server, so deployed users get that ~3.0s whenwitan.ol.mit.eduis redeployed. The 6.5s measured above is still carrying the unfixedtask_ready.A witan-core release is wanted but not required.
prime_tool_schemais new in witan-core's[Unreleased], and the class-level feature check means an external install on published 0.37.0 works and just skips priming. To actually get the 4-to-1 reduction there, witan-core needs a release and witan's floor raised to it. I have not bumped either here, because PR #370 is already releasing witan-core 0.37.1 and picking a number in both places would collide.I did not measure the
session-checkpointpath. #349 flags it as an unverified hypothesis and it stays one. The 51s figure is the repo's own recorded measurement, not mine. Raising that timeout costs nothing if the hypothesis is wrong.I left
_OUTPUT_CACHE_TTLat 30s, which #349 raises as an option. A longer TTL trades stalerclaimed bymarkers for fewer cold paths, and that is the wrong trade to make while the cold path is getting cheaper. Worth revisiting after the redeploy if it still bites.Interpreter startup is ~2.3s of every hook invocation, cache hit or not, and is untouched here. On the warm path that is now most of the cost.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WdyXu9F6St8F4E427gpxeP