-
Notifications
You must be signed in to change notification settings - Fork 6
Fix issues that made long runs fail or hang #182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
chandrakananandi
wants to merge
10
commits into
master
Choose a base branch
from
cnandi/run-hardening
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
3578d3e
llm: stream Anthropic calls and give requests a real timeout
chandrakananandi 811ad47
rag: serialize sentence-transformer encodes, add a device override
chandrakananandi 7480549
crucible: revive checkpoint dicts at session readback
chandrakananandi 3e17f8d
diagnostics: read streamed responses' usage from usage_metadata
chandrakananandi 2de7120
rag: share the encode lock with the sync DefaultEmbedder
chandrakananandi 06ab8c5
Merge branch 'master' into cnandi/run-hardening
chandrakananandi cc39721
undo session readback change
chandrakananandi 64b9642
rely on get_normalized_token_usage and also make the usage check simpler
chandrakananandi 4e37f04
Merge branch 'master' into cnandi/run-hardening
chandrakananandi df5165a
more general token accounting
chandrakananandi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| from dataclasses import dataclass | ||
| import asyncio | ||
| import logging | ||
| import threading | ||
| from abc import ABC, abstractmethod | ||
| import os | ||
|
|
||
|
|
@@ -20,6 +21,7 @@ | |
|
|
||
| from composer.rag.types import ManualRef, BlockChunk, ManualSectionHit | ||
| from composer.rag.text import code_ref_tag | ||
| from composer.rag.models import ENCODE_LOCK | ||
|
|
||
| import sqlite3 | ||
|
|
||
|
|
@@ -96,19 +98,29 @@ async def get_manual_section(self, headers: list[str]) -> str | None: | |
| ... | ||
|
|
||
|
|
||
| # Encodes race when concurrent (see ENCODE_LOCK in composer.rag.models — the lock | ||
| # is shared with the sync DefaultEmbedder so neither path can race the other). | ||
| def _encode_query_locked(self, query: str) -> ndarray: | ||
| with ENCODE_LOCK: | ||
| return cast(ndarray, self.tr.encode_query(query, show_progress_bar=False)) | ||
|
|
||
| def _encode_docs_locked(self, docs: list[str]) -> list[ndarray]: | ||
| with ENCODE_LOCK: | ||
| return cast(list[ndarray], self.tr.encode_document(docs, show_progress_bar=False)) | ||
|
|
||
| async def embed_query( | ||
| self, query: str | ||
| ) -> ndarray: | ||
| return cast(ndarray, await asyncio.to_thread( | ||
| self.tr.encode_query, f"search_query: {query}", show_progress_bar=False | ||
| )) | ||
| return await asyncio.to_thread( | ||
| self._encode_query_locked, f"search_query: {query}" | ||
| ) | ||
|
|
||
| async def embed_docs( | ||
| self, doc: list[BlockChunk] | ||
| ) -> list[ndarray]: | ||
| return cast(list[ndarray], await asyncio.to_thread( | ||
| self.tr.encode_document, [f"search_document: {d.chunk}" for d in doc], show_progress_bar=False | ||
| )) | ||
| return await asyncio.to_thread( | ||
| self._encode_docs_locked, [f"search_document: {d.chunk}" for d in doc] | ||
| ) | ||
|
Comment on lines
-102
to
+123
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should also be applied to the |
||
|
|
||
| type RagConnection = str | AsyncConnectionPool[AsyncConnection[TupleRow]] | ||
|
|
||
|
|
||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I happen to know this causes the langgraph api to produce streaming results in its own API. Please double check this doesn't utterly break the TUI and console display handlers, i.e. we aren't streaming chunks that our handlers have no idea what to do with, this somehow opts us out of the complete results, etc.