fix(label): escalate the token budget before splitting on a blank completion (#3747) - #3755
abhay-codes07 wants to merge 1 commit into
Conversation
…pletion (Graphify-Labs#3747) Community labeling batches a set of communities into one prompt with a completion budget of `min(256 + 48*n, 8192)`. When the reply won't parse, `_label_batch_with_retry` splits the batch in half and retries — which for a smaller batch computes a SMALLER budget. For a reasoning model that is a dead end. Such a model spends its whole completion allowance on the chain-of-thought (returned separately from `content`) and comes back with empty content and finish_reason=length. The empty string is unparseable, so the batch splits, the budget shrinks, and every retry — 1216 → 736 → 496 → 352 tokens in the reporter's trace — returns empty again. Labeling degrades to `Community N` placeholders for the whole run. Escalate the budget before splitting: a blank completion with room left under the 8192 cap is the budget-exhaustion signature, so double the budget and retry the same (whole) batch, and only split once more room stops helping. A non-empty but malformed reply is a parse problem a larger budget won't fix, so that still splits immediately — no extra full-budget calls. The escalation is bounded (it stops at 8192), so a persistently-blank batch still raises and the caller falls back to placeholders rather than looping. Tests: a reasoning model that returns empty until the budget doubles now gets labeled with the batch kept whole (fails without the fix); a non-empty malformed reply still splits without escalating; a batch blank even at 8192 raises rather than looping. The existing labeling/retry/backend suites stay green (154 tests). Disabling thinking via `extra_body` or a CLI budget override (also suggested on the issue) are complementary and left out of this minimal fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q
![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. No changes could be formally verified in this run.
Graphify review — findings
Escalates the community-labeling completion budget before splitting when a batch fails to parse: a blank completion with room left doubles max_tokens up to the 8192 cap and retries the whole batch, since that's the signature of a reasoning model burning its allowance on chain-of-thought (#3747). A non-empty but malformed reply still splits immediately (a bigger budget won't fix it), and a batch that stays blank at the cap raises so the caller falls back to placeholders rather than looping forever.
No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 864 functions depend on the 191 functions this change touches.
Health — this change adds coupling hotspots:
- new:
deduplicate_entities()— 77 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:
extract_corpus_parallel()— 26 callers, 11 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
_extract_with_adaptive_retry()— 22 callers, 10 callees - …and 17 more — each is listed as a finding
Verification — 864 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: 529 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
41 of 290 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— impacttests/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— impact, changed-testtests/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.
Formal verification
Could not verify: Could not verify \_label\_batch\_with\_retry.
The verifier did not have enough to check \_label\_batch\_with\_retry, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
· 1 grounded finding(s) anchored inline below; 24 more finding(s) on lines outside this diff (see the check run).
| return left | right | ||
|
|
||
|
|
||
| def label_communities( |
There was a problem hiding this comment.
label_communities()
20 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Fixes #3747.
What
Community labeling batches communities into one prompt with a completion budget of
min(256 + 48*n, 8192). When a reply won't parse,_label_batch_with_retrysplits the batch and retries each half — which, for a smaller batch, computes a smaller budget.For a reasoning model that is a dead end. The model spends its whole completion allowance on the chain-of-thought (returned separately from
content) and comes back with empty content andfinish_reason=length. The empty string is unparseable, so the batch splits, the budget shrinks, and every retry returns empty again — the reporter traced1216 → 736 → 496 → 352tokens, all empty. Labeling degrades toCommunity Nplaceholders for the entire run, and the split-retry can never recover because it moves the budget in the wrong direction.Fix
Escalate the budget before splitting. A blank completion with room left under the 8192 cap is the budget-exhaustion signature, so double the budget and retry the same whole batch; only split once a larger budget stops helping. A non-empty but malformed reply is a parse problem a bigger budget won't fix, so that still splits immediately — no wasted full-budget calls. The escalation is bounded (stops at 8192), so a persistently-blank batch still raises and the caller falls back to placeholders rather than looping.
Verification
Three tests, driven by a mock
_call_llm(no real backend needed):v8splits to 1+1, both blank, and drops the batch).The existing labeling / retry / backend suites stay green (154 tests).
Scope
This is the minimal, robust fix (budget escalation). The issue's other suggestions — disabling thinking via
extra_body({"thinking": {"type": "disabled"}}/reasoning_effort: "low") and a CLI/env budget override — are complementary and left out here.