Conversation
Replace "recording" with user-facing terms in the command catalog: assign → "Assign someone to an item", react → "React with an emoji", recordings → "Browse content by type across projects", show → "Show any item by ID", events → "View change history", timesheet actions → "item" instead of "recording".
Replace "recording" with "item" or "content" in all help text, flag descriptions, breadcrumbs, summaries, and error messages. Agent notes updated to drop leading "Recordings" reference.
Replace "recording" with "item" in user-visible strings across show.go, events.go, and subscriptions.go. Update Use strings from <recording_id|url> to <id|url>. Update e2e assertion.
Replace "recording" with "item" in flag descriptions, help text, breadcrumbs, and error messages. Update react shortcut to "React with an emoji". Update e2e assertion for --on error.
Rename the "recording" subcommand to "item" with "recording" as an alias for backward compat. Update all user-visible text and breadcrumbs to use "item" instead of "recording".
Update search breadcrumb and reports --group-by to use "project" instead of "bucket". "bucket" remains accepted for backward compat and is normalized to the API value before the SDK call.
Replace "vault" with "folder" in user-visible error messages and help text. Simplify Long description to remove API jargon.
Review carefully before merging. Consider a major version bump. |
There was a problem hiding this comment.
4 issues found across 17 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/boost.go">
<violation number="1" location="internal/commands/boost.go:53">
P2: Missed "recording" → "item" replacement on the last line of this Long string: `Use --event to list boosts on a specific event within the recording.` still uses the old terminology.</violation>
<violation number="2" location="internal/commands/boost.go:229">
P2: Missed "recording" → "item" replacement on the last line of this Long string: `Use --event to boost a specific event within the recording.` still uses the old terminology.</violation>
<violation number="3" location="internal/commands/boost.go:387">
P2: Missed "recording" → "item" replacement in this Long string: `Content as positional argument, --on for the recording:` still uses the old terminology. Consider changing to `--on for the item:`.</violation>
</file>
<file name="internal/commands/reports.go">
<violation number="1" location="internal/commands/reports.go:117">
P2: When the user passes `--group-by project`, the summary will still display "grouped by bucket" because `result.GroupedBy` echoes the API response value (`"bucket"`), not the user's input. Normalize the display value to keep the user-facing output consistent with the terminology change — e.g., use the local `groupBy` variable or map `"bucket"` → `"project"` before formatting the summary.</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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22589529d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR is a terminology polishing pass that replaces internal Basecamp API terms ("recording", "bucket", "vault") with more user-friendly equivalents ("item"/"content", "project", "folder") throughout all user-visible CLI strings: help text, flag descriptions, error messages, breadcrumbs, and summaries. The timesheet recording subcommand is renamed to timesheet item with recording kept as a backward-compatible alias. The --group-by bucket flag value is still accepted but normalized to send "bucket" to the API while the preferred user-facing value becomes "project". Go variable names, struct fields, SDK types, command names, and flag names are intentionally left unchanged.
Changes:
- Replace "recording" → "item"/"content" in all user-visible strings across
recordings.go,show.go,events.go,timesheet.go,subscriptions.go,comment.go,boost.go,todos.go,files.go,commands.go, andcli/root.go - Replace "bucket" → "project" in
reports.go--group-by(accepts both, normalizes before SDK call) and search breadcrumbs insearch.goandrecordings.go - Rename
timesheet recordingsubcommand totimesheet itemwithrecordingalias; update e2e tests and.surfacesnapshot accordingly
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
internal/commands/todos.go |
Replace "recording types" → "content types" in agent_notes annotation |
internal/commands/timesheet.go |
Replace "recording" → "item" in Long, breadcrumbs, and subcommand; add recording as alias for backward compat |
internal/commands/subscriptions.go |
Replace "recording" with "item" in Use, Short, Long, and error messages |
internal/commands/show.go |
Replace "recording" → "item" in Short, Long, error messages, and flag description |
internal/commands/search.go |
Replace bucket.id → project_id in breadcrumb command example |
internal/commands/reports.go |
Update --group-by to accept "project" as preferred value, normalize "project"→"bucket" for API, update flag description and error message |
internal/commands/recordings.go |
Replace "recording" → "item"/"content" in Short, Long, flag descriptions, error messages, and breadcrumbs |
internal/commands/files.go |
Replace "vault" → "folder" in error messages and root command Long/Short |
internal/commands/events.go |
Replace "recording" → "item" in Use, Short, Long, breadcrumb action name, and summary |
internal/commands/comment.go |
Replace "recording" → "item" in Long, flag descriptions, and error messages |
internal/commands/commands.go |
Update catalog descriptions for assign, react, recordings, show, and events commands; update timesheet actions list |
internal/commands/boost.go |
Replace "recording" → "item" in Long, Short, error messages, and flag descriptions |
internal/cli/root.go |
Update --on required value error message terminology |
e2e/timesheet.bats |
Update e2e assertion from "recording" → "item" |
e2e/events.bats |
Update e2e assertion from "recording_id" → "id|url" |
e2e/errors.bats |
Update e2e assertion for --on requires an ID |
.surface |
Update CLI surface snapshot: rename timesheet recording → timesheet item and all related flags |
Comments suppressed due to low confidence (1)
internal/commands/reports.go:141
- When
--group-by projectis used, the PR correctly normalizesapiGroupByto"bucket"before the SDK call. However, the summary line at line 140 usesresult.GroupedBydirectly from the API response, which will return"bucket"regardless of whether the user typed"project"or"bucket". This means users will see(grouped by bucket)in the summary output even when they used--group-by project, leaking the internal API term the PR aims to hide.
The normalization should also apply to the display: consider comparing against groupBy (the user-supplied value, normalized to "project") when building the summary, or apply a reverse mapping from "bucket" to "project" before formatting the summary.
if groupBy != "" {
if groupBy != "bucket" && groupBy != "project" && groupBy != "date" {
return output.ErrUsage("--group-by must be 'project' or 'date'")
}
apiGroupBy := groupBy
if apiGroupBy == "project" {
apiGroupBy = "bucket"
}
opts = &basecamp.AssignedTodosOptions{GroupBy: apiGroupBy}
}
result, err := app.Account().Reports().AssignedTodos(cmd.Context(), personID, opts)
if err != nil {
return convertSDKError(err)
}
// Build summary
todoCount := len(result.Todos)
displayName := personName
if displayName == "" && result.Person != nil {
displayName = result.Person.Name
}
if displayName == "" {
displayName = personIDStr
}
summary := fmt.Sprintf("%d todos assigned to %s", todoCount, displayName)
if result.GroupedBy != "" {
summary += fmt.Sprintf(" (grouped by %s)", result.GroupedBy)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix 3 missed "recording" → "item" replacements in boost.go - Map API "bucket" back to "project" in reports assigned summary - Include command aliases in --help --agent output so surface snapshot tracks backward-compatible names (fixes CI surface check)
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
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/cli/root.go">
<violation number="1" location="internal/cli/root.go:602">
P2: `strings.Replace` replaces the *first* occurrence of `sub.Name()` in the path, but the command name is always the *last* segment. If the name is a substring of a parent command (e.g., parent `items`, child `item`), this corrupts the parent instead of swapping the leaf. Use `strings.TrimSuffix` to target the trailing name.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Use strings.TrimSuffix for safer alias path construction in agent help - Replace remaining "vault" with "folder" in files.go subcommand help text - Replace "recordings" with "items" in comment.go batch error messages
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
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/comment.go">
<violation number="1" location="internal/commands/comment.go:368">
P2: The adjacent `ErrUsage` fallback still says `"Failed to comment on all recordings"` — this line was missed in the terminology update. The same code block now mixes "items" and "recordings" in user-facing text.</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.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
User-facing CLI text now speaks two languages: app-UI terms for humans, API terms for machine consumers.
Human-facing (Short, Long, errors, summaries):
--group-by(accepts both values, normalizes before SDK call)timesheet recording→timesheet itemwithrecordingalias for backward compatAgent-facing (agent_notes):
recording_id, API type values (Todo, Message, Document, Kanban::Card, Upload)showcommand (previously had none)bucket_id→project_idin url.go agent_notesFlag descriptions bridge both vocabularies where relevant:
--typeshows friendly names and API type strings--group-byshows'project' (API: 'bucket')Long descriptions bridge vocabularies where the API concept matters:
Go variable names, struct fields, SDK types, command names (
recordings), and flag names (--recording,--vault) are deliberately unchanged.Test plan
make checkpasses after each commit (fmt-check, vet, lint, test, test-e2e)timesheet itemrenamebasecamp recordings --helpshows "content" not "recording" in descriptionsbasecamp timesheet recording --helpstill works (alias)basecamp reports assigned --group-by bucketstill works (backward compat)basecamp reports assigned --group-by projectworks (new preferred value)