fix(graph): report the effective token window in execution info - #1147
Open
Ebenezer-03 wants to merge 1 commit into
Open
fix(graph): report the effective token window in execution info#1147Ebenezer-03 wants to merge 1 commit into
Ebenezer-03 wants to merge 1 commit into
Conversation
An unknown model falls back to an 8192-token window, which chunks long pages and can change the answer without failing. The only signal was a warning on stderr, which is lost in batch, worker and async contexts, so a caller holding the returned object could not tell a truncating fallback from a real limit. The "TOTAL RESULT" entry of the execution info now carries effective_model_tokens and model_tokens_defaulted. AbstractGraph passes both to the graph it builds, so every graph reports them. Fixes ScrapeGraphAI#1121
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #1121.
When a model isn't in
models_tokens,_create_llmfalls back to an 8192 token window. That value becomes thechunk_sizeused for splitting, so on a long page the model never sees the part that mattered. The run still succeeds and the JSON still validates, so the answer just comes back wrong.The only signal today is a
logger.warningon stderr. That gets lost in batch jobs, workers and anything async, so if all you have is the returned object, you can't tell a truncating fallback from a model whose real limit happens to be 8192.#1126 exposed this as
graph.model_tokens_defaulted. Suggestion (2) in the issue asked for it in the result itself, so downstream code can check it per run without reaching into the graph object. This PR does that half.What changed
Two new keys in the
"TOTAL RESULT"entry of the execution info:effective_model_tokens: the input token window the run actually used for chunkingmodel_tokens_defaulted: true when no limit was known and the 8192 fallback kicked inHow it's wired:
BaseGraph.__init__declaresmodel_token = Noneandmodel_tokens_defaulted = False.AbstractGraph.__init__passes both to the graph right after_create_graph(), so every graph picks them up without per-graph changes._create_llmcan be overridden and doesn't setmodel_tokenon every path, so it reads throughgetattr._execute_standardadds the two keys to the"TOTAL RESULT"entry.get_execution_infodocuments them.Existing keys are untouched, so anything already reading that entry keeps working.
Tests
Four in
tests/graphs/abstract_graph_test.py, next to the ones from #1126:"TOTAL RESULT"AbstractGraphpropagates a defaulted window to its graphAbstractGraphpropagates a known window with the flag offWhat I ran
uv run pytestover the same unit suite as.github/workflows/test-suite.yml: 95 passed.ruff checkandisort --check-onlyare clean on the files I touched.black --checkis clean on my changes, but it also wants to reformat theCLICKABLE_URLconstant at the top ofbase_graph.py. That one predates this PR and AGENTS.md says not to reformat untouched files, so I left it alone. Happy to fold it in if you'd rather.Two tests already fail on
pre/betabefore my changes:test_llm_missing_tokensandtest_script_creator_multi_graph.py::test_entry_point_node. I checked both on a clean checkout, didn't touch either, and neither one runs in CI.No dependency changes, so
uv.lockis untouched.The rest of #1121
(4) landed in #1140. (1), raising instead of defaulting, is the breaking option and isn't in here. If that's the direction you want, or an opt-in strict flag, I can do it as a follow-up.
Fixes #1121