Add collections and collection commands#140
Conversation
`hey collections` lists all collections via GET /collections.json. `hey collection <id>` lists every thread in a collection. The collection detail view has no JSON API and renders only ~50 threads per page, so the command walks the full timeline via the "next page" pagination link and parses both card__link (recent) and entry__collection-topic (older) anchors, de-duplicating across pages. Uses the existing OAuth auth; no session cookie required. Includes unit tests for the HTML parser and next-page extraction, and regenerates the .surface baseline for the new commands. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Adds new CLI commands to surface HEY “collections” (labels) and to enumerate all threads within a collection, including walking the HTML-only, paginated collection timeline.
Changes:
- Add
hey collectionsto fetch and display collections from/collections.json, with sorting and--limit/--alltruncation behavior. - Add
hey collection <id>to walk the collection timeline via “next page” pagination and return de-duplicated topic IDs/titles. - Add HTML parsing helpers + unit tests for topic extraction and next-page discovery.
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.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/htmlutil/collections.go | Adds regex-based HTML parsing for collection topics and next-page pagination. |
| internal/htmlutil/collections_test.go | Adds unit tests for collection HTML topic parsing and next-page extraction. |
| internal/cmd/root.go | Registers the new collections and collection commands in the root CLI. |
| internal/cmd/collections.go | Implements hey collections output, sorting, and truncation notice. |
| internal/cmd/collection.go | Implements hey collection <id> timeline walking and de-duplication. |
| .surface | Updates the command/flag surface snapshot baseline to include new commands/flags. |
Comments suppressed due to low confidence (1)
internal/htmlutil/collections.go:72
- ParseCollectionNextPage assumes the next-page regex has exactly one capture group. If the regex is updated to support either attribute order, select the first non-empty capture group so href is returned correctly.
func ParseCollectionNextPage(page string) string {
m := collectionNextPageRe.FindStringSubmatch(page)
if m == nil {
return ""
}
return html.UnescapeString(m[1])
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| table.addRow([]string{ | ||
| fmt.Sprintf("%d", col.Id), | ||
| col.Name, | ||
| col.UpdatedAt.Format("2006-01-02"), | ||
| }) |
| // collectionNextPageRe finds the timeline's "next page" pagination link. | ||
| collectionNextPageRe = regexp.MustCompile(`data-pagination-target="nextPageLink"[^>]*href="([^"]+)"`) | ||
| ) |
| with := `<a class="pagination-link" data-pagination-target="nextPageLink" href="/collections/5?page=abc%3D%3D&x=1">next</a>` | ||
| if got := ParseCollectionNextPage(with); got != "/collections/5?page=abc%3D%3D&x=1" { | ||
| t.Errorf("ParseCollectionNextPage = %q, want unescaped path", got) | ||
| } |
There was a problem hiding this comment.
5 issues found across 6 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/htmlutil/collections.go">
<violation number="1" location="internal/htmlutil/collections.go:19">
P2: Collections silently truncate when the pagination anchor emits `href` before `data-pagination-target`, because this regex treats attribute order as significant. Parse the anchor attributes independently (or handle both orders) so the full timeline walk remains robust.</violation>
</file>
<file name="internal/cmd/collection.go">
<violation number="1" location="internal/cmd/collection.go:55">
P3: Empty collections return `"data": null` in JSON/quiet output instead of an empty list, forcing clients to handle a different result type. Initialize `topics` to an empty slice.</violation>
<violation number="2" location="internal/cmd/collection.go:75">
P1: A collection page containing an absolute `https://` next link makes the next authenticated request go to that host, exposing the OAuth bearer token. Restrict parsed pagination links to a relative collection path (or validate same-origin after resolving it) before assigning `path`.</violation>
</file>
<file name="internal/htmlutil/collections_test.go">
<violation number="1" location="internal/htmlutil/collections_test.go:33">
P2: The existing test only exercises the case where `data-pagination-target` precedes `href`. Adding a test case with `href` appearing before `data-pagination-target` would guard against the attribute-order sensitivity of the regex and serve as a regression test for correct pagination extraction.</violation>
</file>
<file name="internal/cmd/collections.go">
<violation number="1" location="internal/cmd/collections.go:75">
P2: Calling `col.UpdatedAt.Format("2006-01-02")` directly will render as `0001-01-01` when the timestamp is zero-valued. Other commands in this codebase (e.g., `box.go`) use the `formatDate` helper to display an empty string instead. Using `formatDate` here would maintain consistency and avoid confusing output.</violation>
</file>
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
| topics = append(topics, t) | ||
| } | ||
| } | ||
| path = htmlutil.ParseCollectionNextPage(body) |
There was a problem hiding this comment.
P1: A collection page containing an absolute https:// next link makes the next authenticated request go to that host, exposing the OAuth bearer token. Restrict parsed pagination links to a relative collection path (or validate same-origin after resolving it) before assigning path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cmd/collection.go, line 75:
<comment>A collection page containing an absolute `https://` next link makes the next authenticated request go to that host, exposing the OAuth bearer token. Restrict parsed pagination links to a relative collection path (or validate same-origin after resolving it) before assigning `path`.</comment>
<file context>
@@ -0,0 +1,105 @@
+ topics = append(topics, t)
+ }
+ }
+ path = htmlutil.ParseCollectionNextPage(body)
+ }
+
</file context>
| titleAttrRe = regexp.MustCompile(`title="([^"]*)"`) | ||
| anyTagRe = regexp.MustCompile(`<[^>]+>`) | ||
| // collectionNextPageRe finds the timeline's "next page" pagination link. | ||
| collectionNextPageRe = regexp.MustCompile(`data-pagination-target="nextPageLink"[^>]*href="([^"]+)"`) |
There was a problem hiding this comment.
P2: Collections silently truncate when the pagination anchor emits href before data-pagination-target, because this regex treats attribute order as significant. Parse the anchor attributes independently (or handle both orders) so the full timeline walk remains robust.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/htmlutil/collections.go, line 19:
<comment>Collections silently truncate when the pagination anchor emits `href` before `data-pagination-target`, because this regex treats attribute order as significant. Parse the anchor attributes independently (or handle both orders) so the full timeline walk remains robust.</comment>
<file context>
@@ -0,0 +1,72 @@
+ titleAttrRe = regexp.MustCompile(`title="([^"]*)"`)
+ anyTagRe = regexp.MustCompile(`<[^>]+>`)
+ // collectionNextPageRe finds the timeline's "next page" pagination link.
+ collectionNextPageRe = regexp.MustCompile(`data-pagination-target="nextPageLink"[^>]*href="([^"]+)"`)
+)
+
</file context>
| } | ||
| } | ||
|
|
||
| func TestParseCollectionNextPage(t *testing.T) { |
There was a problem hiding this comment.
P2: The existing test only exercises the case where data-pagination-target precedes href. Adding a test case with href appearing before data-pagination-target would guard against the attribute-order sensitivity of the regex and serve as a regression test for correct pagination extraction.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/htmlutil/collections_test.go, line 33:
<comment>The existing test only exercises the case where `data-pagination-target` precedes `href`. Adding a test case with `href` appearing before `data-pagination-target` would guard against the attribute-order sensitivity of the regex and serve as a regression test for correct pagination extraction.</comment>
<file context>
@@ -0,0 +1,43 @@
+ }
+}
+
+func TestParseCollectionNextPage(t *testing.T) {
+ with := `<a class="pagination-link" data-pagination-target="nextPageLink" href="/collections/5?page=abc%3D%3D&x=1">next</a>`
+ if got := ParseCollectionNextPage(with); got != "/collections/5?page=abc%3D%3D&x=1" {
</file context>
| table.addRow([]string{ | ||
| fmt.Sprintf("%d", col.Id), | ||
| col.Name, | ||
| col.UpdatedAt.Format("2006-01-02"), |
There was a problem hiding this comment.
P2: Calling col.UpdatedAt.Format("2006-01-02") directly will render as 0001-01-01 when the timestamp is zero-valued. Other commands in this codebase (e.g., box.go) use the formatDate helper to display an empty string instead. Using formatDate here would maintain consistency and avoid confusing output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cmd/collections.go, line 75:
<comment>Calling `col.UpdatedAt.Format("2006-01-02")` directly will render as `0001-01-01` when the timestamp is zero-valued. Other commands in this codebase (e.g., `box.go`) use the `formatDate` helper to display an empty string instead. Using `formatDate` here would maintain consistency and avoid confusing output.</comment>
<file context>
@@ -0,0 +1,89 @@
+ table.addRow([]string{
+ fmt.Sprintf("%d", col.Id),
+ col.Name,
+ col.UpdatedAt.Format("2006-01-02"),
+ })
+ }
</file context>
| col.UpdatedAt.Format("2006-01-02"), | |
| formatDate(col.UpdatedAt), |
| // page and accumulate the threads, de-duplicating across pages. | ||
| ctx := cmd.Context() | ||
| path := fmt.Sprintf("/collections/%d", collectionID) | ||
| var topics []htmlutil.CollectionTopic |
There was a problem hiding this comment.
P3: Empty collections return "data": null in JSON/quiet output instead of an empty list, forcing clients to handle a different result type. Initialize topics to an empty slice.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cmd/collection.go, line 55:
<comment>Empty collections return `"data": null` in JSON/quiet output instead of an empty list, forcing clients to handle a different result type. Initialize `topics` to an empty slice.</comment>
<file context>
@@ -0,0 +1,105 @@
+ // page and accumulate the threads, de-duplicating across pages.
+ ctx := cmd.Context()
+ path := fmt.Sprintf("/collections/%d", collectionID)
+ var topics []htmlutil.CollectionTopic
+ seen := map[int64]bool{}
+ var truncated bool
</file context>
| var topics []htmlutil.CollectionTopic | |
| topics := []htmlutil.CollectionTopic{} |
Summary
hey collections— lists all collections viaGET /collections.json(sorted by name;--limit/--all/--json)hey collection <id>— lists every thread in a collectionhey collectionwalks the full timeline via the "next page" pagination link, parsing bothcard__link(recent) andentry__collection-topic(older) anchors and de-duplicating across pagesTest plan
hey collectionslists every collection, sorted by namehey collection <id>returns all threads, not just the first page (85-thread collection fully enumerated; previously silently truncated to 50)make checkpasses (gofmt, goimports, vet, golangci-lint, unit tests, tidy)🤖 Generated with Claude Code
Summary by cubic
Added
hey collectionsandhey collection <id>commands to list collections and fully enumerate threads in a collection. This fixes silent truncation at ~50 threads by walking HTML pagination.hey collections: Fetches viaGET /collections.json, sorted by name. Supports--limit,--all, and--json.hey collection <id>: Walks the "next page" timeline, parsingcard__linkandentry__collection-topicanchors. De-duplicates and shows all threads. Includes a safety cap at 200 pages with a truncation notice if hit.--jsonreturns structured data.Written for commit d7363a6. Summary will update on new commits.