fix: migrate Assistant prompts to the current conversation API - #145
Conversation
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesAssistant API migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant AssistantAPI
participant UploadAPI
participant BranchStream
CLI->>AssistantAPI: create or prepare conversation
CLI->>UploadAPI: upload attachments
CLI->>AssistantAPI: post assistant message
AssistantAPI->>BranchStream: open branch SSE stream
BranchStream-->>CLI: markdown update frames
BranchStream-->>CLI: final frame and DONE marker
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/api.rs (2)
634-637: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider preparing the conversation before uploading attachments.
If conversation create/init fails, attachments have already been uploaded to Kagi and are orphaned (and the work is wasted). Swapping the two awaits fails fast on the cheaper/likelier-to-fail call.
♻️ Proposed reorder
- let attachment_uuids = - upload_current_assistant_attachments(&request.attachments, token).await?; - let (_conversation, branch) = - prepare_current_assistant_conversation(thread_id.as_deref(), request, token).await?; + let (_conversation, branch) = + prepare_current_assistant_conversation(thread_id.as_deref(), request, token).await?; + let attachment_uuids = + upload_current_assistant_attachments(&request.attachments, token).await?;🤖 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 `@src/api.rs` around lines 634 - 637, In the request flow around prepare_current_assistant_conversation and upload_current_assistant_attachments, await conversation preparation before uploading attachments. Preserve both existing error-propagation paths and pass the resulting conversation/branch and attachment UUIDs to their current downstream uses.
6346-6356: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLeftover
/assistant/promptmocks after the migration. The legacy prompt endpoint is gone, but several tests still register mocks for it; they can never match and only obscure which flow is being asserted.
src/api.rs#L6346-L6356: drop the_promptmock that follows the new stream mock inassistant_prompt_uses_multipart_when_attachments_are_present.src/api.rs#L6443-L6454: drop the_promptmock that follows the delayed stream mock inassistant_prompt_accepts_delayed_stream_response.tests/integration-cli.rs#L2066-L2066: drop the_promptmocks inassistant_stream_prints_text_deltas_by_default,assistant_stream_can_print_ndjson_updates, andassistant_once_creates_prompts_and_deletes_temporary_profile.🤖 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 `@src/api.rs` around lines 6346 - 6356, Remove the obsolete /assistant/prompt mock registrations from the listed tests: src/api.rs lines 6346-6356 in assistant_prompt_uses_multipart_when_attachments_are_present, src/api.rs lines 6443-6454 in assistant_prompt_accepts_delayed_stream_response, and tests/integration-cli.rs line 2066 in assistant_stream_prints_text_deltas_by_default, assistant_stream_can_print_ndjson_updates, and assistant_once_creates_prompts_and_deletes_temporary_profile. Keep the new stream mocks and all other test setup unchanged.
🤖 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 `@src/api.rs`:
- Around line 802-812: Update the SSE reader around response.bytes_stream() so
it buffers raw bytes across chunks instead of converting each chunk with
String::from_utf8_lossy. Detect complete "\n\n"-delimited frames at the byte
level, decode each complete frame as UTF-8 only after extraction, and retain
incomplete frame bytes for the next chunk so split multi-byte characters are
never replaced or corrupted.
- Around line 711-741: Update both assistant conversation HTTP requests in the
conversation initialization/create flow: the GET request before
read_current_assistant_json_response and the POST request using
KAGI_ASSISTANT_CONVERSATIONS_PATH. Add an Accept header requesting
application/json alongside the existing headers, preserving all other request
behavior.
---
Nitpick comments:
In `@src/api.rs`:
- Around line 634-637: In the request flow around
prepare_current_assistant_conversation and upload_current_assistant_attachments,
await conversation preparation before uploading attachments. Preserve both
existing error-propagation paths and pass the resulting conversation/branch and
attachment UUIDs to their current downstream uses.
- Around line 6346-6356: Remove the obsolete /assistant/prompt mock
registrations from the listed tests: src/api.rs lines 6346-6356 in
assistant_prompt_uses_multipart_when_attachments_are_present, src/api.rs lines
6443-6454 in assistant_prompt_accepts_delayed_stream_response, and
tests/integration-cli.rs line 2066 in
assistant_stream_prints_text_deltas_by_default,
assistant_stream_can_print_ndjson_updates, and
assistant_once_creates_prompts_and_deletes_temporary_profile. Keep the new
stream mocks and all other test setup unchanged.
🪄 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: CHILL
Plan: Pro Plus
Run ID: d26f5320-0c62-43cc-af33-9a96f0b3b9d0
📒 Files selected for processing (3)
CHANGELOG.mdsrc/api.rstests/integration-cli.rs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36e0d30fb0
ℹ️ 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".
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
36e0d30 to
1364829
Compare
|
@greptileai review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1364829735
ℹ️ 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".
1364829 to
800c634
Compare
|
@greptileai review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 800c63431f
ℹ️ 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".
| .get(http::kagi_assistant_url(&format!( | ||
| "/api/conversations/{thread_id}/init" | ||
| ))) |
There was a problem hiding this comment.
When --thread-id is a branch-specific URL such as https://assistant.kagi.com/chat/thread-1/branch-1, normalize_assistant_thread_id deliberately reduces it to only thread-1, and this request initializes the conversation without branch_uuid. The subsequent message is therefore posted to whichever branch the API reports as active/default rather than the branch named by the user, so follow-ups can continue the wrong conversation branch. Parse the prompt target with normalize_assistant_thread_ref and pass its branch UUID to the init request.
Useful? React with 👍 / 👎.
| if !response.status().is_success() { | ||
| let status = response.status(); | ||
| let body = http::read_error_body(response, "Assistant prompt stream").await; | ||
| if status == StatusCode::UNAUTHORIZED || status == StatusCode::FORBIDDEN { |
There was a problem hiding this comment.
When an expired session is redirected to a successful 200 login HTML response, this status check accepts it as an SSE stream. The HTML is later ignored because it contains no data: lines, and finish() reports a misleading parse error instead of an authentication error; structured callers consequently receive parse_error without the session-auth remediation. Retain the prior logged-out HTML detection (or validate the response content type/body) on the successful stream path.
Useful? React with 👍 / 👎.
1 participant
Summary
kagi assistantandkagi ask-pagefrom the removed legacy prompt endpoint to Kagi's current conversation, branch message, upload, and SSE endpointsFixes #144
Verification
cargo fmt --checkcargo clippy --all-targets --all-features -- -D warningscargo test -qkagi assistantprompt returned the expected completed Markdown responsekagi ask-pageprompt returned a completed non-empty Markdown answerDocs
Auth / Secrets
Summary by CodeRabbit
kagi assistantandkagi ask-pageto use Kagi’s current Assistant conversation API.Greptile Summary
Migrates Assistant prompt handling to Kagi’s current conversation API.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failures remain.
Files Needing Attention: No files require additional attention.
What T-Rex did
Important Files Changed
Reviews (3): Last reviewed commit: "fix: migrate Assistant prompts to curren..." | Re-trigger Greptile