fix: honor Claude CLI model env during labeling - #3876
oleksii-tumanov wants to merge 1 commit into
Conversation
Use GRAPHIFY_CLAUDE_CLI_MODEL for plain Claude CLI completions when callers do not pass an explicit model. Preserve explicit-model precedence and the CLI default when the setting is absent. AI assistance: OpenAI Codex was used to help implement and validate this change.
|
Thanks for the pull request, @oleksii-tumanov. 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. |
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
Lets the plain Claude CLI backend pick up a model from GRAPHIFY_CLAUDE_CLI_MODEL when no explicit model is passed to _call_llm, while an explicit model still wins and a blank/whitespace env value falls back to omitting --model entirely. Adds parametrized coverage for the precedence rules and confirms the label batch chain forwards env-only model selection.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 880 functions depend on the 239 functions this change touches.
Health — this change adds coupling hotspots:
- new:
deduplicate_entities()— 79 callers, 24 callees - new:
build_merge()— 76 callers, 14 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
build()— 52 callers, 6 callees - new:
_call_claude_cli()— 33 callers, 9 callees - new:
main()— 98 callers, 3 callees - new:
extract_corpus_parallel()— 26 callers, 11 callees - new:
dispatch_command()— 2 callers, 125 callees - …and 18 more — each is listed as a finding
Verification — 880 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: 543 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
41 of 302 test file(s) selected (14%) via static blast radius.
tests/test_backend_env_isolation.py— impacttests/test_backend_extras.py— impacttests/test_build.py— impacttests/test_build_merge_dedup_scope.py— impacttests/test_build_merge_hyperedges_and_prune.py— impacttests/test_build_merge_shrink_guard.py— impacttests/test_carried_hyperedge_remap.py— impacttests/test_charmap_encoding.py— impacttests/test_chunking.py— impacttests/test_claude_cli_backend.py— impact, changed-testtests/test_corrupt_graph_json.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_dedup.py— impacttests/test_dedup_remaps_hyperedges.py— impacttests/test_dedup_survivor_richness.py— impacttests/test_evidence_binding.py— impacttests/test_file_slice.py— impacttests/test_global_graph.py— impacttests/test_go_qualified_resolution.py— impacttests/test_hyperedge_member_shapes.py— impacttests/test_image_vision.py— impacttests/test_injection_sentinel_coverage.py— impacttests/test_issue_3472_source_file_collision.py— impacttests/test_label_retry.py— impacttests/test_labeling.py— impacttests/test_llm_backends.py— impacttests/test_llm_parser.py— impacttests/test_llm_parser_reasoning.py— impacttests/test_no_dedup_flag.py— impacttests/test_non_string_node_ids.py— impacttests/test_ollama.py— impacttests/test_ollama_retry_cap.py— impacttests/test_oversized_document_slicing.py— impacttests/test_partial_cache.py— impacttests/test_pdf_slicing.py— impacttests/test_pdf_token_estimate.py— impacttests/test_provider_registry.py— impacttests/test_prs.py— impacttests/test_prune_sweeps_orphans.py— impacttests/test_semantic_fragment_sanitize.py— impacttests/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.
· 26 more finding(s) on lines outside this diff (see the check run).
1e8a502 to
109107e
Compare
What does this PR do?
Fixes #3846.
GRAPHIFY_CLAUDE_CLI_MODELalready controls the Claude CLI model used forsemantic extraction, but community labeling and the dedup tiebreaker use the
separate
_call_llmpath. That path ignored the environment setting unless acaller passed an explicit model, so
labelandcluster-onlycould silently usethe Claude CLI default instead.
This change applies the existing precedence at the plain-completion adapter:
an explicit model wins, otherwise a nonempty trimmed environment value is
forwarded, and no
--modelflag is added when neither is configured. Existingexplicit empty-string behavior is preserved.
Type of change
Verification & Invariants
The invariant is that both Claude CLI dispatch paths honor the same configured
model without replacing an explicit caller choice or changing the CLI default
when the setting is absent. The new argv matrix proves those boundaries, and a
label-batch regression proves the environment-only value reaches the real
labeling call chain while valid labels are still parsed. No graph, cache, or
generated format changes.
How was this tested?
The tests mock the Claude executable, so no credential, paid request, or live
model call was used. A fresh all-extras environment and full suite were not
available because dependency downloads timed out; the focused and affected
suites above ran against this checkout with the available local test
environment.
Graphify-specific checklist
uv run python -m tools.skillgen --bless) when changing their source fragments. (No source fragments changed.)