docs: TDD rationalizing multimodal RAG + cluster-deploy work (refs #182)#183
Open
stevei101 wants to merge 5 commits into
Open
docs: TDD rationalizing multimodal RAG + cluster-deploy work (refs #182)#183stevei101 wants to merge 5 commits into
stevei101 wants to merge 5 commits into
Conversation
Synthesizes the 20 open issues (epics #145, #171 + bugfixes #167, #178) into three epics and 12 sequenced phases against the current repo state. Audited as of 2026-06-02: - #178 already mostly fixed by PR #179; remainder is consolidating two duplicate RagRunRecorder types in graphrag-aivcs - #167 still present at huggingface.rs:198-204 (silent zero-vector return) - None of the #145 child types exist yet — green-field - No deploy/ or infra/ dir yet; flake.nix has no dockerImage output Replaces the per-issue "100% test coverage" claim with a happy-path + error-path bar that matches the team's actual cadence. Defers cross-repo work (crossplane-heaven#10, dockworker.ai#6, lornu.ai#2347) explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`UpdateMonitor::complete_operation` held the `operations_log` `parking_lot::Mutex` guard while calling `update_performance_stats`, which re-acquires the same mutex. parking_lot mutexes are not reentrant, so the second lock blocked the thread forever. Because the lock is a synchronous (blocking) lock, on the `#[tokio::test]` current-thread runtime it wedged the single worker thread, hanging the whole test rather than just the one task. This is what caused `nix flake check`'s nextest run to spin for 6h and hit the GitHub Actions max-execution timeout. Affected tests: - graph::incremental::tests::test_basic_entity_upsert - graph::incremental::tests::test_production_graph_store_entity_upsert - graph::incremental::tests::test_production_graph_store_relationship_upsert - graph::incremental::tests::test_production_graph_store_event_publishing Scope the lock guard so it is dropped before `update_performance_stats`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Make workspace flake check compile-only (doCheck=false); nextest owns tests - Set nix-check job timeout to 120 minutes - Drop duplicate cargo test --workspace after flake check (already in checks.tests) - Merge develop (deadlock fix, benches doCheck, embedding fixes) Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
CI fix pushed (
|
principle-lgtm
requested changes
Jun 5, 2026
Contributor
principle-lgtm
left a comment
principle-lgtm
left a comment
There was a problem hiding this comment.
Needs work before approval.
The main blocker is a CI coverage regression:
- P2: The PR removes the explicit
cargo test --workspacestep, disables the workspacebuildPackagetest phase withdoCheck = false, and relies onchecks.tests/cargoNextestduringnix flake check.cargo-nextestdoes not run doctests; nextest documents doctests as needing a separatecargo test --docpath: https://nexte.st/docs/running/. That means doctest coverage can silently drop out of CI, which conflicts with this TDD's own future test bar for trait/type PRs. Please add an explicit doctest check back into CI/Nix, or otherwise show where doctests are still run.
Smaller cleanup items:
- P3: The TDD says "eight phases" in the intro and section 5, but the table defines phases
0through12. - P3: The doc still says
Tracking issue: TBD, while the PR is tied to meta-epic#182.
Current checks: latest CI run for d88b704 is still in progress at nix-check / Flake check, with later incremental and SurrealDB steps pending.
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
2 participants
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.
Summary
docs/tdd-multimodal-and-service.md— a Technical Design Doc that synthesizes the 20 open issues (epics Add support for New multimodal Gemini Embedding model #145, Epic: oxidizedRAG as cluster-native semantic-search service #171 + bugfixes graphrag-core: HuggingFace embeddings + neural.rs are stubs (return zero vectors) #167, graphrag-aivcs: integration tests reference a RagRunRecorder API that doesn't exist #178) into three epics and 12 sequenced phases.Why
The open queue grew into a flat list of ~20 issues with overlapping acceptance criteria and several broken dependency chains (impls filed before their traits; epic #145 children block each other in undocumented orders). This TDD reframes them so each phase is one PR landable against
develop, and calls out where issues are already partially resolved (e.g. #178 → PR #179).Audited 2026-06-02:
graphrag-core/src/embeddings/huggingface.rs:198-204.RagRunRecordertypes.deploy/dir;flake.nixhas nodockerImageoutput yet.Companion issue
Meta-epic #182 lists the full sequenced user stories tracked by this TDD.
Test plan
markdownlintclean locally🤖 Generated with Claude Code