Repository navigation
llm log analyzer: stop dropping low/medium/high priorities llms: declare SUPPORTS_STREAMING on the base provider - #3945
Open
rootkiller6788 wants to merge 2 commits into
Conversation
The mapping only knew the SecGemini severity names, so a provider that reports the Timesketch names instead got no label on the question. Worse, the fallback branch skips the warning for exactly low/medium/high, so nothing was logged either - the value just vanished. Add the three names to the mapping and drop the now-dead inner check. medium was unreachable from a provider before this.
log_analyzer reads llm_provider.SUPPORTS_STREAMING to raise a readable 'does not support streaming' error, but only the SecGemini provider ever defined it. Every other provider (ollama, vertexai, aistudio, azureai) hit an AttributeError instead, which tells a provider author nothing. Default it to False on LLMProvider so the existing ValueError is what you actually get. None of those providers implement generate_stream_from_logs, so nothing that worked before stops working.
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
rootkiller6788
marked this pull request as ready for review
October 13, 2026 14:53
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
This branch has not been deployed
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.
Two small things in the LLM log analyzer that bite anyone writing a provider, both from #3891.
Priority: the mapping only had SecGemini's
notice/critical, so a provider sendinglow/medium/highgot no label at all - and no warning either, because the fallback branch explicitly skips those three names. So the value just vanished. I added them to the mapping. That also makesmediumreachable for the first time, which seems right since the API's ownVALID_PRIORITIESis already__ts_priority_low/medium/high.SUPPORTS_STREAMING:
log_analyzerreads it to raise the "does not support streaming" ValueError, but the baseLLMProvidernever defined it - only SecGemini did. Everyone else got an AttributeError instead, which tells a provider author nothing. Declared it asFalseon the base class so the intended error is the one you see. The shipped ollama/vertexai/aistudio/azureai providers don't implementgenerate_stream_from_logs, so this doesn't change what any of them can actually do.Tests: the priority one asserts the label is applied for all three names and that a genuinely unknown value still lands in
_errors_encountered. The streaming one asserts a provider subclass without the flag raises ValueError. Both fail on master - the priority test showsadd_labelbeing called zero times.I left the docstrings and the developer docs alone (that's the third point in #3891) since I wasn't sure which side you'd want changed. Happy to do that too if you want it.