Skip to content

chore(workspace): remove dead defensive block in load_skills AST gate - #2554

Merged
HongmingWang-Rabbit merged 1 commit into
stagingfrom
chore/remove-dead-ast-defensive-block
May 3, 2026
Merged

chore(workspace): remove dead defensive block in load_skills AST gate#2554
HongmingWang-Rabbit merged 1 commit into
stagingfrom
chore/remove-dead-ast-defensive-block

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

Self-review of #2553 (#119 PR-4) caught an unreachable defensive block at test_load_skills_call_sites.py:99-103:

for call in _find_load_skills_calls(tree):
    if call.func.__class__.__name__ == "Name" and call.func.id == "load_skills":
        # Skip the function DEFINITION itself (it appears as a
        # FunctionDef, not a Call — but the Call check ensures
        # we only trip on actual invocations). Defensive.
        pass
    if not _has_current_runtime_kwarg(call):

The inner check guarded against FunctionDef, but _find_load_skills_calls already restricts its return type to ast.Call — a FunctionDef cannot reach that loop body. The block is a no-op pass with a misleading comment that implies the check matters for correctness when it doesn't.

Removing keeps the gate behaviorally identical; both tests still pass (pytest workspace/tests/test_load_skills_call_sites.py → 2 passed).

The five-axis review that turned this up also approved the substantive logic of #2553 — this is the only follow-up cleanup it surfaced.

Test plan

  • pytest workspace/tests/test_load_skills_call_sites.py → 2 passed
  • Diff is -5 lines, no behavior change

🤖 Generated with Claude Code

Self-review of PR #2553 caught an unreachable defensive block at
test_load_skills_call_sites.py:99-103: the inner check guarded
`call.func.__class__.__name__ == "Name"` from a FunctionDef, but
`_find_load_skills_calls` already filters its return type to
`ast.Call` — `FunctionDef` cannot reach that loop body. The block
was a no-op `pass` with a misleading comment.

Removing keeps the gate behaviorally identical; tests still pass.

Same five-axis review pass that turned this up also approved the
substantive logic of #2553, so no behavior change here.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@HongmingWang-Rabbit
HongmingWang-Rabbit added this pull request to the merge queue May 3, 2026
Merged via the queue into staging with commit 872f8e8 May 3, 2026
20 checks passed
@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the chore/remove-dead-ast-defensive-block branch May 3, 2026 08:37
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
…phan-goroutine race (#2490)' (#2554) from fix/core-2490-bootstrapfailed-rescue-race into main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant