Improve ChainedLLM to handle when to move to next llm in chain, add r… #1354
Conversation
…etries to Gemini, remove retries from VLMTableStructureExtractor
|
If you're OK with the approach taken here, I can add some tests. |
| return d | ||
|
|
||
| @staticmethod | ||
| def extract_table_block(html_str: str): |
There was a problem hiding this comment.
I don't see a concrete problem with this, but another option might be to use BeautifulSoup, which we should already have as a dependency. It theoretically handles "broken" HTML, and might theoretically be able to recover from other kinds of issues. Not a big priority.
There was a problem hiding this comment.
Switched to using BeautifulSoup.
| def __init__( | ||
| self, | ||
| chain: list[LLM], | ||
| response_checker: Optional[Callable[[str], bool]] = None, |
There was a problem hiding this comment.
Cool. let's start with this. I'd love to get to the point where we can have a library of "validators" that can be used with LLM calls.
lib/sycamore/sycamore/llms/gemini.py
Outdated
| timeout=120.0, | ||
| ) | ||
| async def generate_content_async(self, model, contents, config): | ||
| return self._client.models.generate_content(model=model, contents=contents, config=config) |
There was a problem hiding this comment.
Doesn't this need to be self._client.aio.models.generate_content or need an await?
There was a problem hiding this comment.
Ah yes! I'll add 'await'.
| if res.startswith("```html"): | ||
| res = res[7:].rstrip("`") | ||
| res = res.strip() |
There was a problem hiding this comment.
One thought is that you could have the html checker handle this as well, since it's pretty common for llms to use the backtick thing.
There was a problem hiding this comment.
I don't know if we want the response check to modify the response. Also, it's only used when the LLM is a ChainedLLM so we still need this code for non-chained LLMs.
…etries to Gemini, remove retries from VLMTableStructureExtractor