feat: add agent mode output foundation#69
Merged
Conversation
|
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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (24)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (20)
📝 WalkthroughWalkthroughAdds a global --agent flag and OutputContext; introduces agent JSON envelopes and emit_* APIs; threads ctx through cli::run and command handlers; updates main to return ExitCode with agent-aware error emission; updates many commands to use emit_list/emit_one/emit_success and adds operation()/resource() helpers; expands tests for agent-mode JSON. ChangesAgent Output Mode
Sequence Diagram (high level): sequenceDiagram
participant User as CLI invocation
participant CLI as clap parsing & cli::run
participant Command as command.execute
participant Emit as emit_* functions
participant Serde as serde_json
participant Stdout as stdout
User->>CLI: coval ... --agent
CLI->>Command: command.execute(..., &OutputContext)
Command->>Emit: emit_one(ctx, resource, operation, data)
Emit->>Emit: check ctx.agent
alt agent
Emit->>Serde: serialize AgentEnvelope
Serde->>Stdout: print pretty JSON
else human
Emit->>Stdout: format/table output
end
Estimated code review effort: 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 docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli.rs (1)
158-217:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winBind the authenticated command once before the inner dispatch.
The outer
match cli.commandconsumescli.command, so the innermatch cli.commandin the fallback arm attempts to reuse a moved value and will not compile. Bind the fallback arm ascommand => { ... }and match on that binding instead.Suggested fix
match cli.command { Commands::Login(args) => commands::auth::login(args, ctx).await, Commands::Whoami => { commands::auth::whoami(api_key.as_ref(), ctx); Ok(()) } Commands::Config { command } => commands::config::execute(command, ctx), - _ => { + command => { let api_key = api_key.ok_or_else(|| { anyhow::anyhow!( "Not authenticated. Run `coval login` or set COVAL_API_KEY environment variable." ) })?; let client = CovalClient::new(api_key, api_url.as_deref()); - match cli.command { + match command { Commands::Agents { command } => { commands::agents::execute(command, &client, ctx).await }🤖 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/cli.rs` around lines 158 - 217, The fallback arm currently re-uses the moved value cli.command causing a compile error; change the outer match to bind the arm (e.g., use `command => { ... }`) and then perform the inner dispatch by matching on that binding (match command { Commands::Agents { command } => commands::agents::execute(...).await, ... }). Ensure you reference the same symbols (cli.command, Commands::Agents/Conversations/Runs/... and functions like commands::agents::execute) and construct the CovalClient before the inner match using the bound `command`.
🤖 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 `@src/commands/config.rs`:
- Around line 39-42: The current masking uses byte-index slicing on the API key
(see the closure mapping "api_key" => config.api_key.map(...) in config.rs and
the similar mapping in auth.rs) which can panic on multi-byte UTF-8 characters;
change both places to operate on Unicode scalar values by using character-based
operations: compute the character count, if >8 build the masked string by
collecting the first four chars (e.g., key.chars().take(4).collect()) and the
last four chars by taking from the end (e.g.,
key.chars().rev().take(4).collect() and reverse that result), otherwise return
the key as-is; keep the same mask format "{}...{}" and preserve the existing
closures/mapping structure (the unique symbols to edit are the "api_key" => ...
closure in config.rs and the analogous closure in auth.rs).
In `@src/commands/conversations.rs`:
- Around line 209-214: The None branch currently prints audio.audio_url directly
when ctx.agent is false; change it so only the human output path uses println!,
and all non-human/output-context flows use emit_one. Specifically, in the
conversations audio handler where you check ctx.agent, keep emit_one(ctx,
"conversations", operation, &audio) for agent=true, but for agent=false call
emit_one when OutputContext is non-human (i.e., when ctx.output_context
indicates JSON/machine output) and only call println!("{}", audio.audio_url)
when OutputContext is the human/default path; update the branch around
ctx.agent, operation, and audio to route JSON/non-human through emit_one instead
of printing a bare URL.
In `@src/commands/test_cases.rs`:
- Around line 165-174: The summary for bulk-create is being printed directly
with println! which bypasses the shared output layer; update the branch that
currently calls println! to emit the same summary through the existing helper
(emit_one) or only print when in human mode: replace the println! path with
emit_one(ctx, "test-cases", operation, &json!({ "created": created, "failed":
failed })) (matching the other branch) or wrap println! behind a check like if
ctx.human_mode() so machine-readable formats use the shared output; locate this
logic around the block that checks ctx.agent in the test-cases create handler.
---
Outside diff comments:
In `@src/cli.rs`:
- Around line 158-217: The fallback arm currently re-uses the moved value
cli.command causing a compile error; change the outer match to bind the arm
(e.g., use `command => { ... }`) and then perform the inner dispatch by matching
on that binding (match command { Commands::Agents { command } =>
commands::agents::execute(...).await, ... }). Ensure you reference the same
symbols (cli.command, Commands::Agents/Conversations/Runs/... and functions like
commands::agents::execute) and construct the CovalClient before the inner match
using the bound `command`.
🪄 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
Run ID: 119866e7-bfed-409f-bd4e-ba8e448ef601
📒 Files selected for processing (23)
src/cli.rssrc/client/models/conversation.rssrc/client/models/simulation.rssrc/commands/agents.rssrc/commands/api_keys.rssrc/commands/auth.rssrc/commands/config.rssrc/commands/conversations.rssrc/commands/dashboards.rssrc/commands/metrics.rssrc/commands/mutations.rssrc/commands/personas.rssrc/commands/review_annotations.rssrc/commands/review_projects.rssrc/commands/run_templates.rssrc/commands/runs.rssrc/commands/scheduled_runs.rssrc/commands/simulations.rssrc/commands/test_cases.rssrc/commands/test_sets.rssrc/main.rssrc/output.rstests/cli_tests.rs
lorenjphillips
force-pushed
the
feat/aci-1-agent-mode-foundation
branch
from
322dfe2 to
4ca561b
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 `@src/commands/dashboards.rs`:
- Around line 229-230: The widget handlers are emitting the wrong resource name
("dashboards") when calling emit_list; update those calls to emit the correct
resource "widgets" instead. Locate the emit_list invocations in dashboards.rs
that pass the literal "dashboards" (the occurrences noted around the widget
handlers) and change the first argument to "widgets" so structured output labels
the results correctly; ensure all similar calls (the other occurrences
referenced) are updated consistently.
🪄 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
Run ID: 2411302a-63e0-4a62-83d8-e1008816272b
📒 Files selected for processing (24)
src/cli.rssrc/client/models/conversation.rssrc/client/models/simulation.rssrc/commands/agents.rssrc/commands/api_keys.rssrc/commands/auth.rssrc/commands/config.rssrc/commands/conversations.rssrc/commands/dashboards.rssrc/commands/metrics.rssrc/commands/mutations.rssrc/commands/personas.rssrc/commands/review_annotations.rssrc/commands/review_projects.rssrc/commands/run_templates.rssrc/commands/runs.rssrc/commands/scheduled_runs.rssrc/commands/simulations.rssrc/commands/test_cases.rssrc/commands/test_sets.rssrc/config.rssrc/main.rssrc/output.rstests/cli_tests.rs
🚧 Files skipped from review as they are similar to previous changes (21)
- src/client/models/conversation.rs
- src/client/models/simulation.rs
- src/commands/personas.rs
- src/commands/config.rs
- src/commands/run_templates.rs
- src/commands/scheduled_runs.rs
- src/main.rs
- src/commands/conversations.rs
- src/commands/agents.rs
- tests/cli_tests.rs
- src/commands/review_projects.rs
- src/commands/test_cases.rs
- src/output.rs
- src/commands/mutations.rs
- src/commands/test_sets.rs
- src/commands/api_keys.rs
- src/commands/simulations.rs
- src/commands/runs.rs
- src/cli.rs
- src/commands/auth.rs
- src/commands/metrics.rs
lorenjphillips
force-pushed
the
feat/aci-1-agent-mode-foundation
branch
from
4ca561b to
539ab7c
Compare
lorenjphillips
force-pushed
the
feat/aci-1-agent-mode-foundation
branch
from
539ab7c to
68c64d9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli.rs (1)
158-217:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix the use-after-move error by binding the command in the fallback pattern.
The outer
match cli.commandconsumes the enum value. The fallback arm using_doesn't bind this value, so the innermatch cli.commandattempts to access an already-moved value. Change the fallback pattern tocommand =>and use that binding in the inner match instead of re-accessingcli.command.Suggested fix
match cli.command { Commands::Login(args) => commands::auth::login(args, ctx).await, Commands::Whoami => { commands::auth::whoami(api_key.as_ref(), ctx); Ok(()) } Commands::Config { command } => commands::config::execute(command, ctx), - _ => { + command => { let api_key = api_key.ok_or_else(|| { anyhow::anyhow!( "Not authenticated. Run `coval login` or set COVAL_API_KEY environment variable." ) })?; let client = CovalClient::new(api_key, api_url.as_deref()); - match cli.command { + match command { Commands::Agents { command } => { commands::agents::execute(command, &client, ctx).await }🤖 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/cli.rs` around lines 158 - 217, The outer match currently consumes cli.command and then the fallback arm re-reads cli.command, causing a use-after-move; change the fallback pattern from `_` to `command =>` to bind the moved value and then use that binding in the inner match (e.g., match command { Commands::Agents { command } => ..., Commands::Conversations { command } => ..., etc.) instead of referencing cli.command again; update the inner match to switch on the new local `command` binding so the enum value isn’t moved twice.
🤖 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 `@src/cli.rs`:
- Line 123: Commands::resource() currently returns "dashboards" for the
Self::Dashboards variant even when the inner subcommand is the widget handler,
causing execute_widget() success strings ("widgets") to disagree with error
envelopes; update Commands::resource() to special-case the Dashboards variant by
matching its nested command (e.g., DashboardCommands::Widgets) and return
"widgets" in that case, otherwise keep returning "dashboards" for other
Dashboard subcommands so success and error payload resource names match.
---
Outside diff comments:
In `@src/cli.rs`:
- Around line 158-217: The outer match currently consumes cli.command and then
the fallback arm re-reads cli.command, causing a use-after-move; change the
fallback pattern from `_` to `command =>` to bind the moved value and then use
that binding in the inner match (e.g., match command { Commands::Agents {
command } => ..., Commands::Conversations { command } => ..., etc.) instead of
referencing cli.command again; update the inner match to switch on the new local
`command` binding so the enum value isn’t moved twice.
🪄 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
Run ID: 0f8f7a7b-e074-4a18-b4a7-a0149e77671b
📒 Files selected for processing (24)
src/cli.rssrc/client/models/conversation.rssrc/client/models/simulation.rssrc/commands/agents.rssrc/commands/api_keys.rssrc/commands/auth.rssrc/commands/config.rssrc/commands/conversations.rssrc/commands/dashboards.rssrc/commands/metrics.rssrc/commands/mutations.rssrc/commands/personas.rssrc/commands/review_annotations.rssrc/commands/review_projects.rssrc/commands/run_templates.rssrc/commands/runs.rssrc/commands/scheduled_runs.rssrc/commands/simulations.rssrc/commands/test_cases.rssrc/commands/test_sets.rssrc/config.rssrc/main.rssrc/output.rstests/cli_tests.rs
🚧 Files skipped from review as they are similar to previous changes (20)
- src/client/models/simulation.rs
- src/client/models/conversation.rs
- src/config.rs
- src/commands/mutations.rs
- src/commands/scheduled_runs.rs
- src/commands/test_sets.rs
- src/commands/run_templates.rs
- src/main.rs
- src/commands/agents.rs
- src/commands/test_cases.rs
- src/commands/review_projects.rs
- src/commands/auth.rs
- src/commands/personas.rs
- src/commands/api_keys.rs
- src/commands/conversations.rs
- src/commands/config.rs
- tests/cli_tests.rs
- src/commands/runs.rs
- src/commands/metrics.rs
- src/output.rs
lorenjphillips
force-pushed
the
feat/aci-1-agent-mode-foundation
branch
from
68c64d9 to
68184a5
Compare
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
2 participants
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
--agentmode and a shared Open ACI-style envelope for success and error output.OutputContextwhile preserving current human/table and--format jsonbehavior.runs watchprogress output.Testing
cargo fmtcargo clippy --all-targets -- -D warningscargo testStacked PR 1 of 5. No version bump or release changes.
Summary by CodeRabbit
New Features
Behavior Changes
Tests