Adding Progress Reporting

[geoffreywiseman]

At times it felt like the extension was stalled simply because it was fetching a lot of data. Adding some progress reporting so it’s easier to tell when it’s working. There are still some places where it might go silent for a while, but it’s less frequent.

[closes #59]

[Copilot]

🟡 Changes recommended

Unresolved progress-output corruption and misleading duration-estimate issues remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds progress reporting for long-running usage collections while keeping reports on stdout and status messages on stderr.

Changes:

• Adds collection planning, progress hooks, and duration estimates.
• Adds terminal and non-terminal progress rendering.
• Optimizes pagination and updates documentation and tests.

File summaries

File	Summary and findings
usage/collector.go	Adds planning and progress tracking. Moderate issue: durations near one hour can incorrectly display as 60 minutes.
usage/collector_test.go	Adds collection progress and estimate tests.
README.md	Documents progress output behavior.
progress.go	Implements progress rendering. Moderate issue: rate-limit notices can corrupt in-place terminal output.
progress_test.go	Tests terminal and log rendering.
main.go	Wires progress and status output. Moderate issue: rate-limit notices bypass synchronized output; nit: failures can print on the unfinished progress line, including line 161.
client/transport.go	Exposes request pacing information.
client/client.go	Improves pagination behavior.
client/client_test.go	Updates pagination request tests.

Review details

Suppressed comments (3)

main.go:161

• This success-only separator has the same terminal failure issue for multi-target runs: a listing or fetch error can leave the in-place counter unterminated, and the error is then printed on that line. Separate the status output before checking err (or use a deferred separator) so the error path is readable too.

	status.separate()

progress.go:79

• The terminal renderer leaves the cursor at the end of the in-place counter, but rate-limit backoff notices still write directly to os.Stderr in client/transport.go. If a secondary limit occurs during this fetch, the notice is appended to the progress text and the next carriage return can overwrite part of that notice; route backoff notifications through the same status/output coordination or terminate and redraw the progress line around them.

	_, _ = fmt.Fprintf(p.w, "\r%s", p.line(done, total))
	if done == total {
		_, _ = fmt.Fprintln(p.w)

usage/collector.go:242

• For durations from 59m30s through just under an hour, this branch rounds to 60 minutes and reports 60 minutes even though the next branch's intended coarse unit is hours. Handle the rounded 60-minute boundary so the estimate does not display this misleading value.

	case d < time.Minute:
		return plural(int(d.Round(time.Second)/time.Second), "second")
	case d < time.Hour:
		return plural(int(d.Round(time.Minute)/time.Minute), "minute")

• Files reviewed: 9/9 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The exported constructor change is source-incompatible, and pagination lacks full-page boundary coverage.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

client/client.go:79

• This new pagination exit condition is only covered by a short first page. The exact-100 boundary must request page 2; without a test for that case, a change to this condition could silently truncate repositories with exactly a full first page.

		if len(wfp) < perPage {
			return workflows, nil
		}

client/client.go:175

• The analogous repository pagination branch is also only tested with a short first page. Please cover a full 100-item page followed by a second page, since this changed condition is what determines whether the second page is fetched.

		if len(rp) < perPage {
			return repos, nil
		}

• Files reviewed: 10/10 changed files
• Comments generated: 1
• Review effort level: Lite