CON-128: hybrid Post Assistant — cheap Haiku planner + Sonnet editPost writer - #110
Merged
Merged
Conversation
… write-tool Split the Post Assistant's single Sonnet call — which both routed tools and wrote the edited copy — into a cheap Haiku orchestration loop (RolePlanning) that delegates all copywriting to a Sonnet (RoleGeneration) editPost write-tool, mirroring the Campaign Assistant (CON-112). - editPost tool runs a nested Sonnet generation, streams content_delta, and returns only a compact receipt to the planner; the content flows via SSE + requestState so the cheap planner stays cheap and can't mangle the copy. - clonePost cross-platform adaptation now routes through the same writer. - run.go branches on PlannerEnabled: loop model, output cap, scanner watch list, tool set (+editPost), metering model, and a new editResult branch (before note-handling so edit-and-note turns stay 'edited'). - Writer usage metered under post_assistant_edit for per-model attribution. - Prompt split into planner + writer blocks; the legacy 'system' block is left untouched as the POST_ASSISTANT_PLANNER=false rollback path. - Prewarm targets the planner tool set on RolePlanning. External SSE/REST contract unchanged. Default on; kill-switch reverts to the proven single-Sonnet path. Adds guard unit tests for editPost/runWriter.
…path The suite built PostAssistantFlowConfig with a nil Provider (a CON-86 regression) so the flow's cfg.Provider.Ref panicked at runtime. Construct a real Provider and exercise the hybrid planner path by default, togglable to the legacy single-Sonnet path via POST_ASSISTANT_PLANNER=false so one suite covers both. Also assert content_delta streams on an edit — proving the editPost writer sub-call reaches the client in the hybrid path.
In the hybrid path, action (from the planner) and content (from the editPost writer) are decoupled, so a planner that claims action="edited" without calling editPost would drive post.Content to "" and wipe the post. Guard it: with no editResult and empty content, downgrade the turn to noted/declined and drop the version snapshot. The legacy path is unaffected — there content is emitted before action in the same JSON envelope, so the split can't occur. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughThe Post Assistant adds configurable planner and writer roles. Planner responses route edits through ChangesHybrid Post Assistant
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant PostAssistantFlow
participant PlannerModel
participant editPost
participant WriterModel
Client->>PostAssistantFlow: submit post instruction
PostAssistantFlow->>PlannerModel: run planner with editPost
PlannerModel->>editPost: send edit instruction
editPost->>WriterModel: generate updated Markdown
WriterModel-->>Client: stream content_delta events
editPost-->>PostAssistantFlow: return edit receipt and content
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/genkit/flows/post_assistant/prompts/post_assistant.tmpl`:
- Around line 91-97: Update the editPost-to-runWriter flow described in the post
assistant prompt so retrieved asset-tool content is passed as separate source
material alongside the unchanged, verbatim user instruction. Ensure asset-based
edits use the retrieved excerpts rather than only the asset preview, and add
coverage validating that this content reaches the writer.
- Around line 157-164: Update the editPost writer flow so retrieved asset chunks
from the planner are included in the writer’s input or context alongside the
original instruction. Ensure the writer can use this asset content when
producing asset-based edits, while preserving the existing response schema and
behavior for requests without retrieved chunks.
In `@src/genkit/flows/post_assistant/run.go`:
- Around line 494-505: Update the hybrid edit guard around result.Action and
st.editResult so every planner result claiming "edited" requires st.editResult
to be non-nil, regardless of result.UpdatedContent. When editPost was not
invoked, clear result.UpdatedContent before downgrading the action and retain
the existing noted/declined, SaveVersion, and explanation behavior.
In `@src/genkit/flows/post_assistant/tools.go`:
- Around line 438-442: Validate the adapted output returned by runWriter before
assigning it to content or creating the cross-platform clone. In the adaptation
flow around runWriter, treat an empty adapted string as an error and stop
processing, while preserving the existing wrapped error handling for non-nil
errors.
- Around line 426-433: Update the registered clonePost tool contract and planner
instructions to require omitting content for cross-platform clones so the
server-side writer handles adaptation. Document content as an explicit override
only, preserving the existing content check in the clone flow and ensuring
omitted content reaches runWriter and RoleGeneration.
🪄 Autofix
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: 2d0befda-94a2-4e53-b6cf-d835a59cd07d
📒 Files selected for processing (10)
src/config/config.gosrc/genkit/flows/post_assistant/edit_tool_test.gosrc/genkit/flows/post_assistant/flow.gosrc/genkit/flows/post_assistant/prewarm.gosrc/genkit/flows/post_assistant/prompts/post_assistant.tmplsrc/genkit/flows/post_assistant/run.gosrc/genkit/flows/post_assistant/tools.gosrc/genkit/flows/post_assistant/types.gosrc/integration/post_assistant_test.gosrc/server/post_assistant.go
In the hybrid path the planner retrieves asset chunks, but the Sonnet writer only saw the short previews in the context block — so asset-grounded edits worked from the preview rather than the retrieved text. Capture the chunks getAssetChunks/searchAssetChunks return on requestState (deduped by chunk ID) and pass them to the writer as source material alongside the unchanged, verbatim instruction (composeWriterInstruction). Update the planner prompt to retrieve before editPost and note that retrieved excerpts are handed to the writer. Adds coverage that the excerpts reach the writer input.
The editPost writer's system prompt described only "a single instruction" and never mentioned the ## Source material section composeWriterInstruction now appends. Note it in the writer block and add a rule to treat retrieved excerpts as authoritative for asset-based facts, preferring them over the shorter asset previews. Prompt-only; no behavior change for turns without retrieved chunks.
The hybrid safety guard only fired when result.UpdatedContent was also empty, but in the planner path UpdatedContent is populated from scanner.Values() even when only explanation is streamed. A planner that emitted inline content and claimed "edited" without calling editPost would slip past the guard and persist Haiku-written copy, defeating the split. Drop the UpdatedContent condition so any "edited" result requires st.editResult to be non-nil, and clear result.UpdatedContent before downgrading so the illegitimate inline content is never persisted or returned. Keeps the existing noted/declined, SaveVersion, and explanation behavior.
The registered clonePost tool description still told the model to "provide content adapted to that platform" for a cross-platform clone, contradicting the hybrid design (and the planner prompt): in the planner path the model must OMIT content so the server's writer adapts it. A model following the stale contract would pass content, bypassing the runWriter adaptation branch and leaking Haiku copy. Update the description to require omitting content for cross-platform clones (server adapts, optional instruction to steer) and document content as an explicit override only. Prompt/field docs and the content check were already aligned; the legacy system block is left as-is (no server writer there).
runWriter can return ("", nil) when the model yields no text. In the clone
adaptation path that empty string was assigned to content, and the downstream
`if content != ""` guard then skipped the override — so a cross-platform clone
would silently produce a verbatim, unadapted copy of the source on the target
platform. Treat an empty adapted string as an error and stop, mirroring the
guard toolEditPost already has; the existing wrapped error handling for non-nil
errors is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/genkit/flows/post_assistant/edit_tool_test.go (1)
64-68: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the complete excerpt and source identity labels.
The current checks search for only a substring and the generic
Source materialheading. They pass if the writer prompt truncates the excerpt or dropsasset1andc1. Assert the complete content and the emitted asset/chunk label.Proposed test update
- if !strings.Contains(out, "multiplexed onto OS threads") { - t.Fatalf("the retrieved excerpt content must reach the writer as source material; got: %q", out) + if !strings.Contains(out, excerpts[0].Content) { + t.Fatalf("the complete retrieved excerpt must reach the writer as source material; got: %q", out) } + if !strings.Contains(out, "Asset asset1, chunk c1:") { + t.Fatalf("the source identity labels must reach the writer; got: %q", out) + }🤖 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/genkit/flows/post_assistant/edit_tool_test.go` around lines 64 - 68, Strengthen the assertions in the relevant test around the writer output variable out: require the complete retrieved excerpt text rather than a partial substring, and verify the emitted source identity includes both asset1 and c1 instead of only the generic “Source material” heading. Preserve the existing failure diagnostics while ensuring truncation or missing labels causes the test to fail.
🤖 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.
Nitpick comments:
In `@src/genkit/flows/post_assistant/edit_tool_test.go`:
- Around line 64-68: Strengthen the assertions in the relevant test around the
writer output variable out: require the complete retrieved excerpt text rather
than a partial substring, and verify the emitted source identity includes both
asset1 and c1 instead of only the generic “Source material” heading. Preserve
the existing failure diagnostics while ensuring truncation or missing labels
causes the test to fail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 98ca66cc-18cf-4d76-93b9-ccfc2a57be47
📒 Files selected for processing (4)
src/genkit/flows/post_assistant/edit_tool_test.gosrc/genkit/flows/post_assistant/prompts/post_assistant.tmplsrc/genkit/flows/post_assistant/run.gosrc/genkit/flows/post_assistant/tools.go
🚧 Files skipped from review as they are similar to previous changes (3)
- src/genkit/flows/post_assistant/prompts/post_assistant.tmpl
- src/genkit/flows/post_assistant/run.go
- src/genkit/flows/post_assistant/tools.go
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.
Summary
The Post Assistant ran its whole turn on one Sonnet call that did two jobs at once: routing the tools (clone/restore/schedule/note/asset-lookup/Q&A) and writing the edited copy inline. This PR splits them — the orchestration loop now runs on the cheap/fast planning model (Haiku,
RolePlanning) and delegates all copywriting to a Sonnet (RoleGeneration)editPostwrite-tool — mirroring the Campaign Assistant (CON-112). Routine turns become Haiku-only; edit turns pay Haiku for routing plus a lean Sonnet call for prose (no 8-tool grammar), so both turn types get cheaper with no change to the model that actually writes.Backend-only. The external SSE/REST contract is unchanged.
What changed
editPostwrite-tool (tools.go) — runs a nested Sonnet generation, streamscontent_delta, and returns only a compact receipt ({ok, chars}) to the planner. The full content flows via SSE +requestState, never back through the cheap model, so Haiku can't mangle (or re-bill) the copy.editPostandclonePostcross-platform adaptation; verbatim/same-platform clone, restore, schedule, note, asset retrieval, and Q&A stay Haiku-only.run.gobranches onPlannerEnabled: loop model, output cap, scanner watch-list (explanation-only vs+updatedContent), tool set (+editPost), metering model, and a neweditResultauthoritative branch (placed before note-handling so edit-and-note turns stayedited).plannerblock (routes, delegates writing) andwriterblock (Markdown copywriter). The originalsystemblock is left untouched as the legacy fallback.post_assistant(Haiku), writer aspost_assistant_edit(Sonnet), for clean per-model attribution; no double count.editPost) onRolePlanning.action: "edited"without invokingeditPost, the runner never persists an empty body (it would wipe the post); the turn downgrades tonoted/declinedand drops the version snapshot. The legacy path can't hit this (content precedes action in one JSON envelope).Config & rollout
POST_ASSISTANT_PLANNER(defaulttrue) — the kill-switch. Setfalseto revert the whole assistant to the proven single-Sonnet path.POST_ASSISTANT_PLANNER_MAX_OUTPUT_TOKENS(default8192) — planner envelope cap; the writer keepsMAX_OUTPUT_TOKENS(64000).MODEL_ID/PLANNING_MODEL_ID.Backward compatibility
content_deltasimply originates from the writer sub-call now.POST /api/posts/:id/assistantrequest/response shapes unchanged.Testing
go build ./...,go vet ./..., andpost_assistantpackage tests green.editPost/runWriter.Provider— it previously built the flow config with a nilProviderand would panic) and extended: exercises the hybrid path by default (togglable to legacy viaPOST_ASSISTANT_PLANNER=false), and assertscontent_deltastreams on an edit — proving theeditPost→ writer → client wiring end-to-end. Compiles with-tags integration.go test -tags integration ./src/integration/...content_delta.Risk
QUALITY_MODEL_IDstayed Sonnet for post-quality) is exactly why the kill-switch + eval exist.Related
CON-112 (Campaign Assistant — the hybrid precedent this mirrors), CON-86 (role-based Provider + metering), CON-59 (clonePost shared-service pattern), CON-85 (post-quality Haiku-underdelivered precedent).
Summary by CodeRabbit
New Features
Bug Fixes
Tests