Skip to content

fix: resolve poll update option text#153

Merged
steipete merged 4 commits into
openclaw:mainfrom
veteranbv:fix-poll-update-option-text
Jul 4, 2026
Merged

fix: resolve poll update option text#153
steipete merged 4 commits into
openclaw:mainfrom
veteranbv:fix-poll-update-option-text

Conversation

@veteranbv

Copy link
Copy Markdown
Contributor

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_text empty even though poll.vote.option_id is 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 TestsLinux
  • swift test --filter MessagePollTests
  • swift test
  • make test

Local note: make lint could not complete because this machine does not have swiftlint installed (make: swiftlint: No such file or directory). The swift format portion passes.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 = ?)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 8:44 AM ET / 12:44 UTC.

Summary
The branch updates native poll readback to merge source poll options with associated option update rows, normalize update-row vote references back to the source poll GUID, and add poll regression tests.

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.

  • Diff surface: 3 files, +211/-18. The patch is confined to poll model helpers, poll store lookup, and deterministic poll tests.
  • Live proof: 1 redacted live-output block. The contributor supplied after-fix output from a real Messages database shape at the current PR head.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #152
Summary: This PR is the candidate fix for the canonical native poll add-choice option-text issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Next step before merge

  • No ClawSweeper repair lane is needed; the remaining action is maintainer review and merge judgment for the open candidate PR.

Security
Cleared: The diff only changes local SQLite poll readback logic and deterministic tests; no security or supply-chain concern was found.

Review details

Best 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 changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. A follow-up comment provides redacted live output from a fresh checkout at 9f3820d showing update rows and vote rows with option_text present and source poll GUIDs resolved.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): A follow-up comment provides redacted live output from a fresh checkout at 9f3820d showing update rows and vote rows with option_text present and source poll GUIDs resolved.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This fixes a scoped but user-visible native iMessage poll readback bug with limited blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): A follow-up comment provides redacted live output from a fresh checkout at 9f3820d showing update rows and vote rows with option_text present and source poll GUIDs resolved.
  • proof: sufficient: Contributor real behavior proof is sufficient. A follow-up comment provides redacted live output from a fresh checkout at 9f3820d showing update rows and vote rows with option_text present and source poll GUIDs resolved.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read fully; its guidance for focused Swift changes and regression tests applies to this poll parsing PR. (AGENTS.md:1, 1560a678ce0f)
  • Current main lookup gap: Current main's decodedPollOptions(guid:) reads only one exact GUID row with LIMIT 1, so native Add Choice update rows are not included for vote option-text resolution. (Sources/IMsgCore/MessageStore+Polls.swift:64, 1560a678ce0f)
  • PR implementation: At the PR head, enrichedPollEvent resolves a source poll GUID, decodedPollOptions selects the source row plus associated update rows, and update-row votes are remapped back to the source poll. (Sources/IMsgCore/MessageStore+Polls.swift:18, 9f3820de20de)
  • Regression coverage: The PR adds fixtures for both p/original-poll-guid and p:0/original-poll-guid update associations and verifies the added option resolves to Custom choice. (Tests/IMsgCoreTests/MessagePollTests.swift:558, 9f3820de20de)
  • Real behavior proof: A follow-up PR comment includes redacted live output from a fresh checkout at 9f3820d showing update rows, related vote rows, source poll GUID matching, and option_text present. (9f3820de20de)
  • Poll feature history: History ties the affected poll readback and pollOptions surface to merged native poll and poll vote work before this PR. (Sources/IMsgCore/MessageStore+Polls.swift, 617d941f7b1e)

Likely related people:

  • veteranbv: Prior merged native iMessage poll work introduced and completed the poll decoder/readback surface that this PR extends. (role: native poll feature contributor; confidence: high; commits: 0c9d85db44f0, 2529aa7ad011; files: Sources/IMsgCore/MessagePolls.swift, Tests/IMsgCoreTests/MessagePollTests.swift)
  • Peter Steinberger: Local history shows this person as committer for the native poll support/readback commits and the v0.12.1 release snapshot that contains the current poll store file. (role: poll feature merger and release-area contributor; confidence: medium; commits: 0c9d85db44f0, 2529aa7ad011, 2c1021ac39f3; files: Sources/IMsgCore/MessagePolls.swift, Sources/IMsgCore/MessageStore+Polls.swift, Tests/IMsgCoreTests/MessagePollTests.swift)
  • omarshahine: Recent merged poll vote work added callers that depend on MessageStore.pollOptions(guid:) and sits adjacent to this readback path. (role: recent adjacent poll contributor; confidence: medium; commits: 617d941f7b1e, 1560a678ce0f; files: Sources/IMsgCore/MessageStore+Polls.swift, Sources/imsg/Commands/PollCommand.swift, Sources/imsg/RPCServer+BridgeMessageHandlers.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Jul 3, 2026
@veteranbv
veteranbv force-pushed the fix-poll-update-option-text branch from 204ac70 to 8d66129 Compare July 3, 2026 12:11
@veteranbv

Copy link
Copy Markdown
Contributor Author

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 original poll creation row
  • a native Polls update row with associated_message_type=2
  • vote rows where one points at the original poll and another points at the update row

The earlier patch resolved vote.option_text, but a vote pointed at the update row could still surface poll_guid / original_guid as the update row instead of the source poll. Commit 9f3820d fixes that. Votes that physically point at a poll update row now resolve back to the source poll for poll_guid and original_guid, while still using the updated option set to fill vote.option_text.

Redacted live proof from a fresh checkout at 9f3820d:

{
  "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:

swift format lint --recursive Sources Tests TestsLinux
swift test
make test

All three passed. make lint still cannot complete in my local environment because swiftlint is not installed; its swift format step passes before swiftlint is invoked.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 3, 2026
@steipete
steipete merged commit 2bf4bd7 into openclaw:main Jul 4, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resolve poll vote option text from native option update rows

2 participants