Add command dashboard cards#201
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughIntroduces ChangescommandCard rendering and view refactoring
Sequence DiagramssequenceDiagram
participant Caller
participant renderCommandCard
participant formatCommandCardSection
participant compactCommandCardList
Caller->>renderCommandCard: commandCard{title, summary, sections[], actions[]}
renderCommandCard->>renderCommandCard: compact title (fallback to "Zero")
renderCommandCard->>compactCommandCardList: summary items
compactCommandCardList-->>renderCommandCard: filtered items
loop for each section
renderCommandCard->>formatCommandCardSection: commandCardSection
formatCommandCardSection->>compactCommandCardList: fields + lines + rows
compactCommandCardList-->>formatCommandCardSection: filtered entries
formatCommandCardSection->>formatCommandCardSection: compute key column width
formatCommandCardSection-->>renderCommandCard: aligned section text
end
renderCommandCard->>compactCommandCardList: actions items
compactCommandCardList-->>renderCommandCard: filtered items
renderCommandCard-->>Caller: newline-separated card string
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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: 1
🧹 Nitpick comments (1)
internal/tui/command_output_test.go (1)
52-88: ⚡ Quick winAdd a redaction regression test for the command-card path.
This new test validates layout, but it doesn’t assert token-like redaction in
Summary,Fields, orActions. Adding one targeted case will protect/contextoutput from secret-leak regressions.Suggested test addition
+func TestFormatCommandCardRedactsTokenLikeText(t *testing.T) { + got := renderCommandCard(commandCard{ + Title: "Context", + Summary: []string{"sk-proj-sensitive-token"}, + Sections: []commandCardSection{{ + Title: "Runtime", + Fields: []commandField{ + {Key: "provider", Value: "openai"}, + {Key: "note", Value: "AIza1234567890abcdef"}, + }, + }}, + Actions: []string{"copy sk-ant-api03-abcdefghijklmnopqrstuvwxyz"}, + }) + for _, secret := range []string{ + "sk-proj-sensitive-token", + "AIza1234567890abcdef", + "sk-ant-api03-abcdefghijklmnopqrstuvwxyz", + } { + if strings.Contains(got, secret) { + t.Fatalf("expected token-like text to be redacted, got:\\n%s", got) + } + } +}🤖 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 `@internal/tui/command_output_test.go` around lines 52 - 88, Add a new test function (such as TestFormatCommandCardRedactsTokens) that creates a commandCard with token-like or secret-like patterns embedded in the Summary field values, Field values within the Sections, and Action strings. Call renderCommandCard with this test data and assert that the output properly redacts or masks these sensitive patterns rather than exposing them as-is. This regression test will ensure that future changes to the command card rendering do not inadvertently leak secrets in the /context output.
🤖 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 `@internal/tui/command_views.go`:
- Around line 475-482: The `style` field in the Fields slice is passed as
`m.responseStyle` without a fallback value, causing it to be skipped during
rendering when empty, which makes the Runtime section shape inconsistent. Modify
the `style` field entry to use the `displayValue` helper function (like the
other fields do) with an appropriate fallback value such as "none" to ensure the
field maintains a stable presence in the context output.
---
Nitpick comments:
In `@internal/tui/command_output_test.go`:
- Around line 52-88: Add a new test function (such as
TestFormatCommandCardRedactsTokens) that creates a commandCard with token-like
or secret-like patterns embedded in the Summary field values, Field values
within the Sections, and Action strings. Call renderCommandCard with this test
data and assert that the output properly redacts or masks these sensitive
patterns rather than exposing them as-is. This regression test will ensure that
future changes to the command card rendering do not inadvertently leak secrets
in the /context output.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 3c1ca95b-9703-4019-be10-b830dc5867f5
📒 Files selected for processing (6)
internal/tui/command_output.gointernal/tui/command_output_test.gointernal/tui/command_polish_test.gointernal/tui/command_views.gointernal/tui/model_test.gointernal/tui/session_controls_test.go
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 `@internal/tui/command_views.go`:
- Around line 17-20: The code unconditionally calls m.registry.All() at multiple
locations (around line 17 and around line 476) without checking if m.registry is
nil, which will cause a panic if the registry is uninitialized. Add a nil-safe
guard before each call to m.registry.All() that checks if m.registry is nil and
returns an empty slice as a fallback when it is, ensuring the file preserves
zero-value model behavior consistently across all code paths.
🪄 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: CHILL
Plan: Pro Plus
Run ID: a1e50fc3-49a2-4d21-8ce3-a51970275360
📒 Files selected for processing (3)
internal/tui/command_output_test.gointernal/tui/command_polish_test.gointernal/tui/command_views.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/tui/command_output_test.go
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 `@internal/tui/rendering.go`:
- Around line 444-446: The actions: label parsing in the rendering code uses
inconsistent variables. The condition checks `strings.HasPrefix(trimmed,
"actions:")` to detect the action label, but then uses `strings.TrimPrefix(line,
"actions:")` which operates on the original untrimed line. When the line has
leading whitespace like " actions: ...", TrimPrefix cannot remove the
"actions:" prefix from the original line since it starts with spaces, causing
duplication. Fix this by changing the TrimPrefix call in the same case statement
to use the `trimmed` variable instead of `line` so that both the prefix
detection and removal operate on the same trimmed version of the input.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 9b73bebc-4dcd-4df9-b9c8-95112bd7283e
📒 Files selected for processing (4)
internal/tui/command_output.gointernal/tui/command_views.gointernal/tui/rendering.gointernal/tui/rendering_lime_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/tui/command_views.go
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
Vasanthdev2004
changed the title
|
@coderabbitai fullreview |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get the PR diff and file contents
gh pr view 201 --repo Gitlawb/zero --json title,body,state,files 2>/dev/null | head -100
echo "=== PR DIFF ==="
gh pr diff 201 --repo Gitlawb/zero 2>/dev/nullLength of output: 23005
Review: Add command dashboard cardsThe overall design is clean — the card abstraction is well-isolated, the transcript sentinel prevents false positives, and the test coverage is solid (positive + negative assertions, sort-order verification, redaction). A few correctness issues need attention before merging. 🔴 Bug — wrong style when card has no summaryFile: case index == 0:
lines = append(lines, zeroTheme.ink.Bold(true).Render(line))
case isCommandCardHeading(trimmed):
lines = append(lines, zeroTheme.accent.Bold(true).Render(line))
Fix: check case index == 0 && !isCommandCardHeading(trimmed):
lines = append(lines, zeroTheme.ink.Bold(true).Render(line))
case isCommandCardHeading(trimmed):
lines = append(lines, zeroTheme.accent.Bold(true).Render(line))🟡 Fragile heuristic in
|
| Location | Note |
|---|---|
command_output.go formatCommandCardSection |
Key alignment uses len(key) (byte count). No issue for current ASCII keys; add a comment if Unicode keys are ever expected. |
command_views.go contextCompactionStatus |
Only maps "not compacted" → "idle"; all other statuses pass through. Fine for now, but worth a comment listing the expected input set so future contributors know what to expect. |
command_output.go renderCommandCard |
lines := []string{} → var lines []string to avoid a redundant heap allocation when the card is empty. |
✅ What's solid
compactCommandCardListcorrectly filters empty/whitespace strings — redaction tests verify this path.pluralizeCountis properly extracted and reusable.TestToolsCommandShowsFullSortedCatalogverifying positional order (not just presence) of tool names is exactly the right way to test sort behaviour.- Negative assertions (
assertNotContains) throughout the new tests catch regression to the old format reliably. - The
\x00sentinel being detected inrenderRowModeUncachedbefore the other system-row branches means there's no ambiguity with/sessionsor/mcprows.
The two issues that need fixes before merge are the heading style bug (wrong colour when no summary) and the dead guard + empty title edge case. The null-byte sentinel is the highest-risk follow-up.
2 participants
Summary
/contextand/toolsaway from rawstatus: ...command dumps/toolsoutput, and action-line renderingWhy
Several slash commands looked unchanged because they still flowed through the generic grey
rowSystemnote renderer. This PR makes the first command-card surfaces visibly product-like: titled card border, summary line, section headings, aligned fields, bullets, and actions.The scope stays intentionally small so it does not collide with active doctor, sandbox/permissions, or transcript polish PRs.
Validation
go test ./internal/tui -run 'TestCommandCardRowTrimsIndentedActionsLabel|TestCommandCardRowRendersAsTitledCard' -count=1go test ./internal/tui -run Tools -count=1go test ./internal/tui -count=1go test ./...go vet ./...go run ./cmd/zero-release buildgo run ./cmd/zero-release smokeSummary by CodeRabbit
/toolscommand display with organized card layout showing registry status and tool counts./contextcommand with improved information organization across Runtime, Session, and Tools sections.