Repository navigation
fix(reborn): correlate run logs with thread_id/run_id for operator Logs panel - #4955
Conversation
…gs panel The operator Logs panel's scoped (thread/run) view showed "0 entries" during active runs: run-execution code emitted no tracing events carrying thread_id/run_id, and OperatorLogLayer reads those correlation fields from the enclosing span, so scoped queries matched nothing. Instrument both run-executor paths with a thread_id+run_id span plus an INFO anchor event so every run yields at least one correlated entry: - ironclaw_reborn::turn_runner::execute_claimed_run (serve/local-dev path) - ironclaw_host_runtime::turn_scheduler executor task (scheduler path) Verified via `ironclaw-reborn serve`: sending a chat message now produces a thread/run-correlated "turn run started" entry in the Logs panel, which was empty before.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughTracing spans keyed by ChangesPer-run tracing instrumentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
github-actions
Bot
added
size: M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces tracing instrumentation to turn runs in both turn_scheduler.rs and turn_runner.rs by tagging events with thread_id and run_id to populate the operator Logs panel. Feedback on the changes suggests avoiding info! logging in REPL/TUI-reachable code like turn_runner.rs to prevent interface corruption, recommending the use of debug! logging instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@crates/ironclaw_host_runtime/src/turn_scheduler.rs`:
- Around line 433-436: The issue is that tracing::info! is being used for run
lifecycle anchors in background execution paths, which violates the REPL/TUI
logging invariant (info! and warn! corrupt the terminal UI). Fix this across
three locations: in crates/ironclaw_host_runtime/src/turn_scheduler.rs lines
433-436, change the tracing::info! call for the "turn run started" message to
tracing::debug!; in crates/ironclaw_host_runtime/src/turn_scheduler.rs line 488,
change the run-finish anchor tracing::info! call to tracing::debug!; and in
crates/ironclaw_reborn/src/turn_runner.rs line 381, change the run-start anchor
tracing::info! call to tracing::debug!. Background tasks and internal
diagnostics must use debug! level logging to preserve correlation via span
fields without corrupting the terminal UI.
In `@crates/ironclaw_reborn/src/turn_runner.rs`:
- Around line 378-381: The tracing::info! call logging "turn run started"
violates the repo logging invariant for background tasks. Replace the
tracing::info! call with tracing::debug! to use the appropriate diagnostic level
for internal background task logging, maintaining correlation through span
fields rather than INFO-level diagnostics.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e2362396-531e-47fe-897d-71678c52797c
📒 Files selected for processing (2)
crates/ironclaw_host_runtime/src/turn_scheduler.rscrates/ironclaw_reborn/src/turn_runner.rs
zmanian
left a comment
zmanian
left a comment
There was a problem hiding this comment.
Correct, well-targeted fix — the root cause and the field names are right. Requesting changes on one hard blocker (missing test, which CI already enforces) plus one rule-conflict to confirm.
What's good
- Root cause is right.
OperatorLogLayercorrelates by reading exactlythread_id/run_idfrom spans viafrom_root()(operator_logs.rs:171-174, 302-304). The new span fields use those exact names, so the fix lands where the layer actually looks. - The two paths are genuinely distinct, so no double-instrumentation:
turn_schedulerdrives theTurnRunExecutortrait (execute_claimed_run(claimed, transitions)), whileturn_runner's isTurnRunnerWorker::execute_claimed_run(claimed, cancel)— different signatures, not nested. skip_allis the right call — only the two correlation fields are recorded, soself/claimed/cancelaren't captured as span fields (no secret/bloat leak).- The INFO anchor is a sound workaround for the
info-levelEnvFilter(adebug!anchor would be filtered before capture, leaving the panel empty).
Blocking
- No regression test — and the
Regression test enforcementcheck is failing because of it. This is afix:with no test, which the repo gate rejects. A test would also lock the bug: installOperatorLogLayer, drive a run through the span, and assert a thread/run-scoped query returns ≥1 entry. Must-fix to merge.
Please confirm
info!in background run-executor tasks vs. the repo rule.CLAUDE.md: "Background tasks must NEVER useinfo!— it breaks the interactive REPL/TUI." Theinfo!here is deliberate (to survive theinfofilter), which is fine if these Reborn executors never run under the v1 Ratatui TUI subscriber (theservepath is an HTTP server, whereinfo!is appropriate). Please confirm that — and if so, add a one-line note in the code so the next reader doesn't trip on the rule. If these can run under the TUI, the two anchor events per run will corrupt it and a different approach is needed (e.g. capture below the global filter rather than emittinginfo!).
Minor / polish
- Span-name inconsistency:
turn_schedulernames the span"turn_run";turn_runner's#[instrument]defaults to the fn name"execute_claimed_run". Addname = "turn_run"to theturn_runnerinstrument for a consistent operator-facing span name across both paths. - Anchor asymmetry:
turn_scheduleremits both "turn run started" and "turn run finished";turn_runneremits only "started". Harmless, but a "finished" anchor would be symmetric.
serrrfirat
left a comment
serrrfirat
left a comment
There was a problem hiding this comment.
Multi-agent review for nearai/ironclaw#4955 at 9ce590199f9f96c2951074c1470de7bf88953998.
Summary:
- Security: 0 findings
- Bugs: 0 findings
- Performance/Concurrency: 0 findings
- Tests: 2 findings
- Conventions: 1 finding
Event: COMMENT. No Critical/High findings were found.
Retained findings:
- Medium tests: Reborn executor log correlation is untested.
- Medium tests: Scheduler executor log correlation is untested.
- Low conventions: Scheduler-only INFO finish log adds unnecessary operator/terminal noise.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@crates/ironclaw_host_runtime/src/turn_scheduler.rs`:
- Line 433: The tracing::debug! call for "turn run started" at line 433 is
emitted at DEBUG level, which means it gets filtered out when the default
EnvFilter is set to INFO. This prevents the operator correlation logic from
capturing the per-run anchor event, resulting in zero-row results for thread/run
scoped queries. Change the tracing::debug! call to tracing::info! so the anchor
event remains visible and can be captured for operator log correlation at the
default INFO log level.
In `@crates/ironclaw_reborn/src/turn_runner.rs`:
- Line 379: The tracing::debug! call for "turn run started" is filtered out by
the default EnvFilter (which passes only INFO and above) before it can reach
OperatorLogLayer, preventing the run-start anchor from being recorded in the
operator log buffer and causing scoped queries to show 0 entries in production.
Change the tracing::debug! macro to tracing::info! so the event passes through
the filter and reaches OperatorLogLayer, allowing the run-start anchor to
populate the operator log buffer correctly.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 13238bc3-a6ca-40bb-8e90-e1953af463af
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (6)
crates/ironclaw_host_runtime/Cargo.tomlcrates/ironclaw_host_runtime/src/turn_scheduler.rscrates/ironclaw_host_runtime/tests/turn_scheduler_contract.rscrates/ironclaw_reborn/Cargo.tomlcrates/ironclaw_reborn/src/turn_runner.rscrates/ironclaw_reborn/tests/loop_driver_host.rs
|
Human final-review guidance for current head CI is green, GitHub reports the PR as mergeable, and all review threads are resolved. The remaining blocker is human review state. Please focus final review on:
I would not spend time re-reviewing unrelated tracing or scheduler behavior outside those anchor/correlation paths unless something in the above looks wrong. |
…debug Addresses review: run lifecycle anchors must be debug! (info!/warn! corrupt the REPL/TUI per the logging invariant). But debug! anchors were being dropped by the info-level capture filter, leaving the Logs panel empty again. Decouple terminal safety from Logs-panel visibility with per-layer filters in init_tracing: the stderr/fmt layer stays at info (terminal-safe), while OperatorLogLayer captures ironclaw run-path crates at debug. Both run anchors move to debug!. Regression tests capture at DEBUG to mirror the operator filter. Verified via `ironclaw-reborn serve`: stderr no longer prints the anchors, and the scoped Logs panel shows the run's correlated entries (anchors plus the model-gateway/loop-exit debug steps), which previously did not appear.
|
Code review update for current head I reviewed the new commit
No actionable code findings from this pass. The split tracing filters preserve the terminal invariant by keeping stderr at info while allowing CI note: Not ready for human final review yet: CI is still in progress/red. |
|
Human final-review guidance for current head Current status:
Suggested human review focus:
I did not find additional actionable code issues on the current head. |
zmanian
left a comment
zmanian
left a comment
There was a problem hiding this comment.
Approving — the fix is correctly diagnosed and well-architected. The per-layer filter split cleanly decouples terminal (REPL/TUI) safety from Logs-panel visibility, and the thread_id/run_id span fields match exactly what OperatorLogLayer correlates on via from_root. Tests drive both call sites end-to-end with the right runtime flavor.
A few minor, non-blocking items posted inline (addressing them or consciously deferring is fine):
- The ~115-line tracing-capture test harness is duplicated byte-for-byte across two crates.
- The new
IRONCLAW_REBORN_OPERATOR_LOGenv var and theIRONCLAW_REBORN_LOGbehavior change should be documented in.env.example. operator_filtercaptures alldebug!from the wholeironclaw_reborncrate (broader than just run anchors) — fine given the bounded ring buffer, just flagging the volume/eviction tradeoff.
| // populated, while those `debug!` events are NOT written to stderr. This is | ||
| // a *separate* per-layer filter, so terminal safety and Logs-panel | ||
| // visibility are decoupled. Override via IRONCLAW_REBORN_OPERATOR_LOG. | ||
| let operator_filter = |
There was a problem hiding this comment.
Two config nits worth documenting in .env.example:
- New var undocumented.
IRONCLAW_REBORN_OPERATOR_LOGis a new knob — please add it to.env.examplealongsideIRONCLAW_REBORN_LOG. - Silent behavior change. Previously
IRONCLAW_REBORN_LOGalso governedOperatorLogLayerverbosity; now it only controls the stderr layer. Anyone who setIRONCLAW_REBORN_LOG=debugto enrich the Logs panel will silently lose that and must switch toIRONCLAW_REBORN_OPERATOR_LOG. Worth a note in the PR body and.env.example.
Also minor: this sets ironclaw_reborn=debug for the entire crate, so every debug! in it lands in the operator buffer, not just run-lifecycle anchors. The buffer is a bounded ring (operator_logs.rs pop_front at capacity), so no leak — but under heavy debug volume run anchors can be evicted faster than expected. Non-blocking; flagging the tradeoff.
| events: Arc<Mutex<Vec<CapturedEvent>>>, | ||
| } | ||
|
|
||
| struct CorrelatedEventLayer { |
There was a problem hiding this comment.
This entire tracing-capture harness (CapturedEvent, CorrelatedEventCapture, CorrelatedEventLayer, CaptureVisitor, CapturedSpanFields, and the Layer/Visit impls — ~115 lines) is duplicated byte-for-byte in crates/ironclaw_reborn/tests/loop_driver_host.rs. Dev-only so not blocking, but a fix to one capture bug won't propagate. Consider extracting into a shared test-support location (ironclaw_common / ironclaw_loop_support both exist as candidate homes, behind a test-only feature), or at minimum add a // keep in sync with loop_driver_host.rs note.
| events: Arc<Mutex<Vec<CapturedEvent>>>, | ||
| } | ||
|
|
||
| struct CorrelatedEventLayer { |
There was a problem hiding this comment.
Duplicate of the capture harness in crates/ironclaw_host_runtime/tests/turn_scheduler_contract.rs (see note there). Extracting to a shared test-support module would keep the two in sync.
|
@claude review |
Code Review PR #4955SummaryThis PR adds tracing instrumentation to correlate operator run logs with their thread/run IDs, enabling scoped log filtering in the operator Logs panel. The changes properly separate stderr (info level) from operator logs (debug level) to prevent debug output from corrupting the REPL/TUI. FindingsCode Quality[MEDIUM:75] Test code duplication across two test files The
Recommendation: Extract these test utilities into a shared test module to reduce duplication and maintenance burden. Architecture & CLAUDE.md Compliance✅ Logging levels correct: Uses Security & Safety✅ No security vulnerabilities: No command injection, SSRF, or credential leaks Performance & Production✅ No performance regressions: Minimal overhead from span creation Result: No blocking issues foundThe code is production-ready with a recommendation to refactor test helper duplication in a follow-up PR. |
…gs panel (nearai#4955) * fix(reborn): correlate run logs with thread_id/run_id for operator Logs panel The operator Logs panel's scoped (thread/run) view showed "0 entries" during active runs: run-execution code emitted no tracing events carrying thread_id/run_id, and OperatorLogLayer reads those correlation fields from the enclosing span, so scoped queries matched nothing. Instrument both run-executor paths with a thread_id+run_id span plus an INFO anchor event so every run yields at least one correlated entry: - ironclaw_reborn::turn_runner::execute_claimed_run (serve/local-dev path) - ironclaw_host_runtime::turn_scheduler executor task (scheduler path) Verified via `ironclaw-reborn serve`: sending a chat message now produces a thread/run-correlated "turn run started" entry in the Logs panel, which was empty before. * fix(reborn): keep run log anchors debug scoped * fix(reborn): keep run log anchors visible at info * fix(reborn): keep run log anchors at debug; capture operator logs at debug Addresses review: run lifecycle anchors must be debug! (info!/warn! corrupt the REPL/TUI per the logging invariant). But debug! anchors were being dropped by the info-level capture filter, leaving the Logs panel empty again. Decouple terminal safety from Logs-panel visibility with per-layer filters in init_tracing: the stderr/fmt layer stays at info (terminal-safe), while OperatorLogLayer captures ironclaw run-path crates at debug. Both run anchors move to debug!. Regression tests capture at DEBUG to mirror the operator filter. Verified via `ironclaw-reborn serve`: stderr no longer prints the anchors, and the scoped Logs panel shows the run's correlated entries (anchors plus the model-gateway/loop-exit debug steps), which previously did not appear. --------- Co-authored-by: Yuting Wang <yuting.wang@near.ai> Co-authored-by: think-in-universe <46699230+think-in-universe@users.noreply.github.com>
4 participants
Problem
The operator Logs panel's scoped (thread/run) view shows "0 entries" during active runs. Run-execution code emits no tracing events carrying
thread_id/run_id, andOperatorLogLayercorrelates entries by reading those fields from the enclosing span — so scoped queries match nothing.Fix
Instrument both run-executor paths with a
thread_id+run_idspan plus an INFO anchor event, so every run yields at least one correlated entry:ironclaw_reborn::turn_runner::execute_claimed_run— the serve/local-dev path the standalone binary actually usesironclaw_host_runtime::turn_schedulerexecutor task — the scheduler-backed path other deployments useVerification
Built locally, ran
ironclaw-reborn serve --port 3030, sent a chat message:INFO execute_claimed_run{thread_id=… run_id=…}: turn run startedNote
Default
EnvFilterisinfo, so DEBUG-level driver/tool logs are still filtered before capture; surfacing those is a separate filter change. This PR fixes the "scoped logs are always empty" bug.