Repository navigation
perf(opencode): resolve instruction files once per turn - #28
PierrotAWB wants to merge 1 commit into
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/session/prompt.ts">
<violation number="1" location="packages/opencode/src/session/prompt.ts:1104">
P2: When `loop()` is called for an already-finished session, this fetch runs before the early-exit check and can wait 5 seconds for a configured URL. It also runs serially before history and tool preparation, whereas the previous call overlapped the first-step context effects. Move resolution after the no-op check and overlap it with first-step preparation while retaining the result for later steps.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| // Per turn, not per step: an instruction file edited by the turn itself applies next turn. | ||
| const instructionsStart = Date.now() | ||
| const instructions = yield* instruction.system().pipe(Effect.orDie) |
There was a problem hiding this comment.
P2: When loop() is called for an already-finished session, this fetch runs before the early-exit check and can wait 5 seconds for a configured URL. It also runs serially before history and tool preparation, whereas the previous call overlapped the first-step context effects. Move resolution after the no-op check and overlap it with first-step preparation while retaining the result for later steps.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/session/prompt.ts, line 1104:
<comment>When `loop()` is called for an already-finished session, this fetch runs before the early-exit check and can wait 5 seconds for a configured URL. It also runs serially before history and tool preparation, whereas the previous call overlapped the first-step context effects. Move resolution after the no-op check and overlap it with first-step preparation while retaining the result for later steps.</comment>
<file context>
@@ -1099,6 +1099,11 @@ const layer = Layer.effect(
+ // Per turn, not per step: an instruction file edited by the turn itself applies next turn.
+ const instructionsStart = Date.now()
+ const instructions = yield* instruction.system().pipe(Effect.orDie)
+ contextMs += Date.now() - instructionsStart
+
</file context>
|
Consolidated into #29 (one commit per concern there; review history for the prep_ms retry semantics is on this PR's threads). |
Issue for this PR
Issues are disabled on this fork; motivation below. Stacked on #27 (top of the per-step-prep stack #23 → #25 → #26 → #27); re-target after it merges.
Type of change
What does this PR do?
instruction.system()ran inside the step loop: a findUp walk for AGENTS.md/CLAUDE.md/CONTEXT.md plus reads of every match — and an HTTP GET with a 5s timeout for any URL instruction — repeated on every step of every turn. Instruction files cannot change mid-turn except by the turn's own edits.The resolve now happens once, before the loop. Deliberate behavior change, documented in-line with a one-line comment: an instruction file edited by the turn itself applies on the next turn (matching how skills and config already behave). Its cost stays visible in
context_ms(#25).How did you verify your code works?
prompt.test.ts60/60; fullpackages/opencodesuite over the whole stack (#23+#25+#26+#27+this): 3129 pass, failures limited to the pre-existingTruncatedate bug (#24) and one pty load flake that passes in isolation.bun run typecheckclean,bun run lint0 errors. No new test: pinning "mid-turn instruction edits are deferred" would freeze a deliberate simplification as if it were a contract.Checklist
🤖 Generated with Claude Code
Summary by cubic
Resolves instruction files once per turn instead of once per step to remove redundant I/O and network calls while keeping their cost visible in context_ms. Previously, instructions were re-resolved on every step; now they are precomputed before the loop, so instruction edits during a turn apply on the next turn.
instruction.system()is called once before the loop with timing and removed from the per-stepEffect.all; verifycontextMsaccumulation and that precomputed instructions are used throughoutpackages/opencode/src/session/prompt.ts. No migration needed.Written for commit 11e0d51. Summary will update on new commits.