Skip to content

fix: round-trip long select/reply callback values via durable payload store#3327

Open
praisonai-triage-agent[bot] wants to merge 2 commits into
mainfrom
claude/issue-3312-20260723-1030
Open

fix: round-trip long select/reply callback values via durable payload store#3327
praisonai-triage-agent[bot] wants to merge 2 commits into
mainfrom
claude/issue-3312-20260723-1030

Conversation

@praisonai-triage-agent

Copy link
Copy Markdown
Contributor

Fixes #3312

Summary

Interactive reply/select menus whose option values exceed the channel callback byte-cap (_MAX_CALLBACK_LEN = 64, e.g. Telegram's 64-byte inline limit) were silently broken: oversize values were hashed with no server-side store to reverse them, so quick-reply clicks did nothing and select clicks delivered an unusable digest.

This generalises the durable-reference pattern PraisonAI already uses for approvals (ApprovalStoreProtocol) to ordinary interactive callbacks, so long values round-trip losslessly on every channel.

Changes (core-only, additive, backward compatible)

  • bots/protocols.py β€” new CallbackPayloadStoreProtocol (peer of ApprovalStoreProtocol) + bounded, zero-dependency InMemoryCallbackPayloadStore (TTL + FIFO cap; put_sync/put/get).
  • bots/presentation.py β€” encoders persist an overflowing value under a short, collision-resistant ref and emit reply:@<ref> / select:<action_id>:@<ref>. Inline values are unchanged; with no store the prior hash fallback is preserved. adapt_presentation gains an optional callback_store= kwarg.
  • bots/interactive.py β€” InteractiveRegistry(store=...) resolves @<ref> β†’ exact value on dispatch (preserving the select action_id: prefix), fails closed on unknown/expired refs, and only treats a strict @<16-hex> token as a ref so an ordinary @ value (e.g. @channel, user@example.com) is never misrouted.
  • bots/__init__.py β€” exports the new symbols.

Design notes (kept lightweight per AGENTS.md)

  • Core ships the protocol + in-memory default; the durable SQLite/KV backend belongs in the praisonai-bot runtime (injected into the registry), mirroring ApprovalStore. No new dependencies, no new user-facing API β€” all new params are optional.

Testing

tests/unit/test_callback_payload_store.py (18 tests): store lifecycle (TTL, FIFO cap, protocol conformance), inline passthrough, ref encoding within the byte-cap (incl. long action_id), no-store hash fallback, end-to-end select/reply round-trip, fail-closed paths, and @-in-value handling. Ran alongside existing presentation/interactive/reply/bots suites β€” 84 passed, 0 regressions. (Pre-existing test_botos_protocol.py failures are unrelated: they require the praisonai wrapper package.)

Generated with Claude Code

… store (fixes #3312)

Generalise the durable-reference pattern PraisonAI already uses for
approvals to ordinary interactive callbacks, so long reply/select values
round-trip losslessly even on 64-byte-cap channels.

- protocols.py: new CallbackPayloadStoreProtocol + bounded, zero-dependency
  InMemoryCallbackPayloadStore (TTL + FIFO cap).
- presentation.py: encoders persist an overflowing value under a short ref
  and emit reply:@<ref> / select:<action_id>:@<ref>. Inline values unchanged;
  no store preserves prior hash fallback. adapt_presentation gains an optional
  callback_store= kwarg.
- interactive.py: InteractiveRegistry(store=...) resolves @<ref> back to the
  exact value on dispatch (preserving the select action_id: prefix), fails
  closed on unknown/expired refs, and never mistakes an ordinary @ value for a ref.
- bots/__init__.py: export the new symbols.

Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
@MervinPraison

Copy link
Copy Markdown
Owner

@coderabbitai review

@MervinPraison

Copy link
Copy Markdown
Owner

/review

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more β†’

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account β†’

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us β†’

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor
βœ… Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@MervinPraison, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 46 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
βš™οΈ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b46164d3-47ec-4746-8611-29f2f5b156a3

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 7494bba and c6d49c3.

πŸ“’ Files selected for processing (5)
  • src/praisonai-agents/praisonaiagents/bots/__init__.py
  • src/praisonai-agents/praisonaiagents/bots/interactive.py
  • src/praisonai-agents/praisonaiagents/bots/presentation.py
  • src/praisonai-agents/praisonaiagents/bots/protocols.py
  • src/praisonai-agents/tests/unit/test_callback_payload_store.py
✨ Finishing Touches
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/issue-3312-20260723-1030

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.

❀️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds stored references for oversized interactive callback values.

  • Defines a callback payload store protocol and bounded in-memory implementation.
  • Encodes oversized reply and select values as short references when a store is supplied.
  • Resolves stored references during interactive dispatch and fails closed when resolution fails.
  • Exports the new store APIs and adds unit coverage for encoding, expiry, eviction, and dispatch.

Confidence Score: 5/5

This follow-up appears safe to merge because no new blocking failure eligible for an additional comment was identified.

No blocking failure remains in the accepted follow-up findings.

Important Files Changed

Filename Overview
src/praisonai-agents/praisonaiagents/bots/presentation.py Adds optional persistence and reference encoding for oversized reply and degraded-select callback values.
src/praisonai-agents/praisonaiagents/bots/interactive.py Adds strict stored-reference recognition, asynchronous resolution, and fail-closed dispatch behavior.
src/praisonai-agents/praisonaiagents/bots/protocols.py Introduces the callback payload store contract and a TTL-aware, FIFO-bounded in-memory implementation.
src/praisonai-agents/praisonaiagents/bots/init.py Exports the callback payload store protocol and in-memory implementation.
src/praisonai-agents/tests/unit/test_callback_payload_store.py Covers store lifecycle, callback encoding limits, lossless round trips, fail-closed behavior, and ordinary at-sign values.

Sequence Diagram

sequenceDiagram
    participant Renderer
    participant Store as Callback Payload Store
    participant Channel
    participant Registry as Interactive Registry
    participant Handler
    Renderer->>Store: Persist oversized callback value
    Store-->>Renderer: Short reference
    Renderer->>Channel: "Send callback containing @reference"
    Channel->>Registry: Dispatch callback
    Registry->>Store: Resolve reference
    alt Reference exists
        Store-->>Registry: Original value
        Registry->>Handler: Dispatch decoded value
    else Missing or expired
        Store-->>Registry: No value
        Registry-->>Channel: Decline callback
    end
Loading

Reviews (2): Last reviewed commit: "fix(bots): declare put_sync in CallbackP..." | Re-trigger Greptile

Comment thread src/praisonai-agents/praisonaiagents/bots/presentation.py
Comment thread src/praisonai-agents/praisonaiagents/bots/presentation.py
@MervinPraison MervinPraison added pipeline/blocked:ci Blocked: CI not green on HEAD pipeline/blocked:no-final Blocked: no FINAL @claude trigger yet pipeline/final-claude-pending Reviews done; waiting for FINAL @claude labels Jul 23, 2026
@MervinPraison

Copy link
Copy Markdown
Owner

@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding.

Phase 1: Review per AGENTS.md

  1. Protocol-driven: check heavy implementations vs core SDK
  2. Backward compatible: ensure zero feature regressions
  3. Performance: no hot-path regressions
  4. SDK value: review in depth whether the change genuinely adds value to the SDK β€” never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep
  5. Do not bloat the Agent class with additional params β€” only if absolutely required; we already support many params.
  6. Repo routing: agent-callable tools β†’ PraisonAI-Tools; lifecycle plugins β†’ PraisonAI-Plugins; optional sandbox backends β†’ PraisonAI-Plugins (praisonai.sandbox entry point) β€” request changes if wrongly added to praisonaiagents/

Phase 2: FIX Valid Issues
7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix
8. Also independently identify and fix any gaps or issues you find in the changed code β€” do not rely only on prior reviewer feedback
9. Push all code fixes directly to THIS branch (do NOT create a new PR)
10. Comment a summary of exact files modified and what you skipped

Phase 3: Final Verdict
11. If all issues are resolved, approve the PR / close the Issue
12. If blocking issues remain, request changes / leave clear action items

@MervinPraison MervinPraison added pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:cooldown Blocked: post-push or @claude cooldown and removed pipeline/final-claude-pending Reviews done; waiting for FINAL @claude pipeline/blocked:no-final Blocked: no FINAL @claude trigger yet labels Jul 23, 2026
@praisonai-triage-agent

praisonai-triage-agent Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Claude finished @MervinPraison's task β€”β€” View job


Final Architecture Review β€” PR #3327

  • Read AGENTS.md + all 5 changed files
  • Validate Greptile P1 (put_sync undeclared) β†’ VALID, fixed
  • Validate Greptile P1 (store unwired) β†’ by-design; wiring belongs in praisonai-bot
  • Independent review (protocol-driven, backward-compat, SDK value)
  • Implement fix + run tests (114 passed, 0 regressions)
  • Final verdict

Verdict: βœ… Approve (with one fix applied)

The change is a clean, additive, protocol-driven generalisation of the existing ApprovalStore durable-reference pattern. It is backward compatible (all new params optional; no-store path preserves the prior hash fallback), adds no dependencies, and does not touch Agent. Core ships only the protocol + zero-dep in-memory default β€” heavy backends stay in praisonai-bot, per AGENTS.md. It genuinely strengthens the SDK: oversize reply/select callbacks previously silently no-op'd; they now round-trip losslessly and fail closed on unknown/expired refs.

Reviewer feedback triage

1. Greptile P1 β€” "Protocol stores cannot persist" (_store_ref requires undeclared put_sync) β†’ VALID. FIXED.
_encode_*_callback runs in the synchronous adapt_presentation pass, so it needs put_sync, but CallbackPayloadStoreProtocol only declared async put/get. A durable backend implementing just the declared contract was silently skipped β†’ oversize callbacks degraded to the non-routable hash.
β†’ Added put_sync to the protocol contract (with docs distinguishing the sync render entry point from the async put/get used off the render path). InMemoryCallbackPayloadStore already implements it; the contract now matches the encoder.

2. Greptile P1 β€” "Callback store remains unwired" β†’ BY DESIGN, not a blocker.
Production renderers in praisonai-bot/_presentation_renderer.py call adapt_presentation(...) without callback_store, and registries are created without a store. Per AGENTS.md the heavy renderer/registry wiring lives in the praisonai-bot runtime (the durable SQLite/KV backend is injected there, mirroring ApprovalStore). Wiring it here would be out of the PR's stated core scope and this review's mandate (praisonaiagents/praisonai only). The core contract is now correct and ready for that runtime to inject a store.
β†’ Action item for a follow-up praisonai-bot PR: inject a shared CallbackPayloadStore into both the renderer (callback_store=) and the InteractiveRegistry(store=...) so long values round-trip in production. Until then, oversize callbacks fail closed (safe) rather than mis-route.

Independent checks

  • Fail-closed correctness β€” interactive.py only treats a strict @<16-hex> token as a ref (_CALLBACK_REF_RE), so ordinary @-values (@channel, user@example.com) are never misrouted; unknown/expired refs return None β†’ not routed. βœ”
  • Select action_id: prefix preserved on resolution; long action_id is hashed to stay within the 64-byte cap. βœ”
  • Byte-accurate cap (.encode("utf-8")) β€” correct for Telegram's byte limit. βœ”
  • No hot-path regression β€” store lookups only occur when a @<ref> token is present. βœ”

Files modified (this review)

  • src/praisonai-agents/praisonaiagents/bots/protocols.py β€” declared put_sync on CallbackPayloadStoreProtocol + documented the two persistence entry points.

Skipped (intentionally)

  • Production store wiring in praisonai-bot β€” out of scope for this core PR; belongs in the wrapper/bot runtime per AGENTS.md (noted as follow-up above).

Tests

test_callback_payload_store Β· test_presentation_adapt Β· test_agent_presentation_reply Β· test_interactive_authorization Β· test_bots Β· test_bot_commands_shared β†’ 114 passed, 0 regressions. (Unrelated collection errors elsewhere are the pre-existing missing-pydantic env issue.)

--- Β· Branch: claude/issue-3312-20260723-1030

The synchronous render path (adapt_presentation) persists oversize
callback values via put_sync, but the protocol only declared async
put/get. A durable backend implementing only the declared contract was
silently rejected by the encoder, degrading long callbacks to the
non-routable hash fallback. Declaring put_sync (with docs on both
entry points) makes the contract match what the encoder requires.

Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
@praisonai-triage-agent praisonai-triage-agent Bot added pipeline/merge-ready Eligible for merge gate auto-merge and removed pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:ci Blocked: CI not green on HEAD pipeline/blocked:cooldown Blocked: post-push or @claude cooldown labels Jul 23, 2026
@MervinPraison

Copy link
Copy Markdown
Owner

@claude this PR has merge conflicts with main. Please rebase onto latest main, resolve conflicts (keep this PR's intent, merge in newer main logic), run targeted tests, and force-push with --force-with-lease. Comment which files you resolved. Do not bloat the Agent class with additional params β€” only if absolutely required; we already support many params. Keep the resolution minimal β€” the aim is a lightweight and powerful package; do not introduce scope creep while resolving conflicts.

@praisonai-triage-agent praisonai-triage-agent Bot added pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:conflict Blocked: merge conflict or rebase pending pipeline/blocked:cooldown Blocked: post-push or @claude cooldown and removed pipeline/merge-ready Eligible for merge gate auto-merge labels Jul 23, 2026
@praisonai-triage-agent

praisonai-triage-agent Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Claude finished @MervinPraison's task β€”β€” View job


Rebase onto main β€” PR #3327

  • Setup git identity & auth
  • Fetch latest main & attempt rebase
  • Analyze the 4-file conflict
  • Determine correct resolution
  • Abort redundant rebase (no force-push needed)

Result: ⚠️ This PR is already superseded by main β€” recommend closing (no rebase performed)

While rebasing onto latest main, every conflicting file (protocols.py, interactive.py, presentation.py, and the new test_callback_payload_store.py) conflicted because the exact same feature for #3312 was already merged into main via PR #3322 (commit 6285d02f8 β€” "fix: wire callback payload store through Telegram render + registry (#3312)").

main already contains the complete, production-wired implementation:

Piece On main? Where
CallbackPayloadStoreProtocol + InMemoryCallbackPayloadStore βœ… bots/protocols.py:1280
@<ref> encoding for oversize reply/select βœ… bots/presentation.py (adapt_presentation(callback_store=))
Registry resolution + fail-closed dispatch βœ… bots/interactive.py:286, create_registry(store=)
Public exports βœ… bots/__init__.py:34,123
Production wiring (Telegram render + registry share one store) βœ… praisonai-bot (_presentation_renderer.py, telegram.py)

Notably, main's merged design is cleaner and more complete than this branch:

  • It uses a simple synchronous put/get (no put/put_sync async split), so the "protocol stores cannot persist" concern Greptile raised here doesn't exist there.
  • It fully wires the store through to the praisonai-bot runtime β€” the exact follow-up this PR's own review left open (long values now round-trip in production, not just tests).

Decision (per AGENTS.md β€” lightweight, no scope creep)

I did not complete the rebase or force-push. Rebasing would only resurrect a divergent, redundant design (~223 lines of churn against main) for a feature that is already merged and better implemented β€” that is scope creep, not value.

  • Files I would have resolved: protocols.py, interactive.py, presentation.py, test_callback_payload_store.py β€” all four are add/add or content conflicts against the already-merged feature.
  • Action taken: aborted the rebase; branch left untouched (no force-push).

Recommendation: Close PR #3327 as superseded by #3322. Issue #3312 is already fixed on main. I can't close the PR myself, so leaving that to a maintainer.


Β· Branch: claude/issue-3312-20260723-1030

@praisonai-triage-agent praisonai-triage-agent Bot removed the pipeline/blocked:cooldown Blocked: post-push or @claude cooldown label Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-conflict-pending pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:conflict Blocked: merge conflict or rebase pending

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interactive bot menus silently drop long select/quick-reply values on tight callback-cap channels (no durable callback-payload store)

1 participant