Repository navigation
feat(agent): opt interactive hosted runs into the 1h prompt-cache TTL (RFC 0001) - #3437
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughHosted runtime instruction builders now accept optional prompt-cache TTL values. Interactive chat paths use a one-hour TTL. Structured cache handling limits Anthropic requests to four retained breakpoints and normalizes their TTL values. ChangesInteractive prompt caching
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The change enables a one-hour prompt-cache TTL for interactive hosted runs while preserving the five-minute default for other runs; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant ChatRuntime
participant InteractiveBuilder
participant CallContext
participant AnthropicRequestBuilder
ChatRuntime->>InteractiveBuilder: Build instructions with cacheTtl: "1h"
InteractiveBuilder->>CallContext: Create structured cached messages
CallContext->>AnthropicRequestBuilder: Provide provider cache metadata
AnthropicRequestBuilder->>AnthropicRequestBuilder: Retain four breakpoints and normalize TTLs
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/agent/hosted/cloud-runtime-system-messages.test.ts (1)
175-181: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the repository BDD test API.
The three new tests use
Deno.test. Usedescribe()andit()from#veryfront/testing/bdd.ts. Keep the existing assertions from#veryfront/testing/assert.ts.As per coding guidelines,
*.test.tsfiles must usedescribe()andit()from#veryfront/testing/bdd.ts.Also applies to: 183-192, 194-209
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agent/hosted/cloud-runtime-system-messages.test.ts` around lines 175 - 181, Replace the three Deno.test cases around createVeryfrontCloudRuntimeSystemMessages with describe() and it() from `#veryfront/testing/bdd.ts`, while preserving the existing assertions imported from `#veryfront/testing/assert.ts`.Source: Coding guidelines
src/agent/hosted/cloud-runtime-system-messages.ts (2)
47-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the TTL documentation with all interactive call sites.
The options documentation lists root chat and steering refresh, but
src/agent/hosted/default-chat-runtime.ts:211also uses"1h"for tool-assembly re-rendering. State the general interactive-run rule or include this third path.As per coding guidelines, exported behavior documentation must remain aligned with the implementation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agent/hosted/cloud-runtime-system-messages.ts` around lines 47 - 55, Update the documentation for BuildVeryfrontCloudRuntimeInstructionsOptions.cacheTtl to include tool-assembly re-rendering via default-chat-runtime, or describe the broader rule that all interactive runs may use "1h"; retain the existing distinction from one-shot child/eval runs using the default "5m".Source: Coding guidelines
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the internal alias for this cross-module import.
This import crosses from
src/agent/hostedintosrc/agent/runtime. Replace the relative path with#veryfront/agent/runtime/call-context.ts.As per coding guidelines, internal TypeScript imports must use
#veryfront/*; relative imports are forcli/or same-directory siblings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agent/hosted/cloud-runtime-system-messages.ts` at line 5, Update the AgentCallCacheTtl import in cloud-runtime-system-messages.ts to use the `#veryfront/agent/runtime/call-context.ts` internal alias instead of the relative path, while leaving the imported type unchanged.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/agent/hosted/cloud-runtime-system-messages.test.ts`:
- Around line 175-181: Replace the three Deno.test cases around
createVeryfrontCloudRuntimeSystemMessages with describe() and it() from
`#veryfront/testing/bdd.ts`, while preserving the existing assertions imported
from `#veryfront/testing/assert.ts`.
In `@src/agent/hosted/cloud-runtime-system-messages.ts`:
- Around line 47-55: Update the documentation for
BuildVeryfrontCloudRuntimeInstructionsOptions.cacheTtl to include tool-assembly
re-rendering via default-chat-runtime, or describe the broader rule that all
interactive runs may use "1h"; retain the existing distinction from one-shot
child/eval runs using the default "5m".
- Line 5: Update the AgentCallCacheTtl import in
cloud-runtime-system-messages.ts to use the
`#veryfront/agent/runtime/call-context.ts` internal alias instead of the relative
path, while leaving the imported type unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1af196c4-4f44-4033-a0f5-06c7e9408271
📒 Files selected for processing (4)
src/agent/hosted/cloud-agent-chat-execution.tssrc/agent/hosted/cloud-runtime-system-messages.test.tssrc/agent/hosted/cloud-runtime-system-messages.tssrc/agent/hosted/default-chat-runtime.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1839f82522
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1839f82 to
dff0b23
Compare
|
Rebased onto the exact current #3435 head ( Local verification on exact head
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
dff0b23 to
03fd08f
Compare
|
Rebased again onto the updated exact #3435 head ( Exact-head verification on
|
Review resultNo actionable findings on commit The interactive helper consistently applies the one-hour TTL, and the root chat, steering-refresh, and tool-assembly re-render paths all use that helper. Structured system messages are also retained when constructing the runtime configuration, preserving provider cache metadata through dispatch. src/agent/hosted/cloud-runtime-system-messages.tsL43-L75 src/agent/hosted/cloud-agent-chat-execution.tsL123-L129 src/agent/hosted/cloud-agent-chat-execution.tsL343-L345 src/agent/hosted/default-chat-runtime.tsL197-L211 src/agent/hosted/default-chat-runtime.tsL285-L289 No code changes were needed, so no commit or follow-up pull request was created. Testing
|
03fd08f to
2d70da8
Compare
|
Rebased onto #3435 exact head 90af0fe. Range-diff confirms both #3437 commits are patch-identical. Verification on 2d70da8: 105 focused tests passed (174 steps), focused checks and full repository typecheck passed, test-type baseline held, lint passed, module and extension contracts passed, generated API references are current, public docs validation passed, formatting passed, and git diff --check passed. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Concurrent PR head changes already hardened the production clone and accessor paths for tools. Add the missing message/content regressions so the request-builder suite covers the same adversarial shapes for nested message content arrays and cache-bearing blocks. Constraint: PR #3437 must stay draft and discussion_r3790224234 plus discussion_r3790235077 must remain unresolved for root inspection. Rejected: Reapply local production helper changes | remote dd8f098 already fixed production behavior, and duplicating it would overwrite concurrent work. Confidence: high Scope-risk: narrow Directive: Keep Anthropic cache hardening tests paired across provider tools and message content whenever cache budgeting is changed. Tested: Red-first local regressions showed iterator spreads dropped message/content entries and sibling accessors serialized five cache controls on vulnerable code; pinned Deno 2.7.7 request-builder suite; full Anthropic extension suite; touched static checks; docs checks. Not-tested: Full pre-push remains blocked by pre-existing lint:cli-boundary CLI violations outside this change.
Raw provider records can contain values that JSON serialization coerces after cache breakpoints are counted. Reject boxed primitive records before coercion and use captured reflection intrinsics for every cache-sensitive lookup so tenant mutations cannot hide emitted breakpoints. Constraint: Cache budgeting must match native JSON serialization without invoking tenant-controlled hooks. Rejected: Serialize the raw request before budgeting | serialization would execute the hooks the guard must avoid. Confidence: high Scope-risk: narrow Directive: Keep cache-sensitive inspection on captured intrinsics and reject new serialization hooks before counting breakpoints. Tested: Anthropic request-builder 78 steps, full extension 231 steps, targeted format lint and typecheck. Not-tested: Live Anthropic transport serialization.
|
@codex review exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 968e231cd4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f8b5b3203
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1452f67e22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review exact head |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21279f9a6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Implements the RFC 0001 TTL-gating decision: interactive hosted runs opt into the one-hour prompt-cache TTL so the shared static prefix can stay warm across chat idle gaps.
This PR is stacked on #3435 and includes its exact final dependency head.
What changes
cacheTtlthrough cloud runtime system-message construction. The default remains"5m"."1h"policy in one builder used by root chat execution, steering refresh, and tool-assembly re-rendering.invoke_agentruns on the"5m"default because they do not expect a second cache read.The policy remains at the interactive call boundary because that boundary knows whether repeat reads are expected.
Verification
42cc97e0084c7580d98cd738c4d2bdc14204a06b: 104 tests, 239 steps, 0 failures.docs:api-reference:check,docs:public:check,docs:validate,typecheck, andgit diff --checkall exit 0.docs:validatestill reports the existinggetting-started/index.mdwebhook index warning.42cc97e0084c7580d98cd738c4d2bdc14204a06b:extensions/ext-llm-anthropic src/agentpassed 1,187 tests and 2,213 steps with 0 failures.src/cache/backend.test.ts --filter ApiCacheBackend --trace-leaks. The same 10ApiCacheBackendtests fail on this branch and pristine currentorigin/main(d6bf543771bf4eecbab34452a4f23c5c53c2388a), each with 12 passed, 10 failed, 47 filtered out, and request timeout / abort evidence. This is treated as a reproducible baseline/infrastructure class, not branch-caused.src/providerwas not used as green evidence because it hit unrelated live-provider 401s in env-backed OpenAI / Veryfront Cloud tests.Remaining external evidence
The repository has no configured live-provider/VCR harness for provider-reported cache creation and cache-read token counters. Structural request-builder coverage verifies the exact request shape required for a cache hit; live provider telemetry is not claimed here.
Related: RFC 0001 (#1788), #3433, and #3435.
Summary by CodeRabbit
Improvements
Documentation
Final head verification
Current exact head after the dependency stack refresh:
21279f9a6cd4627e436434d6ebfc1dddcf541ee2.git diff --checkpassed on the exact head.Remaining external evidence
The repository has no configured live-provider/VCR harness for provider-reported cache creation and cache-read token counters. Structural request-builder coverage verifies the exact request shape required for a cache hit; live provider telemetry is not claimed here.