Skip to content

[dead-code] chore: remove dead functions — 1 function removed#38858

Merged
pelikhan merged 1 commit into
mainfrom
dead-code/remove-ResolveDefaultMaxAICredits-2026-06-12-82a5da4088d6c206
Jun 12, 2026
Merged

[dead-code] chore: remove dead functions — 1 function removed#38858
pelikhan merged 1 commit into
mainfrom
dead-code/remove-ResolveDefaultMaxAICredits-2026-06-12-82a5da4088d6c206

Conversation

@github-actions

@github-actions github-actions Bot commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

[dead-code] chore: remove dead functions — 1 function removed

Summary

Removes ResolveDefaultMaxAICredits from the compiler-env manager package. The function was flagged as unreachable by the deadcode analyser: it had no callers outside its own test file. The companion test TestResolveDefaultMaxAICredits is removed in the same commit.

What changed

pkg/workflow/compilerenv/manager.go — breaking removal

  • Deleted the exported function ResolveDefaultMaxAICredits(fallback int64) int64.
    This function read the enterprise env-var GH_AW_DEFAULT_MAX_AI_CREDITS, applied optional K/M suffix normalisation via typeutil.NormalizeInt64KMSuffix, and fell back to the supplied default when the var was absent or invalid. The special sentinel value -1 (disable budget enforcement) was preserved.
  • Removed the "github.com/github/gh-aw/pkg/typeutil" import, which was only needed by the deleted function.
  • Net change: −25 lines.

pkg/workflow/compilerenv/manager_test.go — test clean-up

  • Deleted TestResolveDefaultMaxAICredits and all its subtests (env-var unset, plain integer, K/M suffix, -1 sentinel, invalid values).
  • Net change: −32 lines.

Impact

Dimension Detail
Breaking Yes — ResolveDefaultMaxAICredits is a public symbol. Any caller outside this repo that imported and called this function will fail to compile.
Runtime behaviour None — deadcode confirmed zero callers exist in the repo tree; the env-var GH_AW_DEFAULT_MAX_AI_CREDITS is no longer read at runtime.
Test coverage No coverage gap introduced; the deleted tests covered only the deleted code.
Dependencies pkg/typeutil import dropped from manager.go; no other consumers of typeutil in this file.

Why this is safe

  • The deadcode static analyser flagged ResolveDefaultMaxAICredits as unreachable — confirmed by the commit message: "no callers outside test files".
  • The companion function ResolveDefaultMaxTurns (same file, same pattern) is not removed and remains intact.
  • Total diff is purely subtractive: 57 lines deleted, 0 lines added.

Checklist

  • Dead-code flagged by automated analyser (deadcode)
  • Public symbol removed — breaking change noted
  • Exclusive test deleted alongside production code
  • Unused import cleaned up
  • No new logic introduced

Generated by PR Description Updater for issue #38858 · 103.6 AIC · ⌖ 28.2 AIC · ⊞ 19.8K · ◷

@github-actions
chore: remove dead functions — 1 function removed
7d7e982 
Remove ResolveDefaultMaxAICredits and its exclusive test.
Flagged as unreachable by deadcode (no callers outside test files).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review June 12, 2026 16:08
Copilot AI review requested due to automatic review settings June 12, 2026 16:08
@pelikhan pelikhan merged commit c2e7e6b into main Jun 12, 2026
22 checks passed
@pelikhan pelikhan deleted the dead-code/remove-ResolveDefaultMaxAICredits-2026-06-12-82a5da4088d6c206 branch June 12, 2026 16:08
@github-actions

github-actions Bot commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actions Bot commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

🧪 Test Quality Sentinel completed test quality analysis.

@github-actions

github-actions Bot commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #38858 does not have the implementation label (has_implementation_label=false) and has 0 new lines of code in business logic directories, well below the 100-line threshold (requires_adr_by_default_volume=false).

Copilot AI reviewed Jun 12, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes a dead codepath for resolving the default max AI credits from the compiler process environment, along with its unit test, leaving the runtime variable-expression based configuration as the remaining mechanism.

Changes:

  • Removed ResolveDefaultMaxAICredits from pkg/workflow/compilerenv/manager.go.
  • Dropped the now-unused typeutil import from manager.go.
  • Removed TestResolveDefaultMaxAICredits from pkg/workflow/compilerenv/manager_test.go.
Show a summary per file
File Description
pkg/workflow/compilerenv/manager.go Removes the dead ResolveDefaultMaxAICredits helper and its related import.
pkg/workflow/compilerenv/manager_test.go Removes the unit test that only covered the deleted helper.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@github-actions github-actions Bot mentioned this pull request Jun 12, 2026
Closed
github-actions[bot]
github-actions Bot commented Jun 12, 2026
View reviewed changes

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skills-Based Review 🧠

Applied /zoom-out — clean, well-scoped dead-code removal. ✅

📋 Analysis Summary

Verification

  • ResolveDefaultMaxAICredits has zero callers in the codebase (confirmed by search)
  • The DefaultMaxAICredits constant is correctly retained — still used by BuildDefaultMaxAICreditsExpression, engine.go, and test code
  • The typeutil import removal is correct — it was the sole consumer in this file
  • Architecture decision explicitly captured in awf_config_test.go: env var is NOT used at compile time, resolved at action runtime
  • go build ./... and go vet ./... pass per PR checklist

Positive Highlights

  • ✅ Tests for the removed function removed alongside the function — no orphaned test stubs
  • ✅ DefaultMaxAICredits constant preserved for future runtime use
  • ✅ PR description is clear and accurate

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 193.7 AIC · ⌖ 13.3 AIC · ⊞ 27.8K

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

🧪 Test Quality Sentinel Report

✅ Test Quality Score: N/A — No new tests to analyze

This PR removes 1 dead function (ResolveDefaultMaxAICredits) and its corresponding test (TestResolveDefaultMaxAICredits, 6 sub-cases). No new or modified tests were added.

📊 Metrics & Test Classification (0 tests analyzed)
Metric Value
New/modified tests analyzed 0
Tests removed (dead-code cleanup) 6 sub-tests of TestResolveDefaultMaxAICredits
🚨 Coding-guideline violations 0

Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 0 new/modified tests

i️ The only test file change (pkg/workflow/compilerenv/manager_test.go) consists entirely of deletions — TestResolveDefaultMaxAICredits and all 6 of its sub-cases were removed alongside the dead production function ResolveDefaultMaxAICredits.

Verdict

✅ Check passed. No new or modified tests to evaluate. Dead-code removal is clean — both the production function and its tests were removed consistently.

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · 170.8 AIC · ⌖ 34.6 AIC · ⊞ 27.2K · ◷

github-actions[bot]
github-actions Bot commented Jun 12, 2026
View reviewed changes

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: N/A. No new tests to evaluate — this PR is a clean dead-code removal that correctly removes both the dead production function (ResolveDefaultMaxAICredits) and its corresponding tests.

This was referenced Jun 12, 2026
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

chore dead-code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pelikhan