Repository navigation
[dead-code] chore: remove dead functions — 2 functions removed - #65621
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
There was a problem hiding this comment.
Verdict
Non-blocking, but this dead-code cleanup leaves one stale reference behind.
Themes
pkg/workflow/README.mdstill advertisesGetLogParserBootstrapeven though this PR deletes the function. That leaves the exported API inventory out of sync with the code and sends future readers/tooling toward a symbol that no longer exists.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 22.3 AIC · ⌖ 7.2 AIC · ⊞ 19.4K
Comment /review to run again
Comments that could not be inline-anchored
pkg/workflow/js.go:23
Removing GetLogParserBootstrap without updating pkg/workflow/README.md leaves the exported API inventory pointing at a symbol that no longer exists, which is exactly the kind of stale reference dead-code cleanup should remove in the same PR.
<details><summary>💡 Keep the API inventory in sync with the code removal</summary>
pkg/workflow/README.md still lists GetLogParserBootstrap at line 1412. Please delete that row here so repo docs and any tooling built from that inventory do not im…
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Removing the documented exported bootstrap function breaks the public Go API without a compatibility or major-version plan.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Removes two functions identified as dead code and their obsolete tests.
Changes:
- Removes an unused Codex token-usage wrapper.
- Removes the exported log-parser bootstrap stub.
- Deletes associated tests and implementation comments.
| File | Description |
|---|---|
pkg/workflow/js.go |
Removes bootstrap stub. |
pkg/workflow/js_test.go |
Removes bootstrap test. |
pkg/workflow/codex_logs.go |
Removes unused Codex wrapper. |
pkg/workflow/codex_total_tokens_test.go |
Removes wrapper-specific tests. |
pkg/workflow/codex_engine.go |
Removes obsolete implementation comment. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| func GetReadBufferScript() string { | ||
| return "" | ||
| } |
There was a problem hiding this comment.
Skills-Based Review 🧠
Clean, mechanical dead-code removal — no skill-specific issues found.
📋 Analysis
Change type
Pure deletion (0 additions, 63 deletions across 5 files). No new abstractions, no behavior change, and no bug being fixed, so /tdd, /diagnosing-bugs, /codebase-design, and /improve-codebase-architecture do not surface meaningful findings here.
Verification performed
- Confirmed
CodexEngine.extractCodexTokenUsageandGetLogParserBootstraphave zero remaining references anywhere in the Go codebase. - Ran
go build ./...— succeeds. - Ran
go vet ./...— clean. - Corresponding unit tests (
TestCodexExtractTokenUsageTotalTokensPattern,TestGetLogParserBootstrap) were removed alongside the dead functions, keeping test/code parity. codex_logs.go's remainingextractCodexLegacyTokenCount(the real logicextractCodexTokenUsageused to wrap) is still present and exercised byTestCodexParseLogMetricsLegacyTokenSnapshots.
Positive highlights
- ✅ Comment references updated in
codex_engine.go(stale// extractCodexTokenUsage is implemented in codex_logs.gopointer removed) rather than left dangling. - ✅ Tests removed in lockstep with the functions they covered — no orphaned test failures introduced.
No blocking issues. Approving.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 17.9 AIC · ⌖ 14.3 AIC · ⊞ 10K
Comment /matt to run again
|
🎉 This pull request is included in a new release. Release: |

Functions Removed
CodexEngine.extractCodexTokenUsageGetLogParserBootstrapTests Removed
TestCodexExtractTokenUsageTotalTokensPatternTestGetLogParserBootstrapVerification
go build ./...go vet ./...(pkg/workflow)go vet -tags=integration ./...(pkg/workflow)make fmtNote:
go test ./pkg/workflow/has 6 pre-existing failures that also occur without this change.https://github.com/github/gh-aw/actions/runs/37213835402