harden ingestion & clustering - #22
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds topic-aware event clustering and scoring, preserves AI-derived perspective summaries via provenance fields, fixes embedding dimensions with a helper, tightens ingestion (publishedAt parsing, source creation), makes singleton recluster locking configurable, and improves merge cleanup and presentation recency handling. Changes
Sequence DiagramsequenceDiagram
actor Ingest as Article Ingestion
participant Embed as Embedding Service
participant Cluster as Clustering Engine
participant Merge as Merge/Attach Logic
participant Present as Presentation Refresh
participant DB as Convex DB
Ingest->>Embed: parse article, compute embedding, parsePublishedAt
Embed->>Embed: normalize/pad to 1536 dims (toEventEmbedding)
Embed-->>Cluster: deliver article + topics + fixed-dim embedding
Cluster->>Cluster: generate topic-aware candidates
Cluster->>Cluster: score by similarity + topic overlap
Cluster->>Merge: select best candidate or mark pending
alt Candidate Match
Merge->>Merge: check time-window & similarity gates
Merge->>Merge: respect perspectiveSource (AI-preservation)
Merge->>DB: update event embedding (appendArticleEmbeddingToEventMean)
Merge->>DB: delete related rows (eventShareAssets, eventSummaryJobs, eventClaims) if removed
else No Match
Cluster->>DB: queue as pending, retry attach sorted by seed quality, then create new event
end
Merge-->>Present: event ready
Present->>DB: refresh presentation using bounded recency tie-breaker
Present->>DB: update perspectiveSource, lastSummarizedAt, lastSummarySignature as needed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 38 minutes and 26 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/backend/convex/config.ts (1)
637-703:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThese new thresholds won't reach existing deployments.
seedDefaultsis insert-only, so any environment that already has these config rows will keep the old values after this PR. That means the hardening here (clustering_*,merge_max_time_delta_hours,singleton_recluster_min_similarity) only applies to fresh installs unless you add a migration or targeted upsert for existing keys.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/convex/config.ts` around lines 637 - 703, seedDefaults is insert-only so existing deployments won't get the new thresholds; modify the seed step to perform an upsert/update for the specific keys rather than only inserting. Update the seeding logic that populates seedDefaults to detect existing rows by key (e.g., "clustering_weak_extraction_min_similarity", "clustering_weak_extraction_strong_similarity", "merge_max_time_delta_hours", "singleton_recluster_min_similarity", "clustering_min_similarity", "cluster_publish_min_articles", "cluster_publish_min_sources", "topic_inference_*", "merge_*") and apply a targeted upsert or migration that updates those keys to the new values while leaving other keys intact (or create a one-off migration script that updates only these config keys). Ensure the code path that runs during deployment calls this upsert/migration instead of relying on insert-only seedDefaults so existing environments receive the hardened values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/convex/clustering.ts`:
- Around line 2934-2936: The merge currently preserves mergedPerspectiveSource
=== "ai" but drops AI-summary metadata; update the merge logic handling
mergedPerspectiveSource and all merge call sites (the two merge callsites
referenced and the merge function in clustering.ts) to also carry forward the
most-recent/non-empty lastSummarizedAt and lastSummarySignature values when the
mergedPerspectiveSource is "ai" (or when the incoming/recipient has newer
summary metadata), so shouldResummarize() in summarization.ts sees current
timestamps/signatures; pick the newest lastSummarizedAt and the corresponding
lastSummarySignature (or prefer non-empty signature) and propagate them in the
merged event payload.
- Around line 60-61: The fixed epoch makes the recency clamp stale; replace the
hard constants PRESENTATION_RECENCY_EPOCH_MS and PRESENTATION_RECENCY_HORIZON_MS
with a relative computation so recency is computed against a dynamic reference
(e.g., Date.now() or the newest candidate timestamp). Implement a function like
computePresentationRecency(publishedAt, referenceTime = Date.now(), horizonMs =
365*24*60*60*1000) that returns a clamped 0..1 score using referenceTime -
publishedAt normalized by horizonMs, and update all uses of
PRESENTATION_RECENCY_EPOCH_MS/PRESENTATION_RECENCY_HORIZON_MS (the recency
scoring calls and the branches referenced in the diff) to call this function so
recent items retain discrimination over time.
---
Outside diff comments:
In `@packages/backend/convex/config.ts`:
- Around line 637-703: seedDefaults is insert-only so existing deployments won't
get the new thresholds; modify the seed step to perform an upsert/update for the
specific keys rather than only inserting. Update the seeding logic that
populates seedDefaults to detect existing rows by key (e.g.,
"clustering_weak_extraction_min_similarity",
"clustering_weak_extraction_strong_similarity", "merge_max_time_delta_hours",
"singleton_recluster_min_similarity", "clustering_min_similarity",
"cluster_publish_min_articles", "cluster_publish_min_sources",
"topic_inference_*", "merge_*") and apply a targeted upsert or migration that
updates those keys to the new values while leaving other keys intact (or create
a one-off migration script that updates only these config keys). Ensure the code
path that runs during deployment calls this upsert/migration instead of relying
on insert-only seedDefaults so existing environments receive the hardened
values.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: c2137425-f483-4cd0-aa4c-830a5ecf263a
📒 Files selected for processing (5)
packages/backend/convex/clustering.tspackages/backend/convex/config.tspackages/backend/convex/ingestion.tspackages/backend/convex/schema.tspackages/backend/convex/summarization.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/convex/config.ts`:
- Line 637: The default for clustering_same_source_min_similarity was updated
but not added to the forced reseed allowlist, so existing DB rows won't be
overwritten by seedDefaults; update the forcedDefaultKeys array used by
seedDefaults to include "clustering_same_source_min_similarity" (the same key
string used in the defaults) so that seedDefaults will force-reseed that setting
for existing environments; locate the forcedDefaultKeys definition and add the
key alongside the other entries (also ensure the same change is applied in the
second block referenced around the other occurrence).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 548446cc-8a66-4187-b4c8-514d0ba9434c
📒 Files selected for processing (2)
packages/backend/convex/clustering.tspackages/backend/convex/config.ts
Summary by CodeRabbit
Bug Fixes
Improvements