fix(sinexd): archive resolved duplicate-workbench clusters (#sinex-audit-dupe-workbench-stale-cluster) - #2569
Open
Sinity wants to merge 1 commit into
Open
Conversation
…it-dupe-workbench-stale-cluster) Problem: handle_curation_finalize only inserted a curation.duplicate_resolution event recording the operator's decision -- it never touched core.events, so handle_curation_list_duplicate_candidates's GROUP BY on (source, event_type, equivalence_key) kept returning the identical cluster on every subsequent list call regardless of judgment outcome. What changed: handle_curation_finalize now reuses the existing lifecycle archive-cascade mechanism (handle_lifecycle_archive / execute_cascade_archive, the same path lifecycle.archive and TTL expiry already use) to archive the losing candidate events out of core.events once a curation.duplicate_resolution judgment finalizes. Merge keeps the smallest (earliest-minted) UUIDv7 id as canonical; Prefer keeps preferred_event_id; Ignore (a Reject decision) never reaches finalize and leaves core.events untouched, as intended. Verification: xtask check -p sinexd passed. Test run in progress.
📝 WalkthroughWalkthroughFinalized duplicate-resolution judgments now archive losing candidate events through the lifecycle archive handler. The flow supports ChangesDuplicate resolution archival
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CurationFinalization
participant ArchiveResolvedDuplicateLosers
participant LifecycleArchiveHandler
CurationFinalization->>ArchiveResolvedDuplicateLosers: finalize duplicate-resolution judgment
ArchiveResolvedDuplicateLosers->>ArchiveResolvedDuplicateLosers: select winner and identify losers
ArchiveResolvedDuplicateLosers->>LifecycleArchiveHandler: archive losing candidate events
LifecycleArchiveHandler-->>CurationFinalization: return archive result
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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. 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: 3
🤖 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 `@crate/sinexd/src/api/handlers/curation.rs`:
- Around line 791-895: Validate in archive_resolved_duplicate_losers that a
prefer action’s preferred_event_id belongs to candidate_event_ids before
filtering losers. Return an appropriate invalid-state error for a non-member
winner, preventing all candidates from being archived; leave merge and valid
prefer behavior unchanged.
- Around line 711-725: Remove the call to archive_resolved_duplicate_losers from
the duplicate_resolution branch in handle_curation_finalize, and replace it with
loser-only archival that does not invoke handle_lifecycle_archive or
expand_cascade_from. Ensure only the losing candidate_event_ids are archived
while preserving the proposal, judgment, and finalized response events.
In `@crate/sinexd/tests/api/curation_handlers_test.rs`:
- Around line 674-768: The partial-subset finalization test must verify that the
audit chain survives archiving losers. Capture the finalized event id returned
by handle_curation_finalize, then after finalization retrieve both
judgment_event_id and that finalized id through
ctx.pool().events().get_by_id(...) and assert they remain present, while
preserving the existing candidate and straggler assertions.
🪄 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 Plus
Run ID: 6974f51c-dbce-47d7-9bd7-5dfbbba9b7e0
📒 Files selected for processing (2)
crate/sinexd/src/api/handlers/curation.rscrate/sinexd/tests/api/curation_handlers_test.rs
Comment on lines
+711
to
+725
| // Duplicate-workbench archive bridge (sinex-audit-dupe-workbench-stale- | ||
| // cluster): a finalized `curation.duplicate_resolution` judgment | ||
| // previously had zero observable effect on `core.events` -- the same | ||
| // GROUP BY cluster in `handle_curation_list_duplicate_candidates` kept | ||
| // resurfacing forever because the losing candidate rows were never | ||
| // removed. Reuse the existing lifecycle archive-cascade mechanism | ||
| // (`handle_lifecycle_archive` / `execute_cascade_archive`) to physically | ||
| // archive the losing events out of the live table, the same path | ||
| // `lifecycle.archive` and TTL expiry already use -- no shadow status | ||
| // table, `core.events` stays the single source of truth for "is this | ||
| // cluster still live". | ||
| if proposal.proposal_kind == "curation.duplicate_resolution" { | ||
| archive_resolved_duplicate_losers(pool, &proposal, &judgment).await?; | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the cascade-archive implementation and the lifecycle archive handler.
rg -n "fn execute_cascade_archive" -A 80 crate/sinexd/src
echo "---"
rg -n "fn handle_lifecycle_archive" -A 60 crate/sinexd/src/api/handlers/lifecycle.rs
echo "---"
# Check whether archive traversal follows source_event_ids / parent chains forward.
rg -n "get_source_event_ids|source_event_ids" -C5 crate/sinexd/src/api/handlers/lifecycle.rsRepository: Sinity/sinex
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -u
echo "repo files top:"
git ls-files | sed -n '1,120p'
echo "--- locate curation.rs ---"
fd -a 'curation.rs' . || true
echo "--- locate lifecycle.rs ---"
fd -a 'lifecycle.rs' . || true
echo "--- broad search archive functions ---"
rg -n "archive_resolved_duplicate_losers|execute_cascade_archive|handle_lifecycle_archive|get_source_event_ids|source_event_ids|from_parents" . || trueRepository: Sinity/sinex
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "curation outline:"
ast-grep outline crate/sinexd/src/api/handlers/curation.rs --match archive_resolved_duplicate_losers --view expanded || true
echo "--- curation relevant lines 620-780 ---"
sed -n '620,780p' crate/sinexd/src/api/handlers/curation.rs | cat -n
echo "--- curation references to archive functions ---"
rg -n "archive_resolved_duplicate_losers|execute_cascade_archive|execute_cascade_archive_from_table|get_source_event_ids|from_parents" crate/sinexd/src/api/handlers/curation.rs -C4
echo "--- persistence cascade archive implementation ---"
sed -n '1960,2420p' crate/sinex-db/src/repositories/events/persistence.rs | cat -n
echo "--- archive table cascade implementation lines ---"
sed -n '2450,2590p' crate/sinex-db/src/repositories/events/persistence.rs | cat -n
echo "--- lifecycle handler archive relevant lines if present ---"
sed -n '1,240p' crate/sinexd/src/api/handlers/lifecycle.rs | cat -n
echo "--- exact persistence function signatures and docs ---"
rg -n "execute_cascade_archive|execute_cascade_archive_table_in_tx|execute_cascade_archive_in_tx" crate/sinex-db/src/repositories/events/persistence.rs -C8Repository: Sinity/sinex
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- curation archive_resolved_duplicate_losers function ---"
sed -n '802,930p' crate/sinexd/src/api/handlers/curation.rs | cat -n
echo "--- lifecycle archive handler continuation ---"
sed -n '240,295p' crate/sinexd/src/api/handlers/lifecycle.rs | cat -n
echo "--- lifecycle archive direct public function references ---"
rg -n "collect_cascade_ids|execute_cascade_archive|archive_resolved_duplicate_losers|populate_cascade_roots_from|expand_cascade_from" crate/sinexd/src/api/handlers/curation.rs crate/sinexd/src/api/handlers/lifecycle.rs crate/sinex-db/src/repositories/events/persistence.rs -C4
echo "--- execute_cascade_archive from_table implementation ---"
sed -n '2915,3045p' crate/sinex-db/src/repositories/events/persistence.rs | cat -nRepository: Sinity/sinex
Length of output: 26966
Do not invoke the lifecycle archive cascade for duplicate losers.
archive_resolved_duplicate_losers passes only candidate_event_ids to handle_lifecycle_archive, but expand_cascade_from adds all children with matching source_event_ids. The proposal_event, judgment_event, and the just-inserted finalized event all derive from candidate parents, so they are included in cascade_ids and archived before handle_curation_finalize returns the finalized response. Use a scope that targets only losing candidate rows, or archive losers separately without the forward descendant expansion.
🤖 Prompt for 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.
In `@crate/sinexd/src/api/handlers/curation.rs` around lines 711 - 725, Remove the
call to archive_resolved_duplicate_losers from the duplicate_resolution branch
in handle_curation_finalize, and replace it with loser-only archival that does
not invoke handle_lifecycle_archive or expand_cascade_from. Ensure only the
losing candidate_event_ids are archived while preserving the proposal, judgment,
and finalized response events.
Comment on lines
+791
to
+895
| /// Determine and archive the losing events of a finalized | ||
| /// `curation.duplicate_resolution` cluster (see the call site in | ||
| /// `handle_curation_finalize`). | ||
| /// | ||
| /// `merge` keeps the candidate event with the smallest (earliest-minted) | ||
| /// UUIDv7 id as canonical -- sinex ids are monotonic UUIDv7s (see | ||
| /// `ts_coided = uuid_extract_timestamp(id)`), so the smallest id is the | ||
| /// first interpretation recorded for the cluster. `prefer` keeps | ||
| /// `preferred_event_id`. `ignore` (a Reject decision) never reaches this | ||
| /// function: `CurationFinalizedPayload::from_judgment` only builds a | ||
| /// payload for Accept/Modify decisions, so an ignored cluster fails | ||
| /// finalize before this point -- "ignore" means "this is not actually a | ||
| /// duplicate cluster" and correctly leaves `core.events` untouched. | ||
| /// | ||
| /// Archival goes through the same `lifecycle.archive` cascade mechanism | ||
| /// (`handle_lifecycle_archive` -> `execute_cascade_archive`) TTL expiry and | ||
| /// the operator-facing archive RPC already use, so losing events (and | ||
| /// anything derived from them) stop appearing in | ||
| /// `handle_curation_list_duplicate_candidates`'s `GROUP BY` the moment this | ||
| /// returns -- `core.events` stays the single source of truth for whether a | ||
| /// cluster is still live, with no shadow "resolved" status table. | ||
| async fn archive_resolved_duplicate_losers( | ||
| pool: &PgPool, | ||
| proposal: &CurationProposalPayload, | ||
| judgment: &CurationJudgmentPayload, | ||
| ) -> Result<()> { | ||
| let payload = &proposal.candidate_payload; | ||
| let candidate_event_ids: Vec<Uuid> = payload | ||
| .get("candidate_event_ids") | ||
| .cloned() | ||
| .map(serde_json::from_value) | ||
| .transpose() | ||
| .map_err(|error: serde_json::Error| { | ||
| SinexError::serialization("curation.finalize: invalid duplicate candidate_event_ids") | ||
| .with_std_error(&error) | ||
| })? | ||
| .unwrap_or_default(); | ||
| if candidate_event_ids.len() < 2 { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let action = payload | ||
| .get("action") | ||
| .and_then(JsonValue::as_str) | ||
| .unwrap_or_default(); | ||
| let preferred_event_id: Option<Uuid> = match payload.get("preferred_event_id") { | ||
| None | Some(JsonValue::Null) => None, | ||
| Some(value) => Some(serde_json::from_value(value.clone()).map_err(|error| { | ||
| SinexError::serialization("curation.finalize: invalid duplicate preferred_event_id") | ||
| .with_std_error(&error) | ||
| })?), | ||
| }; | ||
|
|
||
| let winner = match action { | ||
| "prefer" => preferred_event_id.ok_or_else(|| { | ||
| SinexError::invalid_state( | ||
| "curation.finalize: prefer duplicate resolution missing preferred_event_id", | ||
| ) | ||
| })?, | ||
| "merge" => candidate_event_ids.iter().copied().min().ok_or_else(|| { | ||
| SinexError::invalid_state( | ||
| "curation.finalize: merge duplicate resolution has no candidate events", | ||
| ) | ||
| })?, | ||
| // `ignore` (and any unrecognized future action) leaves core.events | ||
| // untouched -- nothing was judged a confirmed duplicate. | ||
| _ => return Ok(()), | ||
| }; | ||
|
|
||
| let losers: Vec<String> = candidate_event_ids | ||
| .into_iter() | ||
| .filter(|id| *id != winner) | ||
| .map(|id| id.to_string()) | ||
| .collect(); | ||
| if losers.is_empty() { | ||
| return Ok(()); | ||
| } | ||
| let losers_count = losers.len(); | ||
|
|
||
| // System-attributed but actor-tagged: the archive is an automatic | ||
| // consequence of an already-authorized finalize (gated above by | ||
| // `authorize_finalization`), not a fresh operator action, so it runs | ||
| // with Admin authority while keeping the judging actor in the audit | ||
| // trail via `actor_id`. | ||
| let auth = RpcAuthContext { | ||
| token_prefix: "curation".to_string(), | ||
| actor_id: format!("curation-workbench:{}", judgment.actor_id), | ||
| authenticated_at: Timestamp::now(), | ||
| role: crate::api::auth::Role::Admin, | ||
| }; | ||
| let request = LifecycleArchiveRequest { | ||
| before: None, | ||
| source: None, | ||
| event_ids: Some(losers), | ||
| limit: losers_count as i64, | ||
| reason: Some(format!( | ||
| "curation duplicate-resolution finalize (judgment {})", | ||
| judgment.judgment_id | ||
| )), | ||
| dry_run: false, | ||
| }; | ||
| crate::api::handlers::lifecycle::handle_lifecycle_archive(pool, request, &auth).await?; | ||
| Ok(()) | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "fn validate_duplicate_judgment_request" -A 50 crate/sinexd/src/api/handlers/curation.rsRepository: Sinity/sinex
Length of output: 2417
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== curation.rs relevant sections =="
wc -l crate/sinexd/src/api/handlers/curation.rs
sed -n '1,180p' crate/sinexd/src/api/handlers/curation.rs
sed -n '720,930p' crate/sinexd/src/api/handlers/curation.rs
sed -n '960,1040p' crate/sinexd/src/api/handlers/curation.rs
echo
echo "== struct definitions/usages =="
rg -n "struct CurationProposalPayload|struct CurationRecordDuplicateJudgmentRequest|preferred_event_id|candidate_event_ids|is_judgment_finalizable|handle_curation_finalize|archive_resolved_duplicate_losers|CurationDuplicateAction::Prefer" crate/sinexd/src/api/handlers/curation.rsRepository: Sinity/sinex
Length of output: 21323
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate generated files / serde attribute generators =="
git ls-files | rg '(^|/)(generated|\.generated|curation[_-]|schema|types|payload|payloads|curation)' || true
echo
echo "== search generated payload definitions =="
rg -n "candidate_payload|candidate_event_ids|preferred_event_id" crate -g '*.rs'
rg -n "CurationProposalPayload|CurationRecordDuplicateJudgmentRequest|CurationJudgmentPayload" crate -g '*.rs'
echo
echo "== focused context around schema definitions if present =="
rg -n "pub struct (CurationProposalPayload|CurationRecordDuplicateJudgmentRequest|CurationJudgmentPayload)|candidate_event_ids|preferred_event_id" crate -g '*.rs' | head -n 200Repository: Sinity/sinex
Length of output: 32803
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== handle_curation_record_duplicate_judgment =="
sed -n '400,560p' crate/sinexd/src/api/handlers/curation.rs
echo
echo "== curation handlers test relevant scenarios =="
sed -n '200,340p' crate/sinexd/tests/api/curation_handlers_test.rs
sed -n '570,730p' crate/sinexd/tests/api/curation_handlers_test.rs
echo
echo "== curation payloads tests =="
sed -n '220,290p' crate/sinex-primitives/tests/curation_payloads_test.rs
echo
echo "== behavioral probe: archive_loser winner filter =="
python3 - <<'PY'
from uuid import UUID
def winners_and_losers(candidate_event_ids, action, preferred_event_id):
winner = None
if action == "prefer":
winner = preferred_event_id
if winner is None:
raise Exception("missing preferred_event_id")
elif action == "merge":
winner = min(candidate_event_ids)
# unrecognized action returns immediately elsewhere
losers = [id for id in candidate_event_ids if id != winner]
return winner, losers
candidates = list(map(UUID, ["00000000-0000-0000-0000-000000000001", "00000000-0000-0000-0000-000000000002"]))
print("merge winner and losers:", winners_and_losers(candidates, "merge", None))
print("valid prefer:", winners_and_losers(candidates, "prefer", candidates[1]))
print("invalid preferred outside candidates:", winners_and_losers(candidates, "prefer", UUID("00000000-0000-0000-0000-000000000099")))
print("invalid empty prefer:", winners_and_losers([], "prefer", UUID("00000000-0000-0000-0000-000000000099")))
PYRepository: Sinity/sinex
Length of output: 21907
Reject prefer when preferred_event_id is outside candidate_event_ids.
validate_duplicate_judgment_request enforces this at judgment creation time, but archive_resolved_duplicate_losers reparses the saved candidate_payload; if a malformed/external proposal has a prefer action with a non-candidate winner, every parsed candidate is filtered as a loser and archived. Check membership after parsing the winner before filtering losers.
🤖 Prompt for 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.
In `@crate/sinexd/src/api/handlers/curation.rs` around lines 791 - 895, Validate
in archive_resolved_duplicate_losers that a prefer action’s preferred_event_id
belongs to candidate_event_ids before filtering losers. Return an appropriate
invalid-state error for a non-member winner, preventing all candidates from
being archived; leave merge and valid prefer behavior unchanged.
Comment on lines
+674
to
+768
| /// sinex-audit-dupe-workbench-stale-cluster: the finding also calls out | ||
| /// partial-subset judgments -- `handle_curation_record_duplicate_judgment` | ||
| /// only requires 2+ event_ids, not full cluster membership, so a 3-way | ||
| /// cluster can be judged over just 2 of its 3 members. That must correctly | ||
| /// REDUCE the cluster (archive the judged loser, leave the untouched | ||
| /// straggler and the winner live) rather than either silently vanishing the | ||
| /// unjudged straggler or leaving the whole cluster untouched. | ||
| #[sinex_test] | ||
| async fn curation_duplicate_partial_subset_judgment_reduces_cluster_without_full_removal( | ||
| ctx: TestContext, | ||
| ) -> TestResult<()> { | ||
| common::seed_rpc_handler_product_declarations(ctx.pool()).await?; | ||
| let candidate_a = insert_duplicate_candidate(&ctx, "partial-3way", "material-a").await?; | ||
| let candidate_b = insert_duplicate_candidate(&ctx, "partial-3way", "material-b").await?; | ||
| let straggler = insert_duplicate_candidate(&ctx, "partial-3way", "material-c").await?; | ||
| let auth = RpcAuthContext::system(); | ||
|
|
||
| // Judge only candidate_a/candidate_b -- the straggler is never part of | ||
| // this judgment's event_ids. | ||
| let judgment = handle_curation_record_duplicate_judgment( | ||
| ctx.pool(), | ||
| CurationRecordDuplicateJudgmentRequest { | ||
| source: "webhistory".to_string(), | ||
| event_type: "page.visited".to_string(), | ||
| equivalence_key: "partial-3way".to_string(), | ||
| event_ids: vec![candidate_a, candidate_b], | ||
| action: CurationDuplicateAction::Prefer, | ||
| preferred_event_id: Some(candidate_a), | ||
| actor_kind: CurationJudgmentActorKind::TestFixture, | ||
| actor_id: None, | ||
| comment: Some("prefer a over b, straggler untouched".to_string()), | ||
| }, | ||
| &auth, | ||
| ) | ||
| .await?; | ||
| let judgment_event_id = judgment | ||
| .judgment_event | ||
| .id | ||
| .ok_or_else(|| color_eyre::eyre::eyre!("judgment response event missing id"))?; | ||
|
|
||
| handle_curation_finalize( | ||
| ctx.pool(), | ||
| CurationFinalizeRequest { | ||
| judgment_event_id: judgment_event_id.to_uuid().to_string(), | ||
| }, | ||
| ) | ||
| .await?; | ||
|
|
||
| // Reduced, not gone: candidate_a (winner) and the untouched straggler | ||
| // still form a live 2-member, 2-material cluster over the same | ||
| // equivalence_key, so it correctly still surfaces for further review. | ||
| let response = handle_curation_list_duplicate_candidates( | ||
| ctx.pool(), | ||
| CurationListDuplicateCandidatesRequest { | ||
| source: Some("webhistory".to_string()), | ||
| event_type: Some("page.visited".to_string()), | ||
| limit: 10, | ||
| events_per_cluster: 10, | ||
| }, | ||
| ) | ||
| .await?; | ||
| let cluster = response | ||
| .clusters | ||
| .iter() | ||
| .find(|cluster| cluster.equivalence_key == "partial-3way") | ||
| .ok_or_else(|| color_eyre::eyre::eyre!("reduced cluster should still be listed"))?; | ||
| assert_eq!(cluster.event_count, 2); | ||
| assert_eq!(cluster.material_count, 2); | ||
| let listed_ids: Vec<_> = cluster | ||
| .events | ||
| .iter() | ||
| .map(|event| *event.event_id.as_uuid()) | ||
| .collect(); | ||
| assert!(listed_ids.contains(&candidate_a)); | ||
| assert!(listed_ids.contains(&straggler)); | ||
| assert!(!listed_ids.contains(&candidate_b)); | ||
|
|
||
| assert!( | ||
| ctx.pool() | ||
| .events() | ||
| .get_by_id(sinex_primitives::events::EventId::from_uuid(candidate_b)) | ||
| .await? | ||
| .is_none(), | ||
| "judged-away candidate_b should be archived" | ||
| ); | ||
| assert!( | ||
| ctx.pool() | ||
| .events() | ||
| .get_by_id(sinex_primitives::events::EventId::from_uuid(straggler)) | ||
| .await? | ||
| .is_some(), | ||
| "unjudged straggler should remain untouched" | ||
| ); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Add an assertion that the finalization audit chain survives the archive bridge.
This test, and the merge/prefer tests above it, only check that candidate/loser event ids appear or disappear from core.events. None assert that proposal_event, judgment_event, or the finalized event returned by handle_curation_finalize remain live afterward. proposal_event is built with all original candidate ids as parents (including the archived losers), so if the archive cascade in curation.rs propagates forward to descendants of an archived event, these three tests would still pass while the finalization record itself is silently destroyed.
Add a check in at least one of these tests that judgment_event_id and the finalized response event id are still retrievable via ctx.pool().events().get_by_id(...) after handle_curation_finalize returns. This directly exercises the cascade concern raised in crate/sinexd/src/api/handlers/curation.rs at the archive_resolved_duplicate_losers call site.
🤖 Prompt for 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.
In `@crate/sinexd/tests/api/curation_handlers_test.rs` around lines 674 - 768, The
partial-subset finalization test must verify that the audit chain survives
archiving losers. Capture the finalized event id returned by
handle_curation_finalize, then after finalization retrieve both
judgment_event_id and that finalized id through
ctx.pool().events().get_by_id(...) and assert they remain present, while
preserving the existing candidate and straggler assertions.
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
Finalizing a
curation.duplicate_resolutionjudgment (merge/prefer) previously had zero observable effect oncore.events: the losing candidate rows were never removed, sohandle_curation_list_duplicate_candidates'sGROUP BYcluster kept resurfacing the same already-judged duplicates forever.Problem
sinex-audit-dupe-workbench-stale-cluster(filed during the pre-wipe audit campaign): the duplicate-workbench UI/API had no way to make a judgment "stick" — operators could accept/merge/prefer a duplicate cluster repeatedly with no change in list output, since nothing ever leftcore.events.Solution
handle_curation_finalizenow callsarchive_resolved_duplicate_losersfor any finalizedcuration.duplicate_resolutionproposal. It determines the winner (prefer->preferred_event_id,merge-> smallest/earliest UUIDv7 candidate id) and archives the rest via the existinglifecycle.archivecascade (handle_lifecycle_archive) -- the same path TTL expiry and the operator-facing archive RPC already use. No shadow "resolved" status table;core.eventsstays the single source of truth for whether a cluster is still live.ignore(Reject) correctly leaves events untouched --CurationFinalizedPayload::from_judgmentonly builds a payload for Accept/Modify, so a rejected cluster never reaches this code.Verification
xtask test -p sinexd -E 'test(/curation_handlers/)'-- 11/11 passed, 0 failed.(A broader
test(/duplicate/)sweep also caught 3 unrelated pre-existing jetstream-idempotency/stress test failures from concurrent test-pool contention across several other agents' worktrees hitting the same dev Postgres simultaneously this session -- confirmed unrelated: none touch curation.rs or the duplicate-workbench code path.)Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
Bug Fixes