Skip to content

fix(serve): get_node/get_neighbors accept node_id/id, not just label - #3725

Closed
ahm3dwasim wants to merge 1 commit into
Graphify-Labs:v8from
ahm3dwasim:fix/node-lookup-accept-node-id-alias
Closed

ahm3dwasim wants to merge 1 commit into
Graphify-Labs:v8from
ahm3dwasim:fix/node-lookup-accept-node-id-alias

Conversation

@ahm3dwasim

Copy link
Copy Markdown

What

get_node and get_neighbors read arguments["label"] directly, so a client that passes the node identifier under node_id (or id) gets KeyError('label') instead of a result:

Error executing get_neighbors: 'label'

Several of the agent frameworks graphify advertises support for send the argument under node_id, so these two tools are unusable from those clients — while query_graph, god_nodes, shortest_path, etc. work fine against the same graph. It reads as "graphify is broken" when only the node-lookup tools are.

Change

  • Add a small module-level _node_arg(arguments) that accepts label / node_id / id (label wins if several are present; non-str coerced).
  • Both handlers return a short guidance string when no identifier is given, instead of raising.
  • Declare node_id in both tools' inputSchema so the parameter is discoverable, and drop the hard required: ["label"] (either key is now valid).
  • Unit test test_node_arg_accepts_label_node_id_and_id_aliases.

Backward compatible: existing label callers are unchanged.

Test

pytest tests/test_serve.py -k node_arg

The two node-lookup MCP tools read arguments["label"] directly, so a client
that passes the node identifier under "node_id" (or "id") gets a
KeyError('label') instead of a result. Several of the agent frameworks graphify
advertises support for send the argument that way, so the tools are unusable
from those clients while query_graph/god_nodes work fine against the same graph.

Extract a small module-level _node_arg() that accepts label/node_id/id, return
a short guidance string when none is given (instead of raising), and declare
node_id in both inputSchemas so the parameter is discoverable. Add a unit test.

@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).


Graphify review — findings

Accepts node_id and id as aliases for label on the get_node and get_neighbors tools via the new _node_arg helper, so clients that pass the node under those keys are served instead of hitting KeyError('label'). Drops label from the tools' required schema and returns a guidance message (listing the accepted keys) when no identifier is given rather than raising.

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

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 615 functions depend on the 354 functions this change touches.

Health — this change adds coupling hotspots:

  • new: _query_graph_text() — 27 callers, 10 callees
  • new: dispatch_command() — 2 callers, 125 callees
  • new: _score_query() — 15 callers, 7 callees
  • new: _query_terms() — 20 callers, 3 callees
  • new: run_benchmark() — 16 callers, 3 callees
  • new: _load_graph() — 16 callers, 3 callees
  • new: _build_server() — 2 callers, 16 callees
  • new: _query_subgraph_tokens() — 7 callers, 3 callees
  • …and 10 more — each is listed as a finding

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

Test selection

Test selection

9 of 290 test file(s) selected (3%) via static blast radius.

  • tests/test_benchmark.py — impact
  • tests/test_benchmark_raw_graph.py — impact
  • tests/test_file_label_disambiguation.py — impact
  • tests/test_query_induced_edges.py — impact
  • tests/test_query_mcp_direction.py — impact
  • tests/test_query_names_its_graph.py — impact
  • tests/test_serve.py — impact, changed-test
  • tests/test_serve_http.py — impact
  • tests/test_terraform.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.

· 1 grounded finding(s) anchored inline below; 17 more finding(s) on lines outside this diff (see the check run).

Comment thread graphify/serve.py
@@ -2079,7 +2096,10 @@ def _tool_get_node(arguments: dict) -> str:
])

def _tool_get_neighbors(arguments: dict) -> str:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Health regression_tool_get_neighbors()

fans out to 6 callees (efferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

@safishamsi

Copy link
Copy Markdown
Collaborator

Shipped in v0.9.66 (on PyPI). Cherry-picked with authorship preserved so it shows under your GitHub contributions. Thanks @ahm3dwasim!

@safishamsi safishamsi closed this Sep 22, 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.

2 participants