refactor: remove dead/vestigial helpers with zero prod usage#499
Conversation
Removes six internal helpers verified to have zero callers across
comfy_cli/ and tests/ (no dynamic dispatch touches them):
- workspace_manager.scan_dir_concur (also buggy: hardcoded Path('.'),
ignoring self.workspace_path) and its only-caller-of check_file_is_model
- workspace_manager.check_file_is_model (only referenced by scan_dir_concur)
- commented-out load_yaml block in workspace_manager
- renderer.Renderer.flush_throttled
- glyphs.glyph_only
- branding.cli_footer
Also drops the now-orphaned concurrent.futures and pathlib.Path imports
that were used only by scan_dir_concur. Pure removal, no behavior change.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
✅ No high-signal findings.
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
ELI-5
We had six little helper functions (and one big commented-out block of old code) that nothing in the project actually uses anymore — leftover scaffolding. This PR just deletes them. Nothing that runs changes; there's simply less dead code to trip over.
What & why
Pure removal of dead/vestigial code — every item below was verified to have zero callers across both
comfy_cli/andtests/(grep is authoritative here: none of these are reached viagetattr/globalsdynamic dispatch, and none are re-exported via any module__all__):workspace_manager.scan_dir_concur— 0 callers. Also latently buggy: it hardcodedbase_path = Path("."), ignoringself.workspace_path, so it would have scanned the wrong directory if ever wired up.workspace_manager.check_file_is_model— referenced only byscan_dir_concur; dead as a pair once that goes.load_yamlblock inworkspace_manager.py(~23 lines) — pure noise. The livesave_yamlimmediately below it (which shares a similar header comment) is real and is left untouched.renderer.Renderer.flush_throttled— 0 callers (its siblingthrottled_eventhas real callers and stays).glyphs.glyph_only— 0 callers (onlystatus_glyphis used).branding.cli_footer— 0 callers (the sole reference is a doc-comment intest_panels.pynoting "no separatecli_footer").Judgment call
Removing
scan_dir_concurorphaned two imports that were used only by it —import concurrent.futuresandfrom pathlib import Path. I removed those too (verified via grep they had no other use in the file), otherwise the linter would flag them as unused. This keeps the change to a clean, lint-green removal.Testing
ruff check comfy_cli/— cleanruff format --checkon all touched files — already formattedpytestsuite — 2455 passed, 36 skippedNo behavior change.