Centralize agent logging behind scoped callbacks and remove artifact_root tramp data#23935
Centralize agent logging behind scoped callbacks and remove artifact_root tramp data#23935luisorofino wants to merge 6 commits into
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 0484d7a | Docs | Datadog PR Page | Give us feedback! |
14218e4 to
480793d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 480793d456
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| """Run a single agent turn with no tools available, firing scoped callbacks. | ||
| The caller is responsible for token accounting.""" | ||
| await self._callbacks.fire_before_agent_send(self._scope, prompt, 1) | ||
| response = await self._agent.send(prompt, allowed_tools=[]) |
There was a problem hiding this comment.
Report run_once send failures to scoped callbacks
When the single-turn path is used for the phase memory step, an agent.send() failure here propagates without firing fire_agent_error, unlike start(). In that scenario the run-wide AgentLogger never writes the error/failed-finish records for the phase, so the centralized agent log shows the last successful ReAct loop but omits the failing memory LLM call that aborted the phase; wrap this send in the same BaseException notification pattern used by start().
Useful? React with 👍 / 👎.
ae0dfdd to
98817d6
Compare
59daa4e to
7d1317c
Compare
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
98817d6 to
f828379
Compare
7d1317c to
e400aa6
Compare
f828379 to
41871ec
Compare
e400aa6 to
b5712e3
Compare
41871ec to
d2233cf
Compare
b5712e3 to
8f29699
Compare
d2233cf to
e07946e
Compare
ed8d868 to
c48dba7
Compare
e07946e to
baf39f2
Compare
f0fc48a to
de3d175
Compare
baf39f2 to
aa9a892
Compare
de3d175 to
8fba7d5
Compare
aa9a892 to
5e4cb70
Compare
8fba7d5 to
d7902e3
Compare
5e4cb70 to
9811e6c
Compare
d7902e3 to
0484d7a
Compare
9811e6c to
627eb4e
Compare
Validation ReportAll 21 validations passed. Show details
|

What does this PR do?
Reworks how the
ddev/aiagent framework does run logging, replacing path-threading with a single, identity-aware, callback-driven logger. No user-facing behavior changes — this is an internal architecture refactor of the agent runtime.Highlights:
AgentScope(agent/scope.py) — a small(owner_id, AgentRole)value carrying explicit agent identity, whereAgentRoleis one ofPHASE,SUBAGENT, orGOAL_REVIEWER. This identity now travels with each event instead of being reconstructed from a path.callbacks/callbacks.py) — agent-tier callbacks now carry anAgentScope. Addedon_agent_start, renamedon_complete→on_agent_finishandon_error→on_agent_error, and addedCallbacks.with_set(...)for composition.AgentLogger(runtime/agent_log.py) — one long-lived observer, created once by the orchestrator, that demultiplexes the callback stream into one append-only JSONL file per agent keyed byAgentScope. It is the only thing that knows the log root.ReActProcessFactory(react/factory.py) — binds the runtime builder and the run-wideCallbacksonce and exposescreate(scope, agent_config, system_prompt). It injects itself into the runtime it builds so a nestedspawn_subagentcan create properly-scoped children. Replaces the looseruntime_builder+callbacksprimitives that callers previously had to re-assemble.ReActProcess(react/process.py) — now requires a scope, fireson_agent_start, stamps every callback with its scope, and gainedrun_once(...)for single-turn calls so phases no longer hand-fire callbacks for memory steps.spawn_subagent— no more manual path/log orchestration; it just derives a childAgentScopeand asks the injected factory for a scoped child process.artifact_rooteverywhere and removed the old per-agenttools/agents/agent_logger.py.Motivation
The previous design had a few related smells:
artifact_roottramp data. A filesystem path was threaded through the orchestrator → resources → phases → agent build → tool registry →spawn_subagent, purely so each layer could reconstruct a per-agent logging directory. Most layers didn't use the path; they just passed it deeper. Classic tramp data.spawn_subagentorchestrated directories and log lines, and phases manually logged memory steps.runtime_builderandcallbacksas separate primitives and had to re-derive how to construct child processes, duplicating setup.This PR pushes identity into the event stream (
AgentScope) and centralizes logging behind callbacks, so the path lives in exactly one place (the orchestrator) and everything downstream is identity-aware and path-free.Notes / follow-ups
The code is intentionally not perfect yet — this is a step, not the destination.
Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged