Fix: clicking into a filtered-out community no longer dies silently (#3378) - #3457
Fix: clicking into a filtered-out community no longer dies silently (#3378)#3457shivamtiwari3 wants to merge 1 commit into
Conversation
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
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) alter behavior, breaking input(s) attached.
Behavior changes: \_html\_script changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_html\_script behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"nodes\_json":"''","edges\_json":"''","legend\_json":"'\\\\x00'"\}, the old code produced '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return… but the new code produces '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return…. Paste that input straight into a regression test.
Graphify review — findings
Reveals a filtered-out community when you click a neighbor link or search hit into it, instead of leaving the canvas untouched while only the NODE INFO panel updates — focusNode now checks whether the target's community is in hiddenCommunities and, if so, re-ticks that community's legend checkbox and fires its own change handler before calling focus()/selectNodes(), so only that one community is revealed and the rest of the user's filter survives. Routes the search-result click through focusNode (keeping its 1.5 scale) rather than a duplicated copy of its body, and registers each legend checkbox in a new CB_BY_CID map so the reveal reuses the checkbox's existing handler. Adds a Node-based harness with a stubbed DOM/vis that models a hidden node's focus() as a no-op to assert the reveal, selection, and legend tri-state behavior.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 636 functions depend on the 340 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 115 callers, 51 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
to_html()— 17 callers, 11 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
watch()— 5 callers, 7 callees - new:
_reconcile_graph_html()— 6 callers, 5 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 636 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: 361 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
262 of 262 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— full-run-safetytests/test_cross_language_call_resolution.py— full-run-safetytests/test_cross_repo_member_calls.py— full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— full-run-safetytests/test_csharp_enum_members.py— full-run-safetytests/test_csharp_field_generic_args.py— full-run-safetytests/test_csharp_generic_callsites.py— full-run-safety- … and 212 more
non-code file(s) changed (
CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests
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
Behavior changes: \_html\_script changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_html\_script behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"nodes\_json":"''","edges\_json":"''","legend\_json":"'\\\\x00'"\}, the old code produced '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return… but the new code produces '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return…. Paste that input straight into a regression test.
· 7 more finding(s) on lines outside this diff (see the check run).
…raphify-Labs#3378) The community filter hides nodes rather than removing them, and vis has no rendered position for a hidden node. So for a target in an unticked community, two of focusNode()'s three actions were no-ops: focus() did nothing and selectNodes() selected something invisible. Only showInfo() worked, because it reads the underlying data. The panel navigated to the new node and listed its neighbours while the canvas sat still, with nothing saying why — a dead click. focusNode now reveals the target's community first, and does it by firing that community's legend checkbox's own change handler rather than duplicating its body, so hiddenCommunities, the dimmed styling, nodesDS and the Select-All tri-state all stay consistent through one code path. Only the target's community is revealed; the rest of the filter the user set up survives the click. The search-result handler carried its own copy of focusNode's body and had the identical defect — a search hit can land in a filtered-out community just as a neighbour can. It goes through focusNode now, which takes the zoom as a parameter so search keeps the 1.5 it has always used. Tests execute the emitted viewer script under node against a stub DOM and vis, following test_hyperedge_convex_hull_js_is_geometrically_sound. The stub models focus() the way real vis behaves for a hidden node — a silent no-op — so what the tests assert is observable browser state: what the camera focused, what is selected, which nodes are hidden, and the legend's own checkboxes and tri-state. Setup addresses the checkboxes through the DOM rather than through the map this fix adds, so the pre-fix failure is the reported symptom rather than a ReferenceError.
46c65aa to
3eaa2c6
Compare
There was a problem hiding this comment.
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) alter behavior, breaking input(s) attached.
Behavior changes: \_html\_script changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_html\_script behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"nodes\_json":"''","edges\_json":"''","legend\_json":"'\\\\x00'"\}, the old code produced '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return… but the new code produces '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return…. Paste that input straight into a regression test.
Graphify review — findings
Fixes dead clicks in graph.html when the target's community is filtered out: focusNode now reveals that one community by firing its legend checkbox's change handler before calling network.focus/selectNodes, so hiddenCommunities, the dimmed styling, nodesDS, and the Select-All tri-state all stay consistent through the existing code path. Search-result clicks route through focusNode (at their usual 1.5 scale) instead of duplicating its body, so they too can reach a hidden node, and a new CB_BY_CID map lets focusNode find each community's checkbox. Only the clicked node's community is un-hidden, leaving the rest of the user's filter intact.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 636 functions depend on the 340 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 115 callers, 51 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
to_html()— 17 callers, 11 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
watch()— 5 callers, 7 callees - new:
_reconcile_graph_html()— 6 callers, 5 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 636 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: 361 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
262 of 262 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— full-run-safetytests/test_cross_language_call_resolution.py— full-run-safetytests/test_cross_repo_member_calls.py— full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— full-run-safetytests/test_csharp_enum_members.py— full-run-safetytests/test_csharp_field_generic_args.py— full-run-safetytests/test_csharp_generic_callsites.py— full-run-safety- … and 212 more
non-code file(s) changed (
CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests
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
Behavior changes: \_html\_script changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_html\_script behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"nodes\_json":"''","edges\_json":"''","legend\_json":"'\\\\x00'"\}, the old code produced '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return… but the new code produces '\<script\>\\nconst RAW\_NODES = ;\\nconst RAW\_EDGES = ;\\nconst LEGEND = \\x00;\\n\\n// HTML\-escape helper — prevents XSS when injecting graph data into innerHTML\\nfunction esc\(s\) \{\\n return…. Paste that input straight into a regression test.
· 7 more finding(s) on lines outside this diff (see the check run).
1 participant
Fixes #3378.
The bug
The community filter hides nodes rather than removing them, and vis has no rendered position for a hidden node. So for a target in an unticked community, two of
focusNode()'s three actions were no-ops:network.focus(nodeId, …)network.selectNodes([nodeId])showInfo(nodeId)The panel navigated to the new node and listed its neighbours while the canvas sat still, with nothing saying why. A dead click.
The fix
focusNodereveals the target's community first, and does it by firing that community's legend checkbox's ownchangehandler rather than duplicating its body — sohiddenCommunities, thedimmedstyling,nodesDSand the Select-All tri-state all stay consistent through one code path. This is the approach @evaldnet suggested and had been running locally.Only the target's community is revealed; the rest of the filter the user set up survives the click. The issue also offered an alternative (annotate/disable links into hidden communities instead), but making the click work seems strictly better than explaining why it can't — happy to switch if you'd rather the filter never change implicitly.
One thing beyond the report: the search-result handler carried its own copy of
focusNode's body and had the identical defect — a search hit can land in a filtered-out community just as a neighbour can. It goes throughfocusNodenow, which takes the zoom as a parameter so search keeps the1.5it has always used and neighbour focus keeps1.4.Tests
Four tests in
tests/test_export.py. Three of them execute the emitted viewer script undernodeagainst a stub DOM and vis, following the precedent oftest_hyperedge_convex_hull_js_is_geometrically_sound(skipped whennodeis absent).The stub models
focus()the way real vis behaves for a hidden node — a silent no-op — so what the tests assert is observable browser state: what the camera focused, what is selected, which nodes are hidden, and the legend's own checkboxes and tri-state. Test setup addresses the checkboxes through the DOM rather than through the map this fix adds, so the pre-fix failure is the reported symptom (assert None == 'b'— "focus() was still refused") rather than aReferenceError.focusNode's body3 of the 4 fail on unpatched code. Full suite:
5497 passed, with only the 3 pre-existing environment-dependent failures on this tree (test_extract_code_only_cli, 2×test_ollama) unchanged.🤖 Generated with Claude Code