Skip to content

[COVI-10] Expose conversation metric outputs in the CLI#93

Open
callumreid wants to merge 2 commits into
mainfrom
callum/covi-10-failure-breakdown-cli
Open

[COVI-10] Expose conversation metric outputs in the CLI#93
callumreid wants to merge 2 commits into
mainfrom
callum/covi-10-failure-breakdown-cli

Conversation

@callumreid

@callumreid callumreid commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add --include-metric-outputs <METRIC_ID> to coval conversations list
  • map the option to the generic include=metric_outputs&metric_id=... API contract while preserving list filters, ordering, and page size
  • deserialize full metric outputs, including structured result.raw_values, into the conversation JSON response
  • remove the specialized failure-breakdown command and response models so aggregation semantics stay in consumers such as Covi

Example

coval --format json conversations list \
  --include-metric-outputs 6zhcrtiNmPrRsbXHoXEoqc \
  --page-size 50

Use normal list filters and pagination to retrieve additional pages.

Validation

  • cargo test: 97 CLI tests + 2 unit tests passed
  • focused wire-contract test passed
  • cargo clippy --all-targets --all-features -- -D warnings passed
  • cargo fmt --check passed

Dependency

  • coval-ai/backend#5678

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: f9a9be98-4e7b-48ef-9cee-a0986d7856fe

📥 Commits

Reviewing files that changed from the base of the PR and between f08539e and 8df5582.

📒 Files selected for processing (4)
  • src/client/mod.rs
  • src/client/models/conversation.rs
  • src/commands/conversations.rs
  • tests/cli_tests.rs

Walkthrough

Conversation listing now supports an optional metric ID that requests full metric outputs. The client adds the required query parameters, and the conversation model deserializes optional metric output data. The CLI conditionally uses the new client method while preserving standard listing behavior. A CLI test verifies pagination, query parameters, and nested metric output fields.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: exposing conversation metric outputs in the CLI.
Description check ✅ Passed The description is directly related to the changeset and covers the new CLI option, API mapping, and JSON output changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR exposes conversation metric outputs through the conversations list command. The main changes are:

  • Added --include-metric-outputs <METRIC_ID> to conversations list.
  • Mapped the option to include=metric_outputs and metric_id query parameters.
  • Added metric_outputs to conversation JSON responses.
  • Replaced the old failure-breakdown CLI path with the generic metric output flow.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Reviews (2): Last reviewed commit: "Expose conversation metric outputs in CL..." | Re-trigger Greptile

Comment thread src/client/models/conversation.rs Outdated
Comment on lines +180 to +183
pub total_scored_conversations: u64,
pub structured_result_conversations: u64,
pub critical_failure_conversations: u64,
pub non_critical_failure_conversations: u64,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Scope Counts Reject Sparse Responses

When the API returns an otherwise valid empty or partial scope with any count omitted or set to null, these required u64 fields make deserialization fail before the CLI can print the breakdown. Existing progress-style count models in this client default missing counters, so this new response shape is brittle for sparse aggregate results.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved by the architecture pivot: the specialized failure-breakdown response and all of its aggregate scope/example models were removed. The CLI now deserializes the existing Conversation plus optional full SimpleMetricOutput rows.

Comment thread src/client/models/conversation.rs Outdated
pub failure: String,
pub expected_bot_response: Option<String>,
pub message_index: Option<i64>,
pub occurred_at: DateTime<Utc>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Examples Require Captured Timestamps

When a returned example has failure text and expected response data but no captured occurrence timestamp, this required occurred_at field rejects the whole API response. That makes the command fail for a bounded examples payload where only one example is missing timing metadata.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved by removing the specialized aggregate response model. The generic conversation list expansion no longer introduces a required example timestamp field.

@callumreid callumreid marked this pull request as draft July 10, 2026 23:32
@callumreid callumreid changed the title [COVI-10] Add conversation failure breakdown CLI [COVI-10] Expose conversation metric outputs in the CLI Jul 11, 2026
@callumreid callumreid marked this pull request as ready for review July 11, 2026 00:08
@callumreid callumreid requested a review from borgesius July 11, 2026 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant