Skip to content

fix(cli): restore detailed subcommand help - #2944

Open
oleksii-tumanov wants to merge 4 commits into
Graphify-Labs:v8from
oleksii-tumanov:fix/cli-subcommand-help
Open

fix(cli): restore detailed subcommand help#2944
oleksii-tumanov wants to merge 4 commits into
Graphify-Labs:v8from
oleksii-tumanov:fix/cli-subcommand-help

Conversation

@oleksii-tumanov

Copy link
Copy Markdown
Contributor

Summary

  • route -h and --help for tree, reflect, prs, and export callflow-html to their existing detailed help handlers before option parsing
  • keep -?, free-text behavior, and commands without safe detailed handlers behind their existing boundaries
  • correct the Bug: Inconsistent help-flag handling across graphify subcommands #821 CHANGELOG wording so it no longer claims that the universal guard covers free-text commands
  • add no-side-effect regression coverage for bare, valid option-before-help, malformed value-position, and guarded help paths

Before

Commands such as graphify tree --help returned only the top-level usage pointer even though the subcommand already had detailed help.

After

The four existing handlers now show detailed help before parsing option values, while unrelated commands retain the universal guard's current behavior.

Scope: This PR addresses Part 1 only. Part 2 changes free-text input semantics and remains open for separate UX discussion.

Testing

  • pytest tests/test_cli_help.py -q (19 passed)
  • affected CLI, PR, tree, reflect, and export tests (330 passed)
  • pytest tests/ -q (4900 passed, 11 skipped)
  • ruff check .
  • python -m tools.skillgen --check
  • graphify update .

Related to #2404

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. 1 change(s) tested, no difference found (not proven).


Graphify review — findings

Routes -h/--help for tree, reflect, prs, and export callflow-html to their detailed help handlers before argument parsing via a new _has_detailed_help_handler check in _run_cli, so these tokens print full usage instead of hitting the universal short-circuit guard. Moves the help checks in each command's dispatch to the top of parsing so a trailing --help after a value-expecting flag still works, and adds tests/test_cli_help.py covering both the reachable handlers and the tokens still behind the guard (-?, benchmark, export html).

No blocking issues surfaced. 6 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 571 functions depend on the 307 functions this change touches.

Health — this change adds coupling hotspots:

  • new: dispatch_command() — 2 callers, 119 callees
  • new: _stale_graph_sources() — 7 callers, 6 callees
  • new: _build_server() — 2 callers, 16 callees
  • new: _run_hook_guard() — 4 callers, 7 callees
  • new: cmd_prs() — 2 callers, 12 callees
  • new: _status_color() — 4 callers, 5 callees
  • new: fetch_prs() — 5 callers, 4 callees
  • new: _main() — 4 callers, 3 callees
  • …and 7 more — each is listed as a finding

Verification — 571 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 550 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify dispatch\_command.

The verifier did not have enough to check dispatch\_command, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)

No difference found (not proven): No behavior difference found in \_run\_cli (not a proof).

The verifier ran both versions of \_run\_cli on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

Could not verify: Could not verify cmd\_prs.

The verifier did not have enough to check cmd\_prs, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

· 15 more finding(s) on lines outside this diff (see the check run).

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