fix(tests): isolate backend-detection tests from the developer's environment - #3484
fix(tests): isolate backend-detection tests from the developer's environment#3484L4XB 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).
Graphify review — findings
Isolates backend-detection tests from the developer's shell: a new autouse fixture in conftest.py clears every environment variable detect_backend() consults before each test, so ambient keys like GOOGLE_API_KEY can no longer steer which backend is detected. Adds backend_detection_env_vars() to enumerate that full set — all registered backends' API keys plus the endpoint/region/host probes tracked in _BACKEND_DETECTION_EXTRA_ENV — which both the fixture and new guard tests consume to keep the list complete.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1041 functions depend on the 393 functions this change touches.
Health — this change adds coupling hotspots:
- new:
deduplicate_entities()— 75 callers, 22 callees - new:
build_merge()— 65 callers, 14 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
_call_claude_cli()— 33 callers, 9 callees - new:
extract_corpus_parallel()— 26 callers, 11 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
_extract_with_adaptive_retry()— 22 callers, 10 callees - new:
_call_llm()— 11 callers, 18 callees - …and 16 more — each is listed as a finding
Verification — 1041 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: 724 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
267 of 267 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_env_isolation.py— impact, changed-test, full-run-safetytests/test_backend_extras.py— impact, full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— impact, full-run-safetytests/test_build_merge_hyperedges_and_prune.py— impact, full-run-safetytests/test_build_merge_shrink_guard.py— impact, 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— impact, full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— impact, full-run-safetytests/test_chunking.py— impact, full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— impact, 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— impact, 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— impact, 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-safety- … and 217 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,tests/conftest.py) — 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.
· 1 grounded finding(s) anchored inline below; 23 more finding(s) on lines outside this diff (see the check run).
| return tuple(seen) | ||
|
|
||
|
|
||
| def detect_backend() -> str | None: |
There was a problem hiding this comment.
detect_backend()
17 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Autouse conftest fixture clears every backend env var detect_backend() reads (API keys, Azure/AWS/Ollama), with a self-guard test that the cleared list covers every variable the detector reads. (#3484)
…ronment detect_backend() probes whatever the shell exports, and each detection test cleared only a hand-picked subset of the dozen variables it reads. With GOOGLE_API_KEY set, three of the four test_ollama.py::test_detect_backend_* tests fail on a clean checkout; other variables break one or two, and the Azure pair or OLLAMA_HOST also break a test in test_provider_registry.py. CI never sees it because no workflow exports any of them. Add backend_detection_env_vars() next to detect_backend(): the API-key variables of every registered backend plus the endpoint/region/host variables it checks directly. An autouse fixture in tests/conftest.py clears that whole set before each test, so a test that wants a backend sets it up explicitly and nothing ambient can steer the result. tests/test_backend_env_isolation.py keeps the list complete (every os.environ.get in detect_backend()/_resolve_ollama_base_url and every built-in backend key must be listed), checks the fixture leaves a clean environment, and re-runs the detection tests in a subprocess with every variable exported. Fixes Graphify-Labs#3481
576ab4b to
2e2ca61
Compare
|
Rebased onto |
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).
Graphify review — findings
Adds a changelog entry documenting a test fix where an autouse fixture clears every environment variable detect_backend() reads (all backend API keys, AZURE_OPENAI_ENDPOINT, AWS_*, OLLAMA_BASE_URL/OLLAMA_HOST), so locally exported vars like GOOGLE_API_KEY no longer cause spurious failures in backend-detection tests.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 203 functions depend on the 203 functions this change touches.
Health — grade A; no new coupling hotspots.
Verification — 203 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: 203 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
268 of 268 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_env_isolation.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-safety- … and 218 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.
1 participant
Fixes #3481.
detect_backend()probes whatever the shell exports, and each detection test cleared only a hand-picked subset of the dozen variables it reads. Onv8withGOOGLE_API_KEY=dummyexported,pytest tests/test_ollama.py -k detect_backendgives3 failed, 1 passed; other variables break one or two tests, and the Azure pair orOLLAMA_HOSTalso breaktest_provider_registry.py::test_detect_backend_custom_provider_after_builtins. CI never sees it because no workflow exports any of them.Change
graphify/llm.py:backend_detection_env_vars()next todetect_backend()returns every variable it consults: the API-key variables of every registered backend (built-in and custom, via_backend_env_keys) plusAZURE_OPENAI_ENDPOINT,AWS_PROFILE/AWS_REGION/AWS_DEFAULT_REGION,OLLAMA_BASE_URL/OLLAMA_HOST. The extra list sits directly above the function so a new probe gets added in one place.tests/conftest.py: autouse fixture_isolate_backend_envclears that whole set before each test (same pattern as the existing_sandbox_home). Tests that want a backend still set it withmonkeypatch.setenv, which runs after the fixture.tests/test_backend_env_isolation.py: keeps the list complete (everyos.environ.get("...")indetect_backend()/_resolve_ollama_base_urland every built-in backend key must be listed), checks the fixture leaves a clean environment, and re-runs the detection tests in a subprocess with every variable exported to a dummy value (the report's repro, widened to all 13).Verification
The existing per-test
delenvcalls are left in place; they are now redundant but harmless, and removing them would only add churn.