Fix Elixir guarded and multi-clause function

[Bakul2006]

Pull Request

Linked issue

Closes #979

What & why

Fixes two Elixir function graphing issues:

• Guarded def functions using when were silently dropped because their
  function heads are wrapped in a binary_operator node. The parser now
  recursively unwraps when expressions to find the actual function head.
• Multi-clause functions were deduplicated by qualified name, causing the last
  clause to overwrite the earlier clause's line range. Function/Test nodes now
  preserve the minimum line_start and maximum line_end across clauses.

Added regression coverage for guarded function calls, zero-arity functions,
and multi-clause function ranges.

How it was tested

uv run pytest tests/test_multilang.py --tb=short -q
uv run pytest tests/ --tb=short -q
uv run ruff check code_review_graph/
uv run mypy code_review_graph/ --ignore-missing-imports --no-strict-optional
uv run python -m py_compile code_review_graph/parser.py code_review_graph/graph.py
git diff --check

[github-actions]

code-review-graph review

Overall risk: 0.55 (MEDIUM) — 12 changed function(s)/class(es), 28 affected flow(s), 5 test gap(s)

Risk-scored changes

Risk	Level	Symbol	Location	Tested
0.55	medium	code_review_graph/graph.py::GraphStore	code_review_graph/graph.py:673	yes
0.35	low	code_review_graph/graph.py::GraphStore._replace_file_data	code_review_graph/graph.py:1007	no
0.30	low	code_review_graph/graph.py::GraphStore.upsert_node	code_review_graph/graph.py:779	yes
0.30	low	code_review_graph/parser.py::CodeParser	code_review_graph/parser.py:2935	yes
0.20	low	code_review_graph/parser.py::CodeParser.function_head	code_review_graph/parser.py:8041	no
0.15	low	code_review_graph/parser.py::CodeParser._elixir_function_name_and_params	code_review_graph/parser.py:8031	no
0.05	low	tests/test_graph.py::TestGraphStore	tests/test_graph.py:23	no
0.05	low	tests/test_graph.py::TestGraphStore.test_upsert_function_preserves_full_line_range	tests/test_graph.py:73	(test)
0.05	low	tests/test_multilang.py::TestElixirParsing	tests/test_multilang.py:2673	no
0.05	low	tests/test_multilang.py::TestElixirParsing.test_guarded_functions_are_parsed_and_called	tests/test_multilang.py:2734	(test)

Affected execution flows

• main — criticality 0.53, 53 node(s) across 3 file(s)
• dump_database — criticality 0.49, 11 node(s) across 3 file(s)
• populate_stub_embeddings — criticality 0.43, 11 node(s) across 3 file(s)
• visit_FunctionDef — criticality 0.41, 55 node(s) across 1 file(s)
• visit_AsyncFunctionDef — criticality 0.41, 55 node(s) across 1 file(s)
• ...and 23 more affected flow(s)

Test gaps

• code_review_graph/graph.py::GraphStore._replace_file_data (code_review_graph/graph.py:1007)
• code_review_graph/parser.py::CodeParser._elixir_function_name_and_params (code_review_graph/parser.py:8031)
• code_review_graph/parser.py::CodeParser.function_head (code_review_graph/parser.py:8041)
• tests/test_graph.py::TestGraphStore (tests/test_graph.py:23)
• tests/test_multilang.py::TestElixirParsing (tests/test_multilang.py:2673)

Token savings: this graph-backed report used ~264,686 fewer tokens (~90%) than reading every changed file in full (estimated, chars/4 approximation).

---

Powered by code-review-graph — local-first analysis; no code leaves the CI runner.

[tirth8205]

This fails the checks once merged into staging.

Merged into origin/staging (clean, ort strategy). pytest (py3.13): "FAILED tests/test_multilang.py::TestElixirParsing::test_multi_clause_functions_keep_the_full_graph_range / 1 failed, 3224 passed, 9 skipped, 2 xpassed, 1 warning in 112.79s". Failure: "assert (5, 5) == (4, 5)" at tests/test_multilang.py:2769. ruff: "All checks passed!". mypy: "Success: no issues found in 72 source files".

To fix:

• Rebase onto current staging and apply the MIN/MAX line-range merge to the ON CONFLICT clause in GraphStore._replace_file_data (code_review_graph/graph.py ~line 446-470), which is the writer used by build/update/forget since PR perf(graph): batch edge/node writes and bare-endpoint resolution #858; without this the multi-clause fix is dead code in the real pipeline...
• Decide and test the semantics of upsert_node when the same Function qualified_name is re-upserted at a new location (probe: 2-4 then 10-12 yields 2-12).
• Add a test that drives the multi-clause range through store_file_batch as well as store_file_nodes_edges.
• Add parser tests for compound guards (when a and b), stacked guards (when g1 when g2), guarded defp/defmacro, and a guarded zero-arity def; all pass on manual probe but nothing pins them.

Merge origin/staging into your branch first to reproduce it.

PRs now target staging, not main. Yours was retargeted already, so nothing to do there.

[Bakul2006]

Ok @tirth8205 I will look into it in a while

[tirth8205]

You pushed b6c6c62 after saying you would look into it, and it covers all four items. The failure is gone.

Gates on b6c6c62 merged into staging 9c13f4c: 3343 passed, 9 skipped, 2 xpassed, ruff clean, mypy clean. test_multi_clause_functions_keep_the_full_graph_range passes through both writers now. All 16 CI checks green.

Item by item:

• The MIN/MAX merge is on the ON CONFLICT in _replace_file_data, so the multi-clause fix is live on build, update and forget rather than dead code behind upsert_node.
• The upsert_node semantics question is answered and pinned: 2-4 then 10-12 gives 2-12, asserted by test_upsert_function_preserves_full_line_range. Union is the right answer here, and it is safe in practice because _replace_file_data deletes the file's rows before inserting, so a function that moves cannot inherit a stale start, and the only production caller of upsert_node is event_resolver.py writing virtual Event nodes at 0,0.
• store_file_batch is now driven by the same range test.
• The five guard forms are parametrised and pass: compound when a and b, stacked when g1 when g2, guarded defp, guarded defmacro, and a guarded zero-arity def.

One nit, not blocking. The two writers now disagree for non-Function kinds. upsert_node guards the MIN/MAX with WHEN excluded.kind IN ('Function', 'Test') AND nodes.kind IN ('Function', 'Test'); _replace_file_data applies it unconditionally. Measured on a C# file with partial class Foo split across lines 2 and 6:

staging, store_file_nodes_edges : Foo 6,6
yours,   store_file_nodes_edges : Foo 2,6
yours,   upsert_node            : Foo 6,6

2,6 is arguably the better answer for a partial class, but it is a side effect of dropping the guard rather than a decision anyone made, and it is untested. Pick one, apply it to both writers, and pin it with a test. That can be a follow-up.

Queued for merge.

Removing checks-failing.

[Bakul2006]

Yeah thanks for the review @tirth8205 Just a follow up from my side Would love to connect with you if you do allow as per your time on linkedin

[tirth8205]

The MIN/MAX line-range merge on the ON CONFLICT in _replace_file_data, code_review_graph/graph.py:1051, corrupts line ranges in every language, not just Elixir. I built psf/requests on both branches and diffed the tables row by row. Node and edge counts are identical and every edge row is byte-identical, but 21 node rows widened their range, claiming 907 extra source lines between them.

tests/test_utils.py::RegHandle                    991-993 -> 894-993
tests/test_utils.py::OpenKey                      997-998 -> 901-998
tests/test_lowlevel.py::digest_response_handler   251-261 -> 152-261
src/requests/models.py::_encode_params            151-180 -> 134-180

class RegHandle is defined twice in tests/test_utils.py, once inside each of two different test functions, at 894 and at 991. The writer sees two rows with the same qualified_name and welds them. It cannot do better. qualified_name is only file_path::parent.identity_name, graph.py:3513, so the SQL cannot tell sibling clauses of one Elixir function from two unrelated definitions. Before this PR the last writer won and the node carried one real definition's range. Now it carries a span that matches no definition at all.

That reaches the read path. map_changes_to_nodes, changes.py:557, attributes a hunk to a node by line-range overlap alone. A one-line edit at tests/test_utils.py:940 maps to 2 nodes on staging and to 6 on this branch. OpenKey, QueryValueEx, RegHandle and RegHandle.Close come back as changed, and they live at 991-1007. detect_changes_tool, risk scores, affected flows and the span widening in tools/review.py:296 all read those ranges.

The queued follow-up does not fix this. upsert_node's guard is excluded.kind IN ('Function','Test') AND nodes.kind IN ('Function','Test'), and 19 of the 21 welded nodes are Functions, so the guarded form welds them too.

Second problem, in the parser. function_head at parser.py:8041 unwraps the when binary_operator and keeps only its left, so a short-form body is never walked.

def guarded(x) when is_binary(x), do: helper(x)

staging misses the guarded node but records a CALLS edge to N.helper. This branch creates the node and drops the edge. On plug that is 11 resolved intra-project call-sites. test_guarded_functions_are_parsed_and_called uses a do ... end body, so nothing covers the short form.

Third, guard calls such as is_binary and user-defined defguard macros no longer produce CALLS edges at all. Defensible, but silent and untested.

The Elixir fix itself is right. On plug it recovers 29 functions that were invisible and 152 correct CONTAINS edges, and most of the 294 lost CALLS are phantom def-head self-calls and guard predicates. Gates are clean at 4263 passed, and the one mypy error is pre-existing on staging.

To merge: move the clause merge into the Elixir path in the parser, where sibling clauses are known to be siblings, and leave both ON CONFLICT clauses writing excluded.line_start and excluded.line_end as staging does. If it has to stay in SQL, guard it far more narrowly than by kind and add a test in a non-Elixir language asserting that two same-name definitions in one file do not weld. Then walk the , do: body after function_head unwraps the guard, and pin it with a test.