Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances basecamp show to include related comments by default (when the API exposes a comment count), improves how comments/attachments/mentions render in styled + markdown output, and documents the new CLI behavior.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Fetch and embed comments by default in
basecamp show(with--no-commentsto opt out), and surface comment counts + a breadcrumb tobasecamp comments list. - Render comments and attachment metadata as dedicated sections in styled/markdown output (instead of dumping raw fields).
- Improve richtext mention-to-markdown conversion for figure-style mention attachments, with new test coverage.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
skills/basecamp/SKILL.md |
Updates Basecamp skill docs to reflect default comment inclusion and the new opt-out flag. |
internal/commands/show.go |
Implements comment fetching logic, summary/breadcrumb updates, and notice handling for failures. |
internal/commands/show_test.go |
Adds comprehensive tests for comment fetching behavior, opt-out flag, and rendering expectations. |
internal/output/render.go |
Adds structured rendering for top-level comments and attachment sections in styled/markdown output. |
internal/output/output_test.go |
Adds tests verifying attachment sections render cleanly (no raw map dumps). |
internal/richtext/richtext.go |
Improves mention rendering by extracting display names from figure-style mention attachments. |
internal/richtext/richtext_test.go |
Adds regression test for figure mention rendering. |
.surface |
Registers the basecamp show --no-comments flag for surfaced CLI metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
1 issue found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/commands/show_test.go">
<violation number="1" location="internal/commands/show_test.go:440">
P3: JSON-output tests added here rely on substring checks of raw output; decode stdout and assert fields structurally to avoid brittle or false-positive assertions.
(Based on your team's feedback about using structured JSON assertions instead of substring checks.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/commands/show_test.go">
<violation number="1" location="internal/commands/show_test.go:340">
P2: Parse/decode the JSON data into a map or struct and assert key presence and values via structured lookups rather than using string containment checks, which are brittle.
(Based on your team's feedback about validating JSON output via structured lookups.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Fixed — updated the JSON-output tests to decode the response envelope/data and assert title/comments structurally instead of relying on substring checks (issues identified by cubic). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Fixed — capped { |
Review carefully before merging. Consider a major version bump. |
|
Fixed — capped basecamp show to 100 embedded comments by default, added --all-comments for full discussions, and now surface a truncation notice when more comments are available. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What
basecamp showwhen the recording exposes a comment count--all-commentsto opt into the full discussion--no-commentsas the opt-out path, and make--no-comments/--all-commentsmutually exclusivebasecamp comments list --alland, when truncated,basecamp show --all-commentscomments_countandcomment_countso supported record types like columns also fetch comments correctlyCloses #385
Why
basecamp showalready gives a good single-item view, but it still required a second command to see the discussion on that item. Making comments part of the default output turnsshowinto a more complete detail view.Capping the default embed at 100 comments keeps that default view practical for large threads, avoiding unexpectedly expensive multi-page fetches and huge output while still making the full discussion available via
--all-comments. The truncation notice and follow-up breadcrumb make that behavior explicit.The
comment_countfallback fixes a gap discovered during review: some supported record types do not usecomments_count, so without the fallback they would silently miss comment fetching.Testing
bin/cipassesSummary by cubic
Fetch and render comments by default in
basecamp show, capped at 100. Add clear notices, breadcrumbs, and polished Styled/Markdown output for comments and attachments; JSON includes a top‑level comments array when fetched.New Features
--all-commentsfetches all,--no-commentsskips.basecamp show --all-comments <id>; summary shows “(n comments)” and adds a breadcrumb tobasecamp comments list --all <id>.comment_countcovers more record types; improved figure-style @mention rendering; docs updated.Bug Fixes
--no-commentsand--all-commentsare provided together.Written for commit 33365f8. Summary will update on new commits.