feat: stage publisher signal indexes - #3501
Draft
jesse-merhi wants to merge 2 commits into
Draft
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Member
Author
|
/clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Contributor
|
Codex review: needs changes before merge. Reviewed August 22, 2026, 3:43 PM ET / 19:43 UTC. ClawSweeper reviewWhat this changesThis PR adds three optional publisher-signal fields and four staged Convex indexes as a prerequisite for later traffic-signal queries. Merge readiness⛔ Blocked by patch quality or review findings - 5 items remain Keep open: the staged-index approach is appropriate, but the branch still adds an unsupported persisted attention lifecycle that the moderation contract explicitly forbids. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (partial) — step 2 Assertions:
How this fits togetherPublisher-abuse scans store durable signals in Convex for staff moderation. The staged indexes prepare those records for a later Signals query rollout and ultimately feed the staff review queue. flowchart LR
A[Publisher traffic data] --> B[Abuse scan]
B --> C[Publisher signal records]
C --> D[Staged Convex indexes]
D --> E[Later Signals queries]
E --> F[Staff review queue]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Retain only the staged indexes needed by the dependent Signals query, and model any future communication workflow separately from the established signal review lifecycle. Do we have a high-confidence way to reproduce the issue? Not applicable: this is an internal staged-schema prerequisite, not a reported user-facing failure. Is this the best way to solve the issue? No: staged indexes are the right rollout mechanism, but the additional persisted attention lifecycle conflicts with the documented three-state Signals contract. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against fb0ef1d21eab. LabelsLabel changes:
Label justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (21 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
added
merge-risk: 🚨 availability
Member
Author
|
/clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🦐 gold shrimp
Contributor
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: feat: stage publisher signal indexes This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
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.
"## What Problem This Solves\n\nThe Signals workflow needs four new Convex indexes, but querying an index in the same production deployment that first creates it can race index backfill.\n\n## Why This Change Was Made\n\nThis prerequisite adds the indexes in staged mode without switching any production query to them. The dependent Signals PR can be deployed only after Convex reports all four staged indexes backfilled.\n\n## User Impact\n\nThere is no user-visible behavior change in this PR. It is an operational safety step for the Signals rollout.\n\n## Validation\n\n-
bunx tsc --noEmit: passed.\n- The full stacked head passedbun run ci:static,bun run ci:unit,bun run ci:types-build, andbun run ci:packages.\n-bunx convex dev --once: passed against development deploymentadmired-dodo-615; no production deployment or backfill was started.\n\nDeployment gate: deploy this PR first, wait for both staged indexes to finish backfilling, then deploy #3493. Do not merge or deploy the dependent query switch first.\n\nReview status: draft. Sign-off has not been requested."