fix(tapestore): survive republic→bub.tape TapeEntry drift and drop _build_llm private-API dependency#49
Closed
CorrectRoadH wants to merge 6 commits into
Conversation
…ve TapeEntry class drift When bub resolves to git HEAD instead of the workspace-locked commit, TapeEntry instances fail pydantic's isinstance check against the TapeEntry class captured at schema-build time, causing every append to raise a ValidationError that the exporter silently swallows — resulting in zero exported spans. Use SkipValidation[list[TapeEntry]] on TraceProjection.entries so pydantic bypasses the isinstance check entirely. The field is internal-only and never needs runtime coercion, so skipping validation is safe and is the explicit recommendation from the pydantic docs for this class of problem. Adds a regression test that feeds build_tape_trace entries whose class object differs from republic.TapeEntry to guard against future drift. Closes bubbuild#47 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcGvz588KPpJVkEP2eaDby
…uild_llm dependency SQLiteTapeStore.__init__ imported and called bub.builtin.agent._build_llm, a private helper that was removed in bub >= 0.3.10.dev2. This caused an ImportError before any tape operation could run, even when the store was used without semantic search. Fixes: - Remove the _build_llm call from __init__; initialize self._llm = None - Add a module-level _build_embedding_client() that uses the public AnyLLM.create() and bub.ensure_config(AgentSettings) API, with a model_client_kwargs() fallback for newer bub versions - Build the client lazily in _compute_embedding(), only when semantic search is actually invoked - Update tests to mock _build_embedding_client in bub_tapestore_sqlite.store instead of the removed bub.builtin.agent._build_llm Non-semantic store operations (append, fetch_all without query, list_tapes, reset) now work against any bub version without touching the LLM layer. Closes bubbuild#43 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcGvz588KPpJVkEP2eaDby
CorrectRoadH
changed the title
bub replaced republic with its own tape implementation (commit 04960b1). Plugins still imported from republic, so bub.tape.TapeEntry instances passed at runtime failed pydantic's isinstance check against republic.TapeEntry — causing ValidationError in otel and ImportError in sqlite (from the now-deleted _build_llm). Changes across both plugins: - Replace all `republic` imports with `bub.tape` / `bub.runtime` - bub-tapestore-otel: remove SkipValidation workaround (no longer needed now that the TapeEntry class identity is correct) - bub-tapestore-sqlite: replace RepublicError→BubError, ErrorKind from bub.runtime; rewrite _build_embedding_client to use the public AnyLLM.create(provider, **settings.model_client_kwargs(provider)) API; type _embedding_client as AnyLLM|None; simplify _compute_embedding to call client.aembedding directly without _core.get_client - Remove republic>=0.5.7 from both pyproject.toml; add bub to sqlite deps Closes bubbuild#43 Closes bubbuild#47 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcGvz588KPpJVkEP2eaDby
CorrectRoadH
changed the title
CorrectRoadH
force-pushed
the
fix/tapestore-otel-tape-entry-validation
branch
from
5ccbfb8 to
bab3d9d
Compare
…blic vs bub.tape) bub >= 0.3.10 vendors the tape module as bub.tape and constructs TapeEntry instances from it, while the plugin validated entries against republic's TapeEntry. The pydantic dataclass isinstance check then rejected every entry, and OTelTapeStore.append swallowed the ValidationError, so the exporter silently emitted zero spans. - import TapeEntry (and tape-store types) from bub.tape first, falling back to republic for older bub - type TraceProjection.entries as list[Any]: entries are an internal payload, and validating their class identity turns any future drift back into the same silent zero-span failure Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DeHZzZ8Xows9kMUHb5RkqZ
… drop list[Any] - move the bub.tape/republic fallback imports into a single _compat module instead of repeating the try/except in exporter, store, and tests - type TraceProjection.entries as list[SkipValidation[TapeEntry]]: keeps the static type while still skipping pydantic's class-identity check, which is what the drift fix requires - add a regression test that entries from a foreign TapeEntry class still project into a trace instead of being dropped Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ZBsYdsdb8K2pvXKjFeWZY
CorrectRoadH
changed the title
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
1 participant
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.
close #43 #48
Root cause
bub is migrating its tape implementation from the external
republicpackage to a vendoredbub.tapemodule (upstream commit04960b1). During this transition the two modules ship structurally identical but distinct classes:bub.tape.TapeEntryandrepublic.TapeEntryfail each other'sisinstancecheck.That broke both plugins, in different ways:
bub.tape.TapeEntry;TapeEntryfromrepublicand used it as a pydantic field type (entries: list[TapeEntry]);isinstancecheck → every entry raisedValidationError;OTelTapeExporter.appendintentionally swallows exceptions so export failures never break the agent loop — so the error was logged as a warning and the exporter emitted zero spans with no visible failure.bub.builtin.agent._build_llmhelper the store used to build its embedding client →ImportErroron any store construction.Fix
An earlier revision of this PR hard-switched all imports to
bub.tape/bub.runtime. That traded one breakage for another: released bub versions still ship therepublicAPI (CI failed withModuleNotFoundError: No module named 'bub.runtime'). The final approach makes the plugins work with both sides of the transition:bub-tapestore-otel
_compat.pymodule as the single place that resolves tape types: trybub.tapefirst, fall back torepublicfor older bub. Exporter, store, and tests all import from it.TraceProjection.entriesis nowlist[SkipValidation[TapeEntry]]. The field is an opaque internal payload (only accessed via duck typing:kind/payload/id/date), and the plugin cannot control whichTapeEntryclass the host bub constructs — so validating class identity adds no safety and re-creates the silent zero-span failure whenever plugin and host disagree.SkipValidationkeeps the static type for readers and type checkers while dropping only the harmful runtimeisinstancecheck.bub-tapestore-sqlite
republicimports (that is what released bub provides here)._build_llmprivate-API dependency: the embedding client is now built lazily via the publicAnyLLM.create(provider, **settings.model_client_kwargs(provider)), typed asAnyLLM | None.Test results
(bub-tapestore-otel + bub-tapestore-sqlite account for 32 of those.)
Note for maintainers: CI runs on this fork PR are pending workflow approval (
action_required).🤖 Generated with Claude Code
https://claude.ai/code/session_017ZBsYdsdb8K2pvXKjFeWZY