feat(reborn): export caller-scoped QA run artifacts - #6344
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
github-actions
Bot
added
scope: docs
📝 WalkthroughWalkthroughAdds caller-scoped ChangesRun artifact export and unified view routing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant WebChatV2
participant RebornServices
participant OperatorLogStore
WebChatV2->>RebornServices: query RUN_ARTIFACT_VIEW with thread_id and run_id
RebornServices->>OperatorLogStore: query bounded scoped logs
OperatorLogStore-->>RebornServices: logs or unavailable status
RebornServices-->>WebChatV2: redacted run artifact
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
⏭️ IronLoop Review Declined: reviewer
Review at a glance
| Disposition | Head |
|---|---|
| ⏭️ Review declined | 64400c36859a |
Head: 64400c36859aff7a15e30257aab30765a7e03933
Reason: The diff changes 2,802 files (215,749 additions and 422,063 deletions), including broad architecture, CI, dependency, and test-suite rewrites unrelated to the stated feature.
Next: Split or provide a focused comparison containing only the run-artifact feature and its required tests; then rerun review against that focused base/head range.
Run details
Status: Current
Trustworthy review produced: no
Summary
Skipped: the supplied base-to-head comparison is a mega-diff, far broader than the stated run-artifact feature, and cannot be reviewed reliably as one change within the available scope.
![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 a feature to export caller-owned, redacted run evidence bundles (under the ironclaw.run_artifact.v1 schema) containing messages, tool calls, and bounded process-local logs, along with a Python script to convert these artifacts into LLM trace candidates. It also removes the channel-connect command resolution from the chat interface. The review feedback highlights a critical issue where removing tokio::spawn in the Slack delivery hook makes external network I/O synchronous, which could block the main request flow. Additionally, the feedback suggests a performance optimization in redact_json to avoid redundant cloning, and robustness improvements in the Python import script to handle case-insensitive message kinds.
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.
| match self.current_driver().await { | ||
| Ok(Some(driver)) => driver.on_trigger_submitted(fire, run_id, scope).await, | ||
| Ok(None) => { | ||
| tracing::debug!( | ||
| %run_id, | ||
| "Slack dynamic triggered-run delivery skipped: Slack setup is not configured" | ||
| ); | ||
| } | ||
| }); | ||
| Err(error) => { | ||
| tracing::warn!( | ||
| %run_id, | ||
| %error, | ||
| "Slack dynamic triggered-run delivery skipped: delivery hook unavailable" | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Removing tokio::spawn here causes the Slack delivery hook to run synchronously within the main request/turn submission flow. Since Slack delivery involves external network I/O, this can introduce significant latency spikes or block the user-facing thread if the Slack API is slow, rate-limited, or experiencing downtime. It is highly recommended to keep this operation asynchronous by running it in a background task.
let hook = self.clone();
tokio::spawn(async move {
match hook.current_driver().await {
Ok(Some(driver)) => driver.on_trigger_submitted(fire, run_id, scope).await,
Ok(None) => {
tracing::debug!(
%run_id,
"Slack dynamic triggered-run delivery skipped: Slack setup is not configured"
);
}
Err(error) => {
tracing::warn!(
%run_id,
%error,
"Slack dynamic triggered-run delivery skipped: delivery hook unavailable"
);
}
}
});There was a problem hiding this comment.
Reviewed; no code change: OBSOLETE — the referenced Slack runtime file is absent from the current PR head.\n
| fn redact_json( | ||
| redactor: &DeterministicTraceRedactor, | ||
| input: &serde_json::Value, | ||
| ) -> (serde_json::Value, bool) { | ||
| let keyed = redact_sensitive_json(input); | ||
| let redacted = match keyed { | ||
| serde_json::Value::String(value) => { | ||
| serde_json::Value::String(redactor.redact_text(&value).0) | ||
| } | ||
| serde_json::Value::Array(values) => serde_json::Value::Array( | ||
| values | ||
| .iter() | ||
| .map(|value| redact_json(redactor, value).0) | ||
| .collect(), | ||
| ), | ||
| serde_json::Value::Object(values) => serde_json::Value::Object( | ||
| values | ||
| .iter() | ||
| .map(|(key, value)| (key.clone(), redact_json(redactor, value).0)) | ||
| .collect(), | ||
| ), | ||
| value => value, | ||
| }; | ||
| let changed = redacted != *input; | ||
| (redacted, changed) | ||
| } |
There was a problem hiding this comment.
The current implementation of redact_json recursively calls redact_sensitive_json on every single node of the JSON tree. Since redact_sensitive_json is only useful for JSON Objects (to redact sensitive keys), calling it on strings, arrays, numbers, booleans, and nulls is highly inefficient and causes a massive amount of unnecessary cloning and memory allocations. We can optimize this by only calling redact_sensitive_json when the input is a serde_json::Value::Object.
fn redact_json(
redactor: &DeterministicTraceRedactor,
input: &serde_json::Value,
) -> (serde_json::Value, bool) {
let redacted = match input {
serde_json::Value::String(value) => {
serde_json::Value::String(redactor.redact_text(value).0)
}
serde_json::Value::Array(values) => serde_json::Value::Array(
values
.iter()
.map(|value| redact_json(redactor, value).0)
.collect(),
),
serde_json::Value::Object(_) => {
let keyed = redact_sensitive_json(input);
if let serde_json::Value::Object(keyed_values) = keyed {
serde_json::Value::Object(
keyed_values
.into_iter()
.map(|(key, value)| (key, redact_json(redactor, &value).0))
.collect(),
)
} else {
keyed
}
}
value => value.clone(),
};
let changed = redacted != *input;
(redacted, changed)
}There was a problem hiding this comment.
Addressed in b3bb9ef: redaction now performs structural sensitive-key redaction once, then redacts text while consuming that result.\nVerification: product-workflow run-artifact tests and clippy passed.\n
|
|
||
| def trace_candidate(artifact: dict[str, Any], model_override: str | None) -> dict[str, Any]: | ||
| messages = sorted(artifact["messages"], key=lambda item: item.get("sequence", 0)) | ||
| user = next((item for item in messages if item.get("kind") == "user"), None) |
There was a problem hiding this comment.
The kind field of a message is serialized from the Rust MessageKind enum. Depending on the serialization settings, it might be serialized as CamelCase (e.g., "User") rather than lowercase. To ensure robustness and prevent the import script from failing with a ValueError when processing CamelCase values, perform a case-insensitive comparison by converting the value to lowercase.
| user = next((item for item in messages if item.get("kind") == "user"), None) | |
| user = next((item for item in messages if str(item.get("kind") or "").lower() == "user"), None) |
There was a problem hiding this comment.
Reviewed; no code change: INCORRECT — MessageKind uses snake_case serialization, so exported artifacts use user.\n
| ( | ||
| item | ||
| for item in reversed(messages) | ||
| if item.get("kind") == "assistant" and str(item.get("content", "")).strip() |
There was a problem hiding this comment.
Perform a case-insensitive comparison for the kind field here as well to ensure robustness against CamelCase serialization of the MessageKind enum.
| if item.get("kind") == "assistant" and str(item.get("content", "")).strip() | |
| if str(item.get("kind") or "").lower() == "assistant" and str(item.get("content", "")).strip() |
There was a problem hiding this comment.
Reviewed; no code change: INCORRECT — MessageKind uses snake_case serialization, so exported artifacts use assistant.\n
|
🚅 Deployed to the ironclaw-pr-6344 environment in ironclaw-ci-preview
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChat-send.test.mjs (1)
2651-2654: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest name promises "without fetching connectable channels" but the fixture doesn't enforce it. Every other prompt test proves the negative with a throwing stub (e.g. throw new Error("ordinary prompts should not fetch connectable channels")); here
fetchQueryhappily runsqueryFn, so a regression that re-introduced a connectable-channel fetch would pass silently.♻️ Make the guarantee load-bearing
queryClient: { - fetchQuery: async ({ queryFn }) => queryFn(), + fetchQuery: async () => { + throw new Error("slash connect prompts should not fetch connectable channels"); + }, invalidateQueries: () => {}, },As per coding guidelines: "Comments and documentation that promise guarantees must match the code and tests."
🤖 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 `@crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChat-send.test.mjs` around lines 2651 - 2654, Update the queryClient fixture in the test for prompts without fetching connectable channels so fetchQuery throws an error if invoked, rather than executing queryFn. Preserve invalidateQueries as a no-op and match the throwing-stub pattern used by the other prompt tests, making any connectable-channel fetch regression fail.Source: Coding guidelines
🤖 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_webui_v2_static/src/assets.rs`:
- Around line 261-265: Add a negative assertion in
chat_omits_connect_action_while_extensions_render_slack_setup_ui for the served
dist/app.js bundle, verifying it excludes ChannelConnectCard,
channelConnectAction, and dismissChannelConnectAction as appropriate. Use the
existing asset_text helper and preserve the source-module assertions.
In `@crates/ironclaw_webui_v2/src/handlers/run_artifact.rs`:
- Around line 1-5: Update the handlers in run_artifact.rs to extract
WebUiV2Capabilities through Axum’s Extension extractor, importing the capability
type from its existing module. Add this required extension to each affected
handler’s parameters so requests missing it are rejected rather than processed.
In `@scripts/import-reborn-run-artifact.py`:
- Around line 74-94: Widen the exception handling fence in main() around
trace_candidate so malformed nested artifact data raising KeyError or TypeError
follows the existing stderr error and return-code-2 path. Preserve the current
ValueError handling and avoid changing trace_candidate’s field access behavior.
- Around line 73-119: Update the step-building logic in the
tool_groups/assistant flow so pending_results from the final tool-call group
cannot be silently discarded when assistant is None. Either attach those
unconsumed results to the last step or raise ValueError for tool calls without a
trailing assistant reply, while preserving the existing no-replayable-message
guard behavior.
- Line 13: Centralize the run-artifact schema and redaction-pipeline constants
in the owning run_artifact module, exposing the pipeline identifier as a public
constant alongside RUN_ARTIFACT_SCHEMA. Update
scripts/import-reborn-run-artifact.py and the webui_v2_handlers_contract tests
to consume the shared/generated contract values instead of hardcoding either
literal, preserving validation against the same contract across languages.
In `@scripts/test-import-reborn-run-artifact.py`:
- Around line 15-39: Extend
test_groups_parallel_calls_and_attaches_results_to_next_step with coverage for
two sequential provider tool-call groups, asserting each group’s
expected_tool_results attaches to the following step correctly. Add a separate
case where the artifact ends immediately after a tool-call step without a
finalized assistant message, and assert the expected trailing-results behavior
from trace_candidate.
---
Outside diff comments:
In
`@crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChat-send.test.mjs`:
- Around line 2651-2654: Update the queryClient fixture in the test for prompts
without fetching connectable channels so fetchQuery throws an error if invoked,
rather than executing queryFn. Preserve invalidateQueries as a no-op and match
the throwing-stub pattern used by the other prompt tests, making any
connectable-channel fetch regression fail.
🪄 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: 04736fa3-f93a-4bef-8706-fd0a5ac39331
⛔ Files ignored due to path filters (1)
tests/fixtures/llm_traces/reborn_qa/README.mdis excluded by!tests/fixtures/**
📒 Files selected for processing (28)
FEATURE_PARITY.mdcrates/ironclaw_product_workflow/src/lib.rscrates/ironclaw_product_workflow/src/reborn_services.rscrates/ironclaw_product_workflow/src/reborn_services/run_artifact.rscrates/ironclaw_product_workflow/tests/reborn_services_contract.rscrates/ironclaw_reborn_composition/src/slack_host_beta/runtime_setup.rscrates/ironclaw_webui_v2/CLAUDE.mdcrates/ironclaw_webui_v2/src/descriptors.rscrates/ironclaw_webui_v2/src/handlers.rscrates/ironclaw_webui_v2/src/handlers/run_artifact.rscrates/ironclaw_webui_v2/src/lib.rscrates/ironclaw_webui_v2/src/router.rscrates/ironclaw_webui_v2/tests/webui_v2_descriptors_contract.rscrates/ironclaw_webui_v2/tests/webui_v2_handlers_contract.rscrates/ironclaw_webui_v2_static/src/assets.rscrates/ironclaw_webui_v2_static/static/js/lib/api.jscrates/ironclaw_webui_v2_static/static/js/lib/channel-connect.jscrates/ironclaw_webui_v2_static/static/js/lib/channel-connect.test.mjscrates/ironclaw_webui_v2_static/static/js/pages/chat/chat.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/channel-connect-card.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/channel-connect-card.test.mjscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-bubble.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/components/message-bubble.test.mjscrates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useChat.jscrates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjscrates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useChat-send.test.mjsscripts/import-reborn-run-artifact.pyscripts/test-import-reborn-run-artifact.py
💤 Files with no reviewable changes (6)
- crates/ironclaw_webui_v2_static/static/js/pages/chat/components/channel-connect-card.test.mjs
- crates/ironclaw_webui_v2_static/static/js/pages/chat/components/channel-connect-card.js
- crates/ironclaw_webui_v2_static/static/js/lib/channel-connect.test.mjs
- crates/ironclaw_webui_v2_static/static/js/lib/channel-connect.js
- crates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useChat.js
- crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/chat.test.mjs
| use axum::Json; | ||
| use axum::extract::{Extension, Path, State}; | ||
| use ironclaw_product_workflow::{ | ||
| RebornRunArtifact, RebornRunArtifactRequest, WebUiAuthenticatedCaller, | ||
| }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail closed on missing WebUiV2Capabilities.
As per path instructions for WebUI v2, handlers must fail closed when the WebUiV2Capabilities extension is missing. Extracting the extension ensures Axum enforces its presence and rejects the request if it is omitted.
🔒️ Proposed fix to enforce capabilities
use ironclaw_product_workflow::{
- RebornRunArtifact, RebornRunArtifactRequest, WebUiAuthenticatedCaller,
+ RebornRunArtifact, RebornRunArtifactRequest, WebUiAuthenticatedCaller, WebUiV2Capabilities,
};
use serde::Deserialize;
// ...
pub async fn get_run_artifact(
State(state): State<WebUiV2State>,
Extension(caller): Extension<WebUiAuthenticatedCaller>,
+ Extension(_capabilities): Extension<WebUiV2Capabilities>,
Path(path): Path<RunArtifactPath>,
) -> Result<Json<RebornRunArtifact>, WebUiV2HttpError> {Also applies to: 18-22
🤖 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 `@crates/ironclaw_webui_v2/src/handlers/run_artifact.rs` around lines 1 - 5,
Update the handlers in run_artifact.rs to extract WebUiV2Capabilities through
Axum’s Extension extractor, importing the capability type from its existing
module. Add this required extension to each affected handler’s parameters so
requests missing it are rejected rather than processed.
Source: Path instructions
There was a problem hiding this comment.
Reviewed; no code change: OBSOLETE — this refers to the former WebUI v2 crate path; the current caller-authorized read route is in ironclaw_webui.\n
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== Candidate run-artifact handlers =='
fd -a 'run_artifact\.rs$' crates || true
printf '%s\n' '== Route registrations and handler references =='
rg -n -C 4 --glob '*.rs' \
'(get_run_artifact|run-artifact|run_artifact|WebUiV2Capabilities|WebUiAuthenticatedCaller)' \
crates/ironclaw_webui crates/ironclaw_webui_v2 2>/dev/null || true
printf '%s\n' '== Handler outlines =='
for file in $(fd -a 'run_artifact\.rs$' crates); do
ast-grep outline "$file" --items all --type function --view expanded || true
doneLength of output: 201073
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== Active run-artifact handler =='
cat -n crates/ironclaw_webui/src/webui_v2/handlers/run_artifact.rs
printf '%s\n' '== Active route registration and extension layers =='
rg -n -C 8 --glob '*.rs' \
'get_run_artifact|run-artifact|run_artifact|Extension\(.*WebUiV2Capabilities|WebUiV2Capabilities' \
crates/ironclaw_webui/src/webui_v2/router.rs \
crates/ironclaw_webui/src/webui_v2 \
| head -n 400
printf '%s\n' '== Focused run-artifact contract test =='
sed -n '1920,1985p' crates/ironclaw_webui/tests/webui_v2_handlers_contract.rsLength of output: 40034
@serrrfirat, agreed the original crates/ironclaw_webui_v2 path is obsolete. However, the underlying finding still applies to the migrated route: crates/ironclaw_webui/src/webui_v2/handlers/run_artifact.rs Line 19 extracts WebUiAuthenticatedCaller but not WebUiV2Capabilities.
The route is registered at crates/ironclaw_webui/src/webui_v2/router.rs Lines 193-196, and the current contract test supplies the capability extension, so it does not cover the missing-extension boundary. Per the WebUI v2 fail-closed invariant, this handler should require the extension as well. I’m retaining the finding with the corrected path.
✏️ Learnings added
Learnt from: serrrfirat
Repo: nearai/ironclaw PR: 6344
File: crates/ironclaw_webui_v2/src/handlers/run_artifact.rs:0-0
Timestamp: 2026-07-21T08:19:09.687Z
Learning: The active caller-authorized WebChat v2 run-artifact route is `crates/ironclaw_webui/src/webui_v2/handlers/run_artifact.rs::get_run_artifact`; the former `crates/ironclaw_webui_v2` crate path is obsolete.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
64400c3 to
9ae7af5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@FEATURE_PARITY.md`:
- Line 334: Update the lower-priority trajectory-export entry near the existing
status listing to match the 🚧 parity status on the Trajectory export row, or
clarify its scope so it does not contradict the documented WebChat v2
capability. Keep the documentation aligned with the implemented and tested
guarantees, including the remaining follow-up limitations.
🪄 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: a15782eb-10d4-42bc-bab2-15909b90c1d9
📒 Files selected for processing (2)
FEATURE_PARITY.mdcrates/ironclaw_architecture/tests/reborn_facade_method_freeze_ratchet.rs
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 86.37% — 320226 / 370760 lines Per-crate breakdown (65 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
9ae7af5 to
f2319dd
Compare
serrrfirat
changed the title
f2319dd to
9ae7af5
Compare
serrrfirat
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_webui/src/webui_v2/handlers.rs`:
- Around line 1989-2008: Extract the repeated view-query dispatch from
query_operator_logs and query_logs into a shared async query_view helper. Have
it serialize the request, construct RebornViewQuery with the supplied view_id
and cursor, call state.services().query, and deserialize page.payload; update
both handlers to delegate to it while preserving their existing request and
response types.
🪄 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: 77242222-d0d2-4aa2-bd3c-7afce5e4d25a
📒 Files selected for processing (7)
crates/ironclaw_architecture/tests/reborn_facade_method_freeze_ratchet.rscrates/ironclaw_product_workflow/src/lib.rscrates/ironclaw_product_workflow/src/reborn_services.rscrates/ironclaw_product_workflow/src/reborn_services/log_views.rscrates/ironclaw_product_workflow/tests/reborn_services_contract.rscrates/ironclaw_webui/src/webui_v2/handlers.rscrates/ironclaw_webui/tests/webui_v2_handlers_contract.rs
| Query(mut query): Query<RebornOperatorLogsQuery>, | ||
| ) -> Result<Json<RebornOperatorCommandPlaneResponse>, WebUiV2HttpError> { | ||
| require_operator_webui_config(capabilities)?; | ||
| let response = state.services().query_operator_logs(caller, query).await?; | ||
| let cursor = query.cursor.take(); | ||
| let params = serde_json::to_value(query).map_err(RebornServicesError::internal_from)?; | ||
| let page = state | ||
| .services() | ||
| .query( | ||
| caller, | ||
| RebornViewQuery { | ||
| view_id: OPERATOR_LOGS_VIEW.id.to_string(), | ||
| params, | ||
| cursor, | ||
| }, | ||
| ) | ||
| .await?; | ||
| let response = | ||
| serde_json::from_value(page.payload).map_err(RebornServicesError::internal_from)?; | ||
| Ok(Json(response)) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract the repeated view-query boilerplate.
query_operator_logs and query_logs both now repeat the same "take cursor → serialize params → build RebornViewQuery → .query() → deserialize page.payload" sequence, differing only in view id and request/response types. This pattern will keep growing as more views (e.g. run-artifact elsewhere in the stack) route through the generic conduit.
♻️ Proposed helper to de-duplicate the view-query dispatch
async fn query_view<Req: Serialize, Resp: DeserializeOwned>(
state: &WebUiV2State,
caller: WebUiAuthenticatedCaller,
view_id: &str,
request: Req,
cursor: Option<String>,
) -> Result<Resp, WebUiV2HttpError> {
let params = serde_json::to_value(request).map_err(RebornServicesError::internal_from)?;
let page = state
.services()
.query(
caller,
RebornViewQuery {
view_id: view_id.to_string(),
params,
cursor,
},
)
.await?;
serde_json::from_value(page.payload).map_err(RebornServicesError::internal_from)
}- let cursor = query.cursor.take();
- let params = serde_json::to_value(query).map_err(RebornServicesError::internal_from)?;
- let page = state
- .services()
- .query(
- caller,
- RebornViewQuery {
- view_id: OPERATOR_LOGS_VIEW.id.to_string(),
- params,
- cursor,
- },
- )
- .await?;
- let response =
- serde_json::from_value(page.payload).map_err(RebornServicesError::internal_from)?;
+ let cursor = query.cursor.take();
+ let response = query_view(&state, caller, OPERATOR_LOGS_VIEW.id, query, cursor).await?;
Ok(Json(response))Also applies to: 2014-2051
🤖 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 `@crates/ironclaw_webui/src/webui_v2/handlers.rs` around lines 1989 - 2008,
Extract the repeated view-query dispatch from query_operator_logs and query_logs
into a shared async query_view helper. Have it serialize the request, construct
RebornViewQuery with the supplied view_id and cursor, call
state.services().query, and deserialize page.payload; update both handlers to
delegate to it while preserving their existing request and response types.
Ninth fold, additive: the run-artifact export/import facade methods, log query views, and the WebUI run-artifact route land verbatim; conflicts were export/import list reflows resolved to this tree's surface (retired connectable-channels rail stays retired) plus main's new identifiers, and both contract suites keep both sides' tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Summary
ironclaw.run_artifact.v1queryread conduit plus typed run/log view descriptorsquery_logsandquery_operator_logs, shrinking the frozen facade from 89 methods onmainto 88Security and deployment model
Thread and run IDs are locators only. Tenant, agent, project, and user authority come from the authenticated WebUI caller. Cross-user access returns an indistinguishable 404 before logs are queried.
Logs use the existing scoped operator-log service. They are bounded and process-local, so the artifact reports
logs.complete=falseand explicitly reports unavailable/truncated state. This stays provider-neutral and does not couple the product to Railway or trace-commons.Provider signatures and reasoning are omitted. Message content, tool arguments, and log messages pass through the deterministic trace redactor; sensitive JSON keys use the canonical structured redactor.
QA workflow
scripts/import-reborn-run-artifact.pyon the JSON.Stacked follow-up
PR #6346 adds complete multi-run thread export on top of this run-only vertical slice. This PR can be tested and merged independently first.
Verification
cargo test -p ironclaw_architecture --test reborn_facade_method_freeze_ratchetcargo test -p ironclaw_product_workflow --test reborn_services_contract query_logscargo test -p ironclaw_product_workflow --test reborn_services_contract query_operator_logscargo test -p ironclaw_product_workflow --test reborn_services_contract run_artifactcargo test -p ironclaw_webui --test webui_v2_handlers_contract logscargo test -p ironclaw_webui --test webui_v2_handlers_contract run_artifactcargo test -p ironclaw_webui --test webui_v2_descriptors_contractcargo clippy -p ironclaw_product_workflow -p ironclaw_webui --all-targets -- -D warningspnpm typecheckpnpm vitest run src/pages/chat/components/message-bubble.test.tspython3 scripts/test-import-reborn-run-artifact.pyscripts/ci/check-reborn-qa-fixtures.sh