Repository navigation
Use a numeric --in as given, without walking /projects.json - #916
Conversation
Resolving a numeric project ID walked every page of /projects.json to find the project's name, on every command given --in <id>. Most callers discard the name, and an ID missing from the list was already passed through for the API to judge, so the walk bought nothing but a request per page. A numeric ID now resolves with no request. Its name comes back only when the session already holds the project list. The two callers that show it get it another way: config project reads that one project (ProjectName) to confirm it exists before saving it, and timeline takes it from the events' bucket.
github-actions
Bot
added
commands
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 589b1aeb82
ℹ️ 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.
🟡 Changes recommended
A populated active-project cache incorrectly prevents name lookup for accessible archived projects.
1 open issue
What changed in this PR
This PR reduces API load by using numeric project IDs without listing projects, complementing #909’s request pacing.
Changes:
- Skip project-list requests for numeric IDs.
- Fetch project names directly for configuration and from event buckets for timelines.
- Update tests for lookup behavior and reduced request counts.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
| internal/names/resolver.go | Adds numeric-ID passthrough and direct name lookup. |
| internal/names/resolver_test.go | Tests request counts, caching, and lookup errors. |
| internal/commands/timeline.go | Uses event buckets for project names. |
| internal/commands/templates_test.go | Updates expected request counts. |
| internal/commands/config.go | Looks up unnamed projects before saving. |
| internal/commands/config_test.go | Serves individual project lookup responses. |
Review effort: Balanced
ResolveProject now returns a numeric ID in canonical form ("00123" →
"123") even when no project list is cached. Callers such as the wormhole
conflict check compare it with bucket IDs as strings, so passing the
spelling through as typed rejected a same-project move.
ProjectName uses the session's project list only for a positive match.
That list holds active projects only, so a miss falls through to reading
the one project, as a fresh resolver already did; an archived project's
name no longer depends on whether the list was fetched first.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
What
A numeric project ID (
--in 123,--project 123) is now used as given, with no request. Until nowResolveProjectwalked every page of/projects.json(resolver.go:432) to find the project's name. It did this on every command that takes--in <id>. Most of the 85 callers discard the name. An ID that wasn't in the list was already passed through for the API to judge, so the walk validated nothing.The name comes back only when the session already holds the project list. The two callers that show it now get it another way:
config project --project <id>reads that one project with the newProjectName(GET /projects/<id>.json) to confirm the project exists before saving it. A 404 or 403 still says "not found".timeline --in <id>takes the name from the events' bucket. With no events it falls back to "project #id", the fallback it already had.Name resolution (
--in "Project name") is unchanged and still lists projects.Why
BC5's API hosts are short of CPU. See this Basecamp card: Ops found that
basecamp-cli/*requests use 18% of Rails CPU on the API hosts, and thatProjectsController#indexis the third most expensive endpoint there.In 5 minutes of API-host logs (19 Oct, 18:56–19:01 UTC), the CLI's
ProjectsController#indexrequests used about 193 s of CPU. About 4.5% of all API-host Rails CPU in that window went to these walks. 161 s of it came from requests that sentIf-None-Match, and only the SDK's rawGetAllpath sends that on page 1, which is the path this resolver uses. Those requests broke down as:The bug-triage tooling we run ourselves calls
basecamp cards list --in <id>and its siblings many times per sync. Each call walked the whole list first.Name-based
--instill walks the list, and versions already installed keep walking until they upgrade. #909 (pace every request) is complementary to this change, not overlapping.Behaviour changes
config project, so it can be saved. Before, the archived project was missing from the active list, so it was refused as not found.Testing
ProjectNamereads exactly one project;00123and+123resolve to123./projects/<id>.json.bin/ciexits 0 on Linux at 47125c4.make lintflagsinternal/connector/driver/proctime_darwin.go(gosec G115, unconvert). That fails onmaintoo and is untouched here.Summary by cubic
A numeric project ID (
--in 123,--project 123) is now used as given, without walking every page of/projects.jsonfirst. That walk ran on every command taking--in <id>and returned a name most callers discard; the API-host CPU cost it caused was called out by Ops as significant.Behavior changes
config project --project <id>reads that one project (GET /projects/<id>.json) to confirm it exists before saving.timeline --in <id>takes the project name from the events' bucket, keeping the fallback format for empty timelines.config project;ProjectNamereads the project itself when it is missing from the session's active-only list.--in "Name") is unchanged and still lists projects.Written for commit 47125c4. Summary will update on new commits.