fix: resolve poll update option text#153
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 204ac70248
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| WHERE m.guid = ? | ||
| OR ( | ||
| m.associated_message_type = ? | ||
| AND (m.associated_message_guid = ? OR m.associated_message_guid = ?) |
There was a problem hiding this comment.
Match p: poll update references
When native poll update rows store associated_message_guid in the p:<part>/<guid> form used elsewhere in this repo for associated rows, this lookup misses them because it only matches the bare GUID or p/<guid>. In that scenario an added-choice update like p:0/original-poll-guid is skipped, so decodedPollOptions still returns only the creation-row options and votes for the added option continue to have no option_text / fail CLI-RPC option validation. Use the same suffix/normalized matching pattern used for other associated GUID lookups.
Useful? React with 👍 / 👎.
|
Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 8:44 AM ET / 12:44 UTC. Summary Reproducibility: yes. Current main reads poll options from one exact-GUID row, while the linked issue, PR fixtures, and redacted live output show option text can require native Add Choice update rows. Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land this focused poll readback repair after ordinary maintainer review, keeping the source-poll GUID normalization and deterministic fixtures. Do we have a high-confidence way to reproduce the issue? Yes. Current main reads poll options from one exact-GUID row, while the linked issue, PR fixtures, and redacted live output show option text can require native Add Choice update rows. Is this the best way to solve the issue? Yes. The PR uses a narrow store-layer lookup change, preserves the no-associated-column fallback, normalizes update-row votes to the source poll GUID, and adds focused fixtures. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 1560a678ce0f. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
204ac70 to
8d66129
Compare
|
Follow-up after retesting this against a real Messages database. The first proof pass exposed one more Apple row shape I did not want to leave awkward for consumers. When a participant adds choices and votes, Messages can produce:
The earlier patch resolved Redacted live proof from a fresh checkout at {
"test_poll_found": true,
"source_created_poll": {
"event": "imessage.poll.created",
"options_count": 2,
"poll_guid_present": true
},
"update_rows": {
"count": 1,
"option_counts": [4]
},
"related_vote_rows": {
"count": 2,
"vote_to_original_count": 2,
"vote_to_update_count": 0,
"with_option_text": 2,
"examples": [
{
"relation": "vote_to_original",
"event": "imessage.poll.voted",
"original_guid_present": true,
"poll_guid_present": true,
"poll_guid_matches_source_poll": true,
"original_guid_matches_source_poll": true,
"original_guid_matches_update_row": false,
"vote": {
"option_id_present": true,
"option_text_present": true,
"option_text_length": 10,
"participant_present": true,
"event_type": "selected"
}
},
{
"relation": "vote_to_original",
"event": "imessage.poll.voted",
"original_guid_present": true,
"poll_guid_present": true,
"poll_guid_matches_source_poll": true,
"original_guid_matches_source_poll": true,
"original_guid_matches_update_row": false,
"vote": {
"option_id_present": true,
"option_text_present": true,
"option_text_length": 10,
"participant_present": true,
"event_type": "selected"
}
}
]
}
}Validation run locally: All three passed. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Fixes #152.
Native poll choices added after creation are stored in poll update rows. A vote row can still point back to the original poll GUID, so resolving option text from only the original creation row can leave
poll.vote.option_textempty even thoughpoll.vote.option_idis present.This updates poll option resolution to read the original poll row plus native option update rows associated to that poll, then merge option IDs in database order. If the schema does not expose associated-message columns, it keeps the existing exact-GUID lookup path.
Tests add the missing shape: original poll row, later native Add Choice update row, and a vote row associated to the original poll GUID selecting the added option.
Verification:
swift format lint --recursive Sources Tests TestsLinuxswift test --filter MessagePollTestsswift testmake testLocal note:
make lintcould not complete because this machine does not haveswiftlintinstalled (make: swiftlint: No such file or directory). Theswift formatportion passes.