Repository navigation
[codex] Add agent run context - #91
Merged
Merged
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughAdds immutable runtime dataclasses ( ChangesRuntime context + propagation
Sequence DiagramsequenceDiagram
participant App as Application
participant Pipeline as Pipeline
participant Subagent as Subagent Tool
participant Child as Child Pipeline
App->>App: create_agent / create_child_pipeline(with overrides)
App->>App: build AgentRunContext(session_id, run_id, agent_id, context_budget, trace_metadata)
App->>Pipeline: create pipeline with PipelineContext.run_context
Pipeline->>Subagent: call subagent tool (invocation)
Subagent->>Subagent: derive parent_agent_id and child_agent_id
Subagent->>Subagent: merge trace_metadata (parent + ids)
Subagent->>Child: call child pipeline builder with agent_id_override, parent_run_id_override, context_budget, trace_metadata
Child->>Child: build child AgentRunContext (inherits session_id, sets parent_run_id)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
delete-cloud
marked this pull request as ready for review
May 2, 2026 08:37
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
coding-agent/src/agentkit/runtime/context.py (1)
55-79: 💤 Low valueMissing
run_idvalidation inderive_child.The method validates that
agent_idis non-empty but doesn't validaterun_id. SinceAgentRunContext.__post_init__validatesrun_idmust be non-empty (line 45-46), an emptyrun_idpassed toderive_childwould fail in the constructor anyway, but the error message would be less clear than a dedicated validation here.Consider adding explicit validation for consistency with
agent_id:🛡️ Suggested fix
def derive_child( self, *, run_id: str, agent_id: str, environment: Environment | None = None, context_budget: ContextBudget | None = None, trace_metadata: Mapping[str, Any] | None = None, ) -> AgentRunContext: + if not run_id: + raise ValueError("child run_id must be non-empty") if not agent_id: raise ValueError("child agent_id must be non-empty")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coding-agent/src/agentkit/runtime/context.py` around lines 55 - 79, derive_child currently checks agent_id but not run_id; add an explicit non-empty validation for the run_id parameter at the start of derive_child and raise ValueError("child run_id must be non-empty") if empty, mirroring the agent_id check and keeping behavior consistent with AgentRunContext.__post_init__; update derive_child (function name) to validate run_id before constructing AgentRunContext so error messages are clear and immediate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@coding-agent/src/agentkit/runtime/context.py`:
- Around line 55-79: derive_child currently checks agent_id but not run_id; add
an explicit non-empty validation for the run_id parameter at the start of
derive_child and raise ValueError("child run_id must be non-empty") if empty,
mirroring the agent_id check and keeping behavior consistent with
AgentRunContext.__post_init__; update derive_child (function name) to validate
run_id before constructing AgentRunContext so error messages are clear and
immediate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 46920492-fddb-4958-8d8a-7a03e5bfa49c
📒 Files selected for processing (8)
coding-agent/src/agentkit/runtime/__init__.pycoding-agent/src/agentkit/runtime/context.pycoding-agent/src/agentkit/runtime/pipeline.pycoding-agent/src/coding_agent/app.pycoding-agent/src/coding_agent/tools/subagent.pycoding-agent/tests/agentkit/runtime/test_run_context.pycoding-agent/tests/coding_agent/test_bootstrap.pycoding-agent/tests/coding_agent/tools/test_subagent.py
delete-cloud
force-pushed
the
codex-run-context-pr2
branch
from
May 2, 2026 08:51
c7de058 to
f50c711
Compare
delete-cloud
force-pushed
the
codex-run-context-pr2
branch
from
May 2, 2026 09:35
f50c711 to
755d257
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coding-agent/src/coding_agent/tools/subagent.py`:
- Around line 196-212: The child pipeline call drops the parent's ContextBudget
and any existing trace keys because it only forwards parent_run_id_override and
a fresh trace dict; fix by passing the parent's budget and merging existing
trace metadata into the child call: retrieve the parent budget from
__pipeline_ctx__.run_context (e.g. parent_budget =
__pipeline_ctx__.run_context.budget or similar) and pass it as budget_override
(or the parameter name expected by
child_pipeline_builder/create_child_pipeline), and merge any existing trace
metadata (e.g. parent_trace = getattr(__pipeline_ctx__, "trace_metadata", {}) or
__pipeline_ctx__.run_context.trace_metadata) with {"parent_agent_id":
parent_agent_id, "child_agent_id": child_agent_id} and pass that merged dict as
trace_metadata to child_pipeline_builder so trace keys and non-default budgeting
are preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 835a4871-ef46-49d1-9ad1-663a8e36a845
📒 Files selected for processing (9)
coding-agent/src/agentkit/runtime/__init__.pycoding-agent/src/agentkit/runtime/context.pycoding-agent/src/agentkit/runtime/pipeline.pycoding-agent/src/coding_agent/app.pycoding-agent/src/coding_agent/tools/subagent.pycoding-agent/tests/agentkit/runtime/test_run_context.pycoding-agent/tests/coding_agent/plugins/test_core_tools.pycoding-agent/tests/coding_agent/test_bootstrap.pycoding-agent/tests/coding_agent/tools/test_subagent.py
✅ Files skipped from review due to trivial changes (2)
- coding-agent/tests/coding_agent/tools/test_subagent.py
- coding-agent/src/agentkit/runtime/init.py
🚧 Files skipped from review as they are similar to previous changes (1)
- coding-agent/src/agentkit/runtime/context.py
CodeRabbit flagged that the child pipeline call dropped the parent's ContextBudget and any existing trace keys. Pass parent_run_context.context_budget through and merge parent trace_metadata with the parent/child agent_id keys so non-default budgeting and upstream trace correlation survive into subagent runs. Amp-Thread-ID: https://ampcode.com/threads/T-019de7f6-ebfd-742c-8223-6187e3784659 Co-authored-by: Amp <amp@ampcode.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coding-agent/src/coding_agent/tools/subagent.py`:
- Around line 97-100: The code currently does
str(pipeline_ctx.config.get("agent_id", "")) which turns None into "None";
change it to fetch agent_id = pipeline_ctx.config.get("agent_id") and treat None
as unset: if agent_id is None or agent_id == "" return None, otherwise return
str(agent_id); update the logic around legacy_agent_id (the variable created
there) so child IDs and trace metadata never receive the literal "None".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5a788025-8ef7-4c7e-93fb-2da868d93cee
📒 Files selected for processing (1)
coding-agent/src/coding_agent/tools/subagent.py
CodeRabbit flagged that str(config.get("agent_id", "")) coerced None
into the literal "None", which could leak into child IDs and trace
metadata. Inspect the raw value first and return None when it is missing
or empty.
Amp-Thread-ID: https://ampcode.com/threads/T-019de7f6-ebfd-742c-8223-6187e3784659
Co-authored-by: Amp <amp@ampcode.com>
Oracle review follow-up — closes the remaining gaps in PR #91: * fail fast on empty parent session_id in subagent dispatch and on empty session_id_override in create_child_pipeline; never silently mint a fresh uuid for the child run * prefer parent_run_context.session_id over the legacy PipelineContext.session_id slot * introduce coding_agent.agent_identity.effective_agent_id as the single resolution helper used by subagent dispatch, the metrics plugin, and the legacy config slot in app.py * namespace the reserved subagent trace keys (subagent.parent_agent_id / subagent.child_agent_id) so caller-supplied metadata cannot collide with them * tighten ContextBudget validation: reject bool token counts and reject reserved_output_tokens > max_output_tokens * document that AgentRunContext.trace_metadata is shallow-frozen and callers must pass JSON-scalar values * add focused regression tests for budget propagation, trace metadata merge, reserved-key collision, empty parent session_id, and the new ContextBudget validation * loosen two over-constrained tests (exact-field-set equality and a TypeError substring match) Amp-Thread-ID: https://ampcode.com/threads/T-019de7f6-ebfd-742c-8223-6187e3784659 Co-authored-by: Amp <amp@ampcode.com>
This was referenced May 2, 2026
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.
Summary
Verification
Summary by CodeRabbit
New Features
Tests