Render metric lists as bulleted sections; ignore .worktrees/#32
Conversation
…o rendered` Pull secondary_metrics, guardrail_metrics, and exploratory_metrics out of the key/value table and emit each as its own `##` section with one bullet per metric, so the metric names stay readable instead of being squeezed into a single comma-separated table cell.
WalkthroughThe experiment summary table construction in the get command now separates the handling of metric-list fields ( Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 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/experiments/get.ts`:
- Around line 107-112: The metric-splitting logic in the deferredMetrics loop
(variables key, val, label, lines) uses val.split(', ') which fails for
different spacing and leaves empty items for trailing commas; replace that with
splitting on a regex that trims spaces around commas (e.g.
val.split(/\s*,\s*/)), then map each item to .trim() and filter out empty
strings (e.g. .filter(Boolean)) before pushing bullets so inputs like "a,b", "a,
b", or "a," produce correct non-empty metric lines.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a50cf011-0bc4-45ec-a140-c535112a4c5b
📒 Files selected for processing (1)
src/commands/experiments/get.ts
| for (const { key, val } of deferredMetrics) { | ||
| const label = key.replace(/_/g, ' '); | ||
| lines.push(`## ${label.charAt(0).toUpperCase() + label.slice(1)}`); | ||
| for (const metric of val.split(', ')) { | ||
| lines.push(`- ${metric}`); | ||
| } |
There was a problem hiding this comment.
Make metric splitting resilient to spacing/trailing separators.
At Line 110, split(', ') only handles a comma followed by exactly one space. Values like a,b or a, b won’t split correctly, and trailing commas can generate empty bullets.
Proposed fix
for (const { key, val } of deferredMetrics) {
const label = key.replace(/_/g, ' ');
lines.push(`## ${label.charAt(0).toUpperCase() + label.slice(1)}`);
- for (const metric of val.split(', ')) {
+ for (const metric of val.split(/\s*,\s*/).map((m) => m.trim()).filter(Boolean)) {
lines.push(`- ${metric}`);
}
lines.push('');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const { key, val } of deferredMetrics) { | |
| const label = key.replace(/_/g, ' '); | |
| lines.push(`## ${label.charAt(0).toUpperCase() + label.slice(1)}`); | |
| for (const metric of val.split(', ')) { | |
| lines.push(`- ${metric}`); | |
| } | |
| for (const { key, val } of deferredMetrics) { | |
| const label = key.replace(/_/g, ' '); | |
| lines.push(`## ${label.charAt(0).toUpperCase() + label.slice(1)}`); | |
| for (const metric of val.split(/\s*,\s*/).map((m) => m.trim()).filter(Boolean)) { | |
| lines.push(`- ${metric}`); | |
| } |
🤖 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/commands/experiments/get.ts` around lines 107 - 112, The metric-splitting
logic in the deferredMetrics loop (variables key, val, label, lines) uses
val.split(', ') which fails for different spacing and leaves empty items for
trailing commas; replace that with splitting on a regex that trims spaces around
commas (e.g. val.split(/\s*,\s*/)), then map each item to .trim() and filter out
empty strings (e.g. .filter(Boolean)) before pushing bullets so inputs like
"a,b", "a, b", or "a," produce correct non-empty metric lines.
1 participant
Summary
experiments get -o renderedsosecondary_metrics,guardrail_metrics, andexploratory_metricsare emitted as their own##sections with one bullet per metric instead of being squashed into a comma-separated table cell..worktrees/to.gitignore.Test plan
node dist/index.js experiments get <id-with-metrics> -o rendered— verify the metrics sections render as bullets and the key/value table no longer contains those rows.node dist/index.js experiments get <id-without-metrics> -o rendered— verify no empty metric sections appear.Summary by CodeRabbit
Release Notes