Refactor/simplification pass#104
Merged
Merged
Conversation
Every test module repeated the same register-extension / connect / migrate block (37 copies) and three files carried identical FakeEmbedder, FakeIndex, and directional_embedding definitions. Add database::test_pool() and an embedding::test_support module and point the tests at them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…repository SqliteEmbeddingRepository and SqliteDailyReviewEmbeddingRepository were copy-pasted SQL differing only in table names, owner id type, and how dates bind. Introduce an EmbeddingSchema trait describing those differences and one SqliteVectorIndex<S> implementation; the two repositories become type aliases over their schemas. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e LIKE wildcards The text search and vector search assembled SQL by pushing predicate strings and then binding in matching order — an easy place to introduce a bind-order bug. QueryBuilder keeps each predicate and its bind adjacent. While here, escape %, _, and backslash in the text-search pattern so user queries match those characters literally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cycle The two delivery workers were mechanical twins: same targets loop, same runner-outcome handling, same delivered_at check, same sent/skipped/ failed bookkeeping. Extract the loop into ReviewDeliveryCycle driven by a ReviewDeliveryKind trait (period arithmetic, runner, formatting) and one ReviewSender trait; the daily and weekly modules shrink to kind implementations. The two hand-rolled FakeSenders collapse into a shared test double, and the daily-only enabled flag now reaches the reconciliation config for both kinds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The LLM-related configs (OpenAI key, embedding/review/extraction/signal models and prompt paths) were read from env vars by scattered from_env() constructors at wiring time, invisible to --help and unvalidated at startup, while everything else flowed through clap. Move them onto Cli / ServeConfig and delete the from_env constructors; only the niche OPENAI_BASE_URL override still reads the environment directly. ServeConfig now carries the runtime configs, which collapses the parallel field lists in JournalServiceRegistryConfig and GlobalWorkersConfig down to the ServeConfig itself. Behavior note: embedding setup is now gated on the API key being present. Previously a missing OPENAI_API_KEY made every per-tenant service construction fail at first message; now the service starts degraded (semantic search and embeddings disabled, with a warning), matching what the tests always encoded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
No description provided.