Skip to content

docs: state graphify path's directed default in the top-level help - #4229

Closed
wanjinhao1 wants to merge 1 commit into
Graphify-Labs:v8from
wanjinhao1:agent/fix-3712-path-directed-default
Closed

wanjinhao1 wants to merge 1 commit into
Graphify-Labs:v8from
wanjinhao1:agent/fix-3712-path-directed-default

Conversation

@wanjinhao1

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #3712.

graphify path runs a directed search by default and never consults the graph.json directed flag (deliberate per #2487), but the top-level help listed neither --directed/--undirected nor the default, while diagnose multigraph documents the opposite default (follows the JSON flag). Reading the two side by side, a correct "no directed path found" result looks like a bug.

This PR adds three help lines to the path entry in graphify/__main__.py so the flags and the directed default (and the fact that the JSON directed flag is not consulted) are visible without triggering the usage error.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Tests or CI
  • Refactor
  • Security fix

Verification & Invariants

  • Read the CONTRIBUTING.md guide.
  • Reproduced the issue and identified the invariant. — Confirmed via the issue's transcript: graphify path on a "directed": false graph runs directed and prints "Re-run with --undirected", while -h output for path stated no default.
  • Made the smallest fix necessary. — Help text only, 3 lines; no behavior change.
  • Added a regression test (if bug fix) or isolated boundary test. — Documentation-only change (issue: "This is a documentation issue only"); no behavior to test.
  • Kept the PR description synchronized with the final implementation.
  • Documented any limitations / unsupported cases explicitly. — The note explicitly says the JSON directed flag is NOT consulted.

How was this tested?

$ python3 -m graphify --help   (repo checkout, v8)
  path "A" "B"            shortest path between two nodes in graph.json
    --graph <path>          path to graph.json (default graphify-out/graph.json)
    --directed              force a directed search (the default;
                            the JSON 'directed' flag is NOT consulted)
    --undirected            ignore edge direction
$ python3 -c "import ast; ast.parse(open('graphify/__main__.py').read())"
syntax ok

No code path changed, so the existing test suite is unaffected; grep confirms no test asserts on the path help block.

Graphify-specific checklist

  • I updated generated skill artifacts (uv run python -m tools.skillgen --bless) when changing their source fragments. — N/A: only the hand-written CLI help in __main__.py changed; no skill source fragments were touched.
  • I confirmed that AST/structural extraction remains deterministic (no ambient state dependencies like ENV variables). — N/A, no extraction change.
  • I reviewed changes for security implications (no unsafe interpolation into shell/Python). — Static string literals only.
  • I confirmed no API keys or local-only graph data are included.
  • (If applicable) I disclosed AI authorship in my commit messages. — See disclosure below.

AI assistance disclosure: this change was authored with AI coding assistance on behalf of the account owner (@wanjinhao1).

@wanjinhao1
wanjinhao1 requested a review from safishamsi as a code owner October 8, 2026 12:05
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

Thanks for the pull request, @wanjinhao1. A maintainer will review it soon.

Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions.

A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic.

@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. PR-changed functions: 1/1 verified (0 proven, 1 may-equivalent, 0 distinguished) · 0 not verified.


Graphify review — findings

Documents the --directed and --undirected flags for path in the _run_cli help text. It notes that directed search is the default and the graph JSON's directed flag is not consulted.

No blocking issues surfaced.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 247 functions depend on the 24 functions this change touches.

Health — this change adds coupling hotspots:

  • new: main() — 102 callers, 3 callees
  • new: dispatch_command() — 2 callers, 129 callees
  • new: _refresh_stale_skills() — 25 callers, 4 callees
  • new: _run_cli() — 6 callers, 7 callees
  • new: _check_skill_version() — 9 callers, 3 callees
  • new: test_poisoned_manifest_is_healed() — 0 callers, 6 callees

Verification — 247 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: 247 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

23 of 344 test file(s) selected (7%) via static blast radius.

  • tests/test_affected_cli.py — impact
  • tests/test_agents_platform.py — impact
  • tests/test_codebuddy.py — impact
  • tests/test_dedup_shrink_refuses_force_write.py — impact
  • tests/test_devin.py — impact
  • tests/test_explain_cli.py — impact
  • tests/test_extract_cli.py — impact
  • tests/test_global_add_tag_inference.py — impact
  • tests/test_god_nodes_cli.py — impact
  • tests/test_hollow_chunks_arm_shrink_guard.py — impact
  • tests/test_incomplete_build_guard.py — impact
  • tests/test_install.py — impact
  • tests/test_install_references.py — impact
  • tests/test_install_version_warning.py — impact
  • tests/test_merge_chunks_validation.py — impact
  • tests/test_multigraph_diagnostics.py — impact
  • tests/test_no_dedup_flag.py — impact
  • tests/test_path_cli.py — impact
  • tests/test_query_cli.py — impact
  • tests/test_query_induced_edges.py — impact
  • tests/test_skill_auto_refresh.py — impact
  • tests/test_skill_version_warning.py — impact
  • tests/test_unverified_semantic_shrink.py — impact

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

Formal verification

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.

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

@safishamsi

Copy link
Copy Markdown
Member

Landed in v0.9.81 via an authorship-preserving cherry-pick, so your commit is on v8 with you credited as the author. Closing as shipped — thanks @wanjinhao1!

@safishamsi safishamsi closed this Oct 8, 2026
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.

docs: path is directed by default but the top-level help never says so, and diagnose multigraph documents the opposite default for the same JSON flag

2 participants