Conversation
Remove 4 unreachable functions identified by deadcode analyzer: - effectiveTokenMultiplier - computeBaseWeightedTokens - computeModelEffectiveTokens - populateEffectiveTokens These functions were superseded by computeModelEffectiveTokensWithWeights and populateEffectiveTokensWithCustomWeights, which are the live implementations called from production code (token_usage.go). Also remove 8 exclusive test functions that only tested the deleted code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude · ● 236K
| // - Embedding model_multipliers.json at compile time | ||
| // - Applying token class weights before the model multiplier | ||
| // - Providing model multiplier lookup with prefix matching for model variants | ||
| // - Computing effective tokens from raw per-model token usage data |
There was a problem hiding this comment.
Good cleanup — removing the Providing model multiplier lookup with prefix matching comment accurately reflects that the dead functions are gone. The comment now matches the live code.
| }) | ||
| } | ||
|
|
||
| func TestModelMultipliersJSONEmbedded(t *testing.T) { |
There was a problem hiding this comment.
Test coverage removal looks correct — these tests were covering effectiveTokenMultiplier, computeBaseWeightedTokens, computeModelEffectiveTokens, and populateEffectiveTokens which are all dead functions. The live implementations (computeModelEffectiveTokensWithWeights) are still tested.
There was a problem hiding this comment.
Pull request overview
Removes dead/unreachable Effective Tokens helper functions and their now-obsolete unit tests after the codebase migrated to the weighted implementations used by token_usage.go.
Changes:
- Deleted 4 unused ET helper functions from
pkg/cli/effective_tokens.go. - Removed the unit tests that exercised the deleted helpers from
pkg/cli/effective_tokens_test.go.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/cli/effective_tokens.go | Removes unused ET helper functions; leaves weighted ET computation as the only path. |
| pkg/cli/effective_tokens_test.go | Deletes tests tied to removed helpers; retains tests for embedded JSON parsing and custom-weight merging. |
Comments suppressed due to low confidence (2)
pkg/cli/effective_tokens.go:129
- populateEffectiveTokensWithCustomWeights is documented as "like populateEffectiveTokens", but populateEffectiveTokens was removed in this PR. Please reword this doc comment to be self-contained (e.g., describe that it populates EffectiveTokens/TotalEffectiveTokens and optionally merges custom weights) so readers don't have to hunt for a non-existent symbol.
// populateEffectiveTokensWithCustomWeights is like populateEffectiveTokens but
// merges custom into the built-in weights before computing effective tokens.
// Custom weights take precedence over the defaults loaded from model_multipliers.json.
// It is a no-op when summary is nil.
pkg/cli/effective_tokens_test.go:19
- This PR removes the direct tests for model multiplier lookup semantics (case-insensitive exact match, longest-prefix match for model variants, and unknown-model default). That behavior still exists in computeModelEffectiveTokensWithWeights, but is no longer covered explicitly. Please add focused tests for these lookup rules to avoid regressions in effective token computation.
func TestModelMultipliersJSONEmbedded(t *testing.T) {
// Verify the embedded JSON parses without error
loadedMultipliers = nil
initMultipliers()
require.NotNil(t, loadedMultipliers, "multipliers should be loaded from embedded JSON")
assert.NotEmpty(t, loadedMultipliers, "should have at least one multiplier entry")
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -23,7 +23,6 @@ package cli | |||
| // Key responsibilities: | |||
| // - Embedding model_multipliers.json at compile time | |||
There was a problem hiding this comment.
The file-level "Key responsibilities" comment no longer mentions the model multiplier lookup behavior, but computeModelEffectiveTokensWithWeights still performs case-insensitive exact matching and longest-prefix matching for model variants. Please update this header comment (or add a brief note near the lookup implementation) so the documented responsibilities stay aligned with the current behavior.
This issue also appears on line 126 of the same file.
| // - Embedding model_multipliers.json at compile time | |
| // - Embedding model_multipliers.json at compile time | |
| // - Resolving model multipliers using case-insensitive exact matching and | |
| // longest-prefix matching for model variants |
Dead Code Removal
This PR removes unreachable Go functions identified by the
deadcodestatic analyzer.Functions Removed
effectiveTokenMultiplierpkg/cli/effective_tokens.gocomputeBaseWeightedTokenspkg/cli/effective_tokens.gocomputeModelEffectiveTokenspkg/cli/effective_tokens.gopopulateEffectiveTokenspkg/cli/effective_tokens.goThese four functions were superseded by
computeModelEffectiveTokensWithWeightsandpopulateEffectiveTokensWithCustomWeights, which are the live implementations called from production code (token_usage.go). The simpler wrappers were never called outside of tests.Tests Removed
TestEffectiveTokenMultiplierExactMatchpkg/cli/effective_tokens_test.goTestEffectiveTokenMultiplierCaseInsensitivepkg/cli/effective_tokens_test.goTestEffectiveTokenMultiplierPrefixMatchpkg/cli/effective_tokens_test.goTestEffectiveTokenMultiplierUnknownModelpkg/cli/effective_tokens_test.goTestComputeBaseWeightedTokenspkg/cli/effective_tokens_test.goTestComputeModelEffectiveTokenspkg/cli/effective_tokens_test.goTestPopulateEffectiveTokenspkg/cli/effective_tokens_test.goTestPopulateEffectiveTokensNilSummarypkg/cli/effective_tokens_test.goVerification
go build ./...— passesgo vet ./...— passesgo vet -tags=integration ./...— passesmake fmt— no changes neededDead Function Count
Automated by Dead Code Removal workflow — https://github.com/github/gh-aw/actions/runs/23945333342
✨ PR Review Safe Output Test - Run 23946149750