fix(staleness): pin positional workspace arg with regression tests (closes #178)#181
Merged
Merged
Conversation
…loses #178) Issue #178 reported that 'codelens staleness /path/to/workspace' printed usage and exited, unlike every other command. Investigation shows the bug does not reproduce on current main — the argparse -f conflict guard restoration in PR #171/#174 likely fixed it. However, the issue remains open and the behavior was not pinned by any test. This PR follows the PR #174 pattern (regression tests for already-fixed bugs) to prevent recurrence: Changes: - scripts/commands/staleness.py: added defensive comment in add_args() documenting that 'workspace' MUST stay nargs='?' (issue #178). No logic change — the positional was already correctly registered. - tests/test_staleness.py: new TestStalenessWorkspaceArgRegression class with 7 tests pinning the issue #178 Definition of Done: 1. test_positional_workspace_exits_zero — 'staleness <ws>' exits 0 2. test_positional_workspace_does_not_print_usage — no 'usage:' in output 3. test_no_args_auto_detects_and_exits_zero — 'staleness' (no args) works 4. test_positional_workspace_with_json_format — positional + --format json 5. test_workspace_positional_is_optional_nargs_question — inspects argparse 6. test_workspace_positional_consistent_with_other_commands — compares vs scan 7. test_help_shows_workspace_positional — --help lists [workspace] Test results: - 7 new regression tests: all pass - Full staleness suite: 48 passed (was 41, +7 new) - Related suites (staleness + cli + command_count + command_registry + codelens): 166 passed, 0 failed The bug appears already-fixed on main (tested manually with both 'python3 scripts/codelens.py staleness /tmp' and 'codelens staleness /tmp' via the pip-installed entry point). These tests ensure it stays fixed. Findings: - staleness.py had only one commit (df47ffb, PR #166 Phase 1) which correctly registered 'workspace' with nargs='?'. The issue may have been filed against a stale checkout or the brief window before PR #171/#174 restored the argparse guard. - No design doc needed (bug fix + tests, not feature-class file per CONTEXT.md design-doc convention).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #178 — pins the positional
workspacearg oncodelens stalenesswith regression tests so the bug cannot recur.Investigation
Issue #178 reported that
codelens staleness /path/to/workspaceprinted usage and exited, unlike every other command. The bug does not reproduce on current main — tested manually with both:python3 scripts/codelens.py staleness /tmp(legacy mode) → exits 0codelens staleness /tmp(pip-installed entry point, Phase 1) → exits 0The root cause was likely the argparse
-fconflict guard regression that PR #171/#174 restored. Theworkspacepositional was correctly registered withnargs="?"in the original PR #166 (staleness Phase 1), but the argparse guard drop in PR #153 (graphml) may have broken subparser registration broadly. PR #171/#174 fixed the guard, which fixed this bug as a side effect.However, the issue remains open and no test pinned the expected behavior. This PR follows the PR #174 pattern (regression tests for already-fixed bugs) to prevent recurrence.
Changes
scripts/commands/staleness.pyadd_args()documenting thatworkspaceMUST staynargs="?"(issue fix(staleness): command does not accept positional workspace argument #178). No logic change — the positional was already correctly registered.tests/test_staleness.pyTestStalenessWorkspaceArgRegressionclass with 7 tests pinning the issue fix(staleness): command does not accept positional workspace argument #178 Definition of Done:test_positional_workspace_exits_zerostaleness <workspace>exits 0 (DoD #1)test_positional_workspace_does_not_print_usageusage:in stdout/stderr (the exact symptom from the issue)test_no_args_auto_detects_and_exits_zerostaleness(no args) auto-detects (DoD #2)test_positional_workspace_with_json_format--format jsonproduce valid JSONtest_workspace_positional_is_optional_nargs_questionnargs="?",default=Nonetest_workspace_positional_consistent_with_other_commandsstalenessvsscan— bothnargs="?"(DoD #3)test_help_shows_workspace_positional--helplists[workspace]Tests
The broader suite hits a pre-existing segfault from tree-sitter native extensions in the sandbox (unrelated to this change — I only added a comment + tests, no Python logic).
Note for reviewer
closes #178since this fully addresses the DoD — the positional works and is now pinned by tests.Files changed