[dead-code] chore: remove dead functions — 5 functions removed - #52508
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis. No new or modified behavioral tests in this PR. PR #52508 is a dead code removal that deletes tests for 5 removed functions (TestParseCatalog, TestParseCostMap, TestFindPricing, TestSpec_PublicAPI_FindPricing, TestSpec_DesignDecision_ProviderAliases). Remaining tests in catalog_test.go and spec_test.go are unmodified. Test Quality Sentinel: No action required.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Diff is a pure dead-code deletion (0 additions besides file headers, 396 deletions) with no new logic or abstractions introduced. Nothing to cut under a ponytail/over-engineering review.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories (0 additions detected).
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR cleanly removes 5 dead functions (FindPricing, ensureCatalog, downloadAndParseCatalog, parseCatalog, parseCostMap) along with all supporting types, variables, and their tests. No callers remain in the codebase. The removal is correct and complete.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 15 AIC · ⌖ 6.35 AIC · ⊞ 5.4K
There was a problem hiding this comment.
Pull request overview
Removes the models.dev pricing lookup implementation and its associated tests.
Changes:
- Removes
FindPricingand catalog download/parsing infrastructure. - Removes pricing-specific tests and fixtures.
- Retains provider and model-ID normalization utilities.
Show a summary per file
| File | Description |
|---|---|
pkg/modelsdev/catalog.go |
Removes pricing lookup and catalog handling. |
pkg/modelsdev/catalog_test.go |
Removes pricing and parsing tests. |
pkg/modelsdev/spec_test.go |
Removes pricing API specification tests. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
| "net/http" | ||
| "strconv" | ||
| "strings" |
There was a problem hiding this comment.
Request changes
This is not a safe dead-code removal as submitted: it deletes a documented public API and its entire behavior/tests without updating the package contract or the user-facing pricing story.
Blocking theme
pkg/modelsdev/README.mdstill documentsFindPricingas public API and includes a usage example that no longer compiles after this PR.- The repository docs still describe AIC pricing as coming from the
models.devcatalog, but this PR removes the implementation without showing the replacement path. - Because the change is pure deletion, there is no compensating migration note, deprecation path, or contract update here.
That makes this a breaking behavior/API change mislabeled as dead-code cleanup. The implementation can be removed, but only alongside the corresponding contract updates or an explicit replacement.
🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 8.49 AIC · ⌖ 4.63 AIC · ⊞ 6.5K
Comment /review to run again
| @@ -1,222 +1,13 @@ | |||
| package modelsdev | |||
There was a problem hiding this comment.
Removing this file’s entire pricing implementation without simultaneously updating the package contract leaves pkg/modelsdev/README.md and the public API spec claiming FindPricing still exists, which will break users and downstream automation that treat those docs/specs as authoritative.
💡 Update the contract docs or keep the API until callers are migrated
This PR deletes FindPricing and all of its behavior/tests, but the package README still documents FindPricing as part of the public API and still shows a compiling usage example. The repo also has broader docs that describe AIC pricing as being sourced from the models.dev catalog. If this removal is intentional, the docs/spec surface needs to be updated in the same change so consumers do not merge a broken contract.
Suggested follow-up in this PR:
- remove the `FindPricing` entry and example from `pkg/modelsdev/README.md`
- update any user-facing docs that describe gh-aw pricing lookup via models.dev
- explain the replacement source of pricing data in the PR body / changelogRight now this is a breaking API deletion disguised as dead-code cleanup.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — clean dead-code removal, no actionable issues.
📋 Summary
- ✅ All 5 removed functions have zero remaining callers
- ✅ Tests removed in lock-step with the implementation
- ✅ Supporting declarations fully pruned — no orphaned scaffolding
- ✅ Build, vet, and fmt verified per the PR checklist
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 17.6 AIC · ⌖ 6.53 AIC · ⊞ 7K
Comment /matt to run again
|
@copilot run pr-finisher skill |
|
@copilot Quick triage for maintainer-ready follow-up: Unresolved review threads (newest first):
Failed checks:
Please refresh the branch if needed, address the unresolved review feedback, rerun validation, and use the skill before handing back.
|
|
🎉 This pull request is included in a new release. Release: |
Summary
Removes unused pricing-catalog logic from
pkg/modelsdev: theFindPricingpublic API, its supporting helpers (ensureCatalog,downloadAndParseCatalog,parseCatalog,parseCostMap), and the associated types/constants/vars (rawCatalog,rawProvider,rawModel,pricingCache,catalogCache,httpClientFactory,catalogURL,fetchTimeout,maxBodyBytes,pkgLog). These functions had no remaining callers. Corresponding unit and spec tests for the removed code were deleted along with the implementation.Change Classification
FindPricingand its helpers were unused dead code; no external callers existed.Key Changes
pkg/modelsdev/catalog.goFindPricing,ensureCatalog,downloadAndParseCatalog,parseCatalog,parseCostMap, and related types/vars/constants; trimmed now-unused importspkg/modelsdev/catalog_test.goTestParseCatalog,TestParseCostMap,TestFindPricing); keptTestNormalizeProviderpkg/modelsdev/spec_test.goFindPricingbehavior and provider-alias handling viaFindPricing; keptTestSpec_PublicAPI_NormalizeProviderImpact Assessment
pkg/modelsdevsurface area and removes now-unnecessary dependencies onnet/http,encoding/json,io,context,strconv,time,pkg/logger, andpkg/syncutilfromcatalog.go.NormalizeProvider) are unaffected.Commits