fix(graph): expose when the 8192 token fallback was used - #1126
Open
aayushbaluni wants to merge 1 commit into
Open
fix(graph): expose when the 8192 token fallback was used#1126aayushbaluni wants to merge 1 commit into
aayushbaluni wants to merge 1 commit into
Conversation
When a model is not in models_tokens, the graph logs a warning and proceeds with an 8192-token window. That window silently truncates long pages, so the run succeeds and the output validates while the model never saw the part that mattered. The warning is only a log record, and nothing on the returned object distinguishes the fallback from a real limit: a caller reading model_token sees 8192 either way. Record the fallback as model_tokens_defaulted so batch pipelines can assert on it. Relates to 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.
Relates to #1121.
Verified the report against
main(27d9d28)Three graphs instantiated with a model that isn't in
models_tokens:So the silent 8192 fallback is real, and it is the important part of the report: a run succeeds, the JSON validates, and the model simply never saw the truncated portion of the page.
One correction to the report, since it affects what needs fixing: the warning is not emitted once per process.
warning_onceexists inscrapegraphai/utils/logging.pybut_create_llmcalls plainlogger.warning, so it fires on every graph construction — I captured 3 emissions from 3 instantiations. So the "long job warns on the first URL and stays silent afterwards" mechanism isn't what's happening; the warning is there every time, it's just a log record.That makes option 2 from the report the right shape rather than the once-per-process fix.
What this PR does
Records the fallback on the graph so it is reachable from code:
Before this, a caller reading
model_tokensaw8192and had no way to tell whether that was the model's real limit or the default — I confirmed there was no attribute anywhere on the instance recording it. Now a batch pipeline can fail fast on its own terms:Why not raise (option 1)
The report's first preference is to raise outright. I didn't do that here because it changes behavior for every existing user of an unlisted model — including local/self-hosted and newly-released models, which are exactly the cases most likely to be missing from the table. That's a maintainer call on breakage, and it can be layered on top of this cheaply (a
strict_model_tokensconfig flag reading the same state) once the state exists. Happy to follow up with that if you'd prefer it.I also didn't thread this into
execution_info, since that object is produced by the graph executor rather than the graph, so surfacing it there is a larger change than the defect warrants.Tests
Three added to
tests/graphs/abstract_graph_test.py, alongside the existingtest_llm_missing_tokens:test_llm_missing_tokens_sets_defaulted_flag— unknown model sets the flagtest_known_model_does_not_set_defaulted_flag— known model does nottest_explicit_model_tokens_does_not_set_defaulted_flag— an explicitmodel_tokensis authoritative, not a fallbackVerification
pytest tests/graphs/abstract_graph_test.py: 24 passed, 1 failed (21 passed, 1 failed on unmodifiedmain— so +3 and no regressions).test_llm_missing_tokensasserts the warning appears incapsysstdout, but the message goes to the logger, socaptured.outis empty. It fails identically before and after this change. It's arguably the same underlying complaint as Unknown model_tokens silently falls back to 8192 and truncates — warning goes to stderr, once per process, and never reaches the result #1121 — the warning isn't where callers look — but fixing that test is a separate change and I left it alone.self.model_tokens_defaulted = Trueline fails exactlytest_llm_missing_tokens_sets_defaulted_flagwhile the other two still pass.