Conversation
consolidate_core now unions each line's tags into its topic group instead of overwriting with the last line's tags. agentflare_backend::vent::upsert now merges incoming tags with what's already stored (union + dedupe) instead of unconditionally replacing the tags column, so a later consolidation run with no/different tags can't silently erase tags a prior run already persisted for the same topic_key. Fixes #332.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughVent consolidation and backend upsert now preserve tags by unioning and deduplicating them across same-topic lines and repeated writes. Rustdoc fetching now returns structured indexing outcomes, which are propagated through CLI and MCP callers. ChangesVent tag union preservation
Rustdoc fetch outcomes
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CliOrMcp
participant rustdoc_fetch
participant ItemIndexer
CliOrMcp->>rustdoc_fetch: request fetch and store
rustdoc_fetch->>ItemIndexer: index fetched items
ItemIndexer-->>rustdoc_fetch: return count or error
rustdoc_fetch-->>CliOrMcp: return FetchOutcome
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/agentflare-backend/src/vent.rs`:
- Around line 56-62: Make the existing-vent read, merge via merge_tags_json, and
UPDATE in the upsert flow atomic by wrapping them in an immediate write
transaction. Ensure concurrent writers cannot read the same old_tags_json and
overwrite each other’s merged tags, while preserving the current update
behavior.
In `@src/vent/consolidate.rs`:
- Around line 93-97: The tag canonicalization currently skips duplicates present
in initial input arrays. In src/vent/consolidate.rs lines 93-97, initialize
grouped tags empty and let the existing loop deduplicate every line; in
crates/agentflare-backend/src/vent.rs lines 101-109, rebuild tags from both JSON
arrays into an empty vector and use the canonical result for new-row inserts.
Add an initial duplicate tag and assert exact insertion order without sorting in
src/vent/consolidate.rs lines 362-380, and cover duplicate tags on the first
upsert with serialized-order assertions in crates/agentflare-backend/src/vent.rs
lines 201-257.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 145d3f8f-92a9-4620-a1a2-407dc59fb312
📒 Files selected for processing (2)
crates/agentflare-backend/src/vent.rssrc/vent/consolidate.rs
…them fetch_and_store/store_fetched now return a FetchOutcome (crate-overview doc, flattened, plus items_indexed and items_error) instead of a bare Document. Previously a per-item indexing failure only reached an eprintln! on the MCP server's own stderr -- invisible to the CLI, the MCP tool response, and tests alike, so a refresh could silently index zero items while reporting success. Fixes #340.
getappz
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/flare-docs/src/rustdoc.rs`:
- Around line 145-149: Remove the library-level eprintln from the per-item
indexing error branch in rustdoc.rs, while preserving the returned (0,
Some(msg)) outcome. In src/cli/docs.rs lines 79-81, retain the CLI-facing
items_error diagnostic; no direct change is required there.
In `@src/cli/docs.rs`:
- Around line 77-82: The fresh-fetch branches must return the same JSON shape as
cache-hit branches without inventing cache-hit telemetry. In
src/cli/docs.rs#L77-L82, update the DocsCmd::Get success output to match the
cached response contract; apply the equivalent change in
src/mcp_server/flare_docs.rs#L119-L122 for the MCP get handler, keeping both
paths document-only or using one shared response envelope consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2ec45637-e192-4b57-9391-86914af6fe5c
📒 Files selected for processing (3)
crates/flare-docs/src/rustdoc.rssrc/cli/docs.rssrc/mcp_server/flare_docs.rs
- agentflare_backend::vent::upsert: wrap the read-merge-write of tags in an IMMEDIATE transaction so concurrent upserts for the same topic_key can't race and clobber each other's merged tag set. Also canonicalize (dedup) tags on first insert, not just on merge with an existing row. - consolidate_core: seed each topic group's tags empty instead of from the first line's raw (possibly internally-duplicated) tags array, so the existing per-line dedup loop covers every line uniformly. - flare-docs: drop the library-level eprintln! for per-item indexing failures now that FetchOutcome carries items_error to every caller; the CLI already prints its own diagnostic from that field. - cli/docs.rs and mcp_server/flare_docs.rs: DocsCmd::Get / the "get" MCP action now return only the document on a cache-miss fetch, matching the cache-hit response shape, instead of the full FetchOutcome (with items_indexed/items_error) that only "refresh" asks for. Adds regression tests for the duplicate-tag dedup fix in both consolidate.rs and vent.rs.
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.
Summary
consolidate_corenow unions each line's tags into its topic group instead of overwriting with the last line seen for that topic within a batch.agentflare_backend::vent::upsertnow merges incoming tags with what's already stored (union + dedupe) instead of unconditionally replacing thetagscolumn.fetch_and_store/store_fetchednow return aFetchOutcome(crate-overview doc, flattened, plusitems_indexed/items_error) instead of a bareDocument, so a per-item indexing failure is visible to the CLI and MCP response instead of only reaching aneprintln!on the server's own stderr.Net effect (vent): a later consolidation run with no tags (or a different subset) on a recurring topic can no longer silently erase tags a prior run had already persisted for that topic_key. Flagged by CodeRabbit on #331, deferred as a follow-up (needs coordinated CLI + backend changes). Fixes #332.
Net effect (flare-docs): discovered while manually verifying that
flare_docs searchactually returns per-item results — a stale/broken installed binary was silently indexing 0 per-item docs for every crate but one, with zero observable signal through the MCP tool. Fixes #340.Test plan
cargo test -p agentflare-backend vent::(newupsert_unions_tags_instead_of_replacing)cargo test --bin agentflare vent::(newsame_topic_lines_union_tags_within_one_batch,later_untagged_consolidation_does_not_erase_prior_tags)cargo test -p flare-docs(26 passed, incl. updated fetch_and_store tests assertingitems_indexed/items_error)cargo test --bin agentflare flare_docs(5 passed)cargo clippyclean on touched crates;cargo fmtappliedcargo build --workspace --all-featuressearchreturns real per-item hits (e.g.docsrs/tokio/latest/item/tokio::task::blocking::spawn_blocking)Summary by CodeRabbit