CON-114: honor an exact post count in generatePosts - #72
Merged
Conversation
Asking the assistant to "generate 1 post" produced 3. The engine already
generates exactly req.Count, and the tool honors any positive in.Count —
but the model-facing instructions told the planner to *infer* the count
and default to "a few = 3", so it omitted count for explicit requests and
the tool fell back to 3.
Make the instructions explicit instead:
- count schema: "set count to the exact number the user names; omit only
when the user gives no number (defaults to 3)".
- prompt: same guidance, plus a "write 1 post" example, and de-bias the
"a few new drafts" framing to "one or more".
Also extract the count resolution into resolveGenerateCount (behavior
unchanged) and unit-test it: exact 1->1 and 5->5, omitted/negative->3,
over-cap->clamped.
Both modes now work: exact numbers pass through, vague ("a few") -> 3.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughThe Campaign Assistant now enforces explicit or default post counts, resolves publish windows, restricts unsupported platforms, reports generated dates, rejects explicit past dates, and caps persistence for planned batches. ChangesCampaign post generation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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: 1
🤖 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/campaign_assistant/tools.go`:
- Around line 447-450: Remove the inline implementation comment above the
resolveGenerateCount call and remove the function docstring for
resolveGenerateCount in src/genkit/flows/campaign_assistant/tools.go at lines
447-450 and 514-521; leave the code behavior 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: 57d8aa46-1c71-4333-95db-3aa61da3108a
📒 Files selected for processing (3)
src/genkit/flows/campaign_assistant/prompts/campaign_assistant.tmplsrc/genkit/flows/campaign_assistant/tools.gosrc/genkit/flows/campaign_assistant/tools_test.go
Comment on lines
+447
to
+450
| // Count: honor an explicit number exactly (so "add 1 post" yields 1); fall | ||
| // back to 3 ("a few") only when the model omitted it; clamp to the per-call | ||
| // cap. Extracted as resolveGenerateCount for unit testing. | ||
| count, requested, clamped := resolveGenerateCount(in.Count, st.maxGeneratePosts) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove comments describing WHAT the code does.
These comments merely detail implementation mechanics. As per path instructions, default to no comments and avoid describing WHAT the code does.
src/genkit/flows/campaign_assistant/tools.go#L447-L450: Remove the inline comment.src/genkit/flows/campaign_assistant/tools.go#L514-L521: Remove the function docstring.
Proposed fix
- // Count: honor an explicit number exactly (so "add 1 post" yields 1); fall
- // back to 3 ("a few") only when the model omitted it; clamp to the per-call
- // cap. Extracted as resolveGenerateCount for unit testing.
count, requested, clamped := resolveGenerateCount(in.Count, st.maxGeneratePosts)-// resolveGenerateCount maps the model-supplied count to the number of posts the
-// generatePosts tool will actually create. An explicit positive count is honored
-// exactly — "add 1 post" yields 1, not the "a few" default — while a missing or
-// non-positive count falls back to 3 ("a few"). Anything above the per-call cap
-// (maxN, default 10) is clamped down. Returns the effective count, the requested
-// count after the default is applied (surfaced to the model as RequestedCount),
-// and whether the request was clamped.
func resolveGenerateCount(requested, maxN int) (count, requestedOut int, clamped bool) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Count: honor an explicit number exactly (so "add 1 post" yields 1); fall | |
| // back to 3 ("a few") only when the model omitted it; clamp to the per-call | |
| // cap. Extracted as resolveGenerateCount for unit testing. | |
| count, requested, clamped := resolveGenerateCount(in.Count, st.maxGeneratePosts) | |
| count, requested, clamped := resolveGenerateCount(in.Count, st.maxGeneratePosts) |
Suggested change
| // Count: honor an explicit number exactly (so "add 1 post" yields 1); fall | |
| // back to 3 ("a few") only when the model omitted it; clamp to the per-call | |
| // cap. Extracted as resolveGenerateCount for unit testing. | |
| count, requested, clamped := resolveGenerateCount(in.Count, st.maxGeneratePosts) | |
| func resolveGenerateCount(requested, maxN int) (count, requestedOut int, clamped bool) { |
📍 Affects 1 file
src/genkit/flows/campaign_assistant/tools.go#L447-L450(this comment)src/genkit/flows/campaign_assistant/tools.go#L514-L521
🤖 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/campaign_assistant/tools.go` around lines 447 - 450, Remove
the inline implementation comment above the resolveGenerateCount call and remove
the function docstring for resolveGenerateCount in
src/genkit/flows/campaign_assistant/tools.go at lines 447-450 and 514-521; leave
the code behavior unchanged.
Source: Path instructions
The generation model can over-produce (stream more posts than a batch asked for), and generatePostsStreaming persisted every valid post — so a targeted "generate 1 post" request yielded 3. Add an expectedCount to the streaming persist loop (withinCount gate): the batched path passes the batch's planned PostCount, the count-less fallback passes 0 (uncapped). Deterministic guarantee: count=1 -> one batch of PostCount 1 -> at most 1 post persisted, regardless of how many the model streams. Also stops the full content-plan path from exceeding a batch's planned size. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
"generate 1 draft for Aug 20" set windowStart=Aug 20 and omitted windowEnd, which resolveWindow expands to a 14-day window — so posts spread across the range (e.g. Aug 29) instead of landing on Aug 20. Instruct the model (prompt + windowEnd schema) to set both bounds to the same date for a single specific date.
Haiku (the planner) omits `count` for "generate 1 post", so the tool's resolveGenerateCount fell through to its default of 3 — and with the engine cap in place, a request for 1 still produced 3. Default a missing/zero count to 1 (the safe minimum) instead: an omitting model can no longer over-produce, and the planner is told to pass an explicit 3 for a vague "a few". Combined with the batch cap, "generate 1" now yields exactly 1 unless the model explicitly asks for more. Trade-off: a vague "a few" with no number now depends on the model passing 3; if it omits, one post is created (safe under-, not over-, delivery).
"generate 1 for Jul 22" landed on Jul 29: the model set windowStart=Jul 22 and omitted windowEnd, resolveWindow derived a 14-day window (->Aug 5), and the lone post was placed at the window's midpoint. The prompt nudge to set windowEnd=windowStart wasn't reliable. Add singlePostWindowEnd: when count==1 and the user gave no explicit end, collapse the window to the start day so newPostValidator pins the publish date exactly. Explicit ends and multi-post requests keep their range.
Two problems surfaced testing "generate 1 for Jul 17" while today is
Jul 19: the past date was silently clamped to today (post landed on
Jul 19), and the assistant's reply invented dates ("Jul 12", wrong
counts) because the tool output carried no dates for the model to report.
- Reject past dates instead of clamping (per product decision). The
planner is told to catch a before-today date and reply conversationally
("that date has passed — pick today or later"); resolveWindow rejects an
explicit past start/end as the backstop so no past-dated draft slips in.
- Stop the date confabulation: GeneratePostsOutput now returns the actual
publish dates of the created posts, and the prompt tells the model to
report only the counts/dates the tool returned, never invent them.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/genkit/flows/campaign_assistant/tools.go (1)
448-451: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove comments describing WHAT the code does.
As per path instructions, default to no comments and avoid explaining what the code does. The previous review flagged similar comments.
src/genkit/flows/campaign_assistant/tools.go#L448-L451: Remove this comment block.src/genkit/flows/campaign_assistant/tools.go#L453-L453: Remove this comment.src/genkit/flows/campaign_assistant/tools.go#L521-L528: Remove this docstring.src/genkit/flows/campaign_assistant/tools_test.go#L186-L188: Remove this comment block.🤖 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/campaign_assistant/tools.go` around lines 448 - 451, Remove the descriptive comments around resolveGenerateCount in src/genkit/flows/campaign_assistant/tools.go lines 448-451 and 453, remove the docstring at lines 521-528, and remove the corresponding comment block in src/genkit/flows/campaign_assistant/tools_test.go lines 186-188; leave the implementation and tests unchanged.Source: Path instructions
🤖 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/campaign_assistant/tools.go`:
- Around line 533-535: Change the fallback in the requested-count handling to
default to 3 instead of 1. Update the related wantCount and wantReq assertions
in src/genkit/flows/campaign_assistant/tools_test.go lines 200-201 to expect 3.
- Line 462: Inline the single-use helper logic and remove its dedicated tests:
in src/genkit/flows/campaign_assistant/tools.go:462-462, replace
singlePostWindowEnd with the count and WindowEnd condition; remove
singlePostWindowEnd at src/genkit/flows/campaign_assistant/tools.go:544-554 and
TestSinglePostWindowEnd at
src/genkit/flows/campaign_assistant/tools_test.go:214-234. In
src/genkit/flows/content_plan/generate.go:392-394, inline the
expectedCount/posts length check; remove withinCount at
src/genkit/flows/content_plan/generate.go:533-538 and TestWithinCount at
src/genkit/flows/content_plan/generate_test.go:5-26.
---
Duplicate comments:
In `@src/genkit/flows/campaign_assistant/tools.go`:
- Around line 448-451: Remove the descriptive comments around
resolveGenerateCount in src/genkit/flows/campaign_assistant/tools.go lines
448-451 and 453, remove the docstring at lines 521-528, and remove the
corresponding comment block in src/genkit/flows/campaign_assistant/tools_test.go
lines 186-188; leave the implementation and tests 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: c64ed1ad-cade-47b7-8d02-b72070dc89e7
📒 Files selected for processing (5)
src/genkit/flows/campaign_assistant/prompts/campaign_assistant.tmplsrc/genkit/flows/campaign_assistant/tools.gosrc/genkit/flows/campaign_assistant/tools_test.gosrc/genkit/flows/content_plan/generate.gosrc/genkit/flows/content_plan/generate_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- src/genkit/flows/campaign_assistant/prompts/campaign_assistant.tmpl
| // for Jul 22" must land ON Jul 22 — not the window's midpoint. When the model | ||
| // gave only a start for a single post, collapse the derived range to that day | ||
| // so validation pins the publish date exactly (CON-114). | ||
| windowEnd = singlePostWindowEnd(windowStart, windowEnd, in.WindowEnd, count) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Inline trivial helpers to avoid premature abstraction.
As per path instructions, avoid premature abstractions; three similar lines is better than a speculative helper. singlePostWindowEnd and withinCount are single-use, one-line conditionals.
src/genkit/flows/campaign_assistant/tools.go#L462-L462: Inline the condition:if count == 1 && in.WindowEnd == "" { windowEnd = windowStart }.src/genkit/flows/campaign_assistant/tools.go#L544-L554: Remove thesinglePostWindowEndfunction.src/genkit/flows/campaign_assistant/tools_test.go#L214-L234: RemoveTestSinglePostWindowEnd.src/genkit/flows/content_plan/generate.go#L392-L394: Inline the check:if expectedCount > 0 && len(posts) >= expectedCount { return }.src/genkit/flows/content_plan/generate.go#L533-L538: Remove thewithinCountfunction.src/genkit/flows/content_plan/generate_test.go#L5-L26: RemoveTestWithinCount.
📍 Affects 4 files
src/genkit/flows/campaign_assistant/tools.go#L462-L462(this comment)src/genkit/flows/campaign_assistant/tools.go#L544-L554src/genkit/flows/campaign_assistant/tools_test.go#L214-L234src/genkit/flows/content_plan/generate.go#L392-L394src/genkit/flows/content_plan/generate.go#L533-L538src/genkit/flows/content_plan/generate_test.go#L5-L26
🤖 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/campaign_assistant/tools.go` at line 462, Inline the
single-use helper logic and remove its dedicated tests: in
src/genkit/flows/campaign_assistant/tools.go:462-462, replace
singlePostWindowEnd with the count and WindowEnd condition; remove
singlePostWindowEnd at src/genkit/flows/campaign_assistant/tools.go:544-554 and
TestSinglePostWindowEnd at
src/genkit/flows/campaign_assistant/tools_test.go:214-234. In
src/genkit/flows/content_plan/generate.go:392-394, inline the
expectedCount/posts length check; remove withinCount at
src/genkit/flows/content_plan/generate.go:533-538 and TestWithinCount at
src/genkit/flows/content_plan/generate_test.go:5-26.
Source: Path instructions
Comment on lines
+533
to
+535
| if requested <= 0 { | ||
| requested = 1 | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Set the default count to 3.
The PR objectives state that omitted or vague counts should default to 3, but the code falls back to 1.
src/genkit/flows/campaign_assistant/tools.go#L533-L535: Changerequested = 1torequested = 3.src/genkit/flows/campaign_assistant/tools_test.go#L200-L201: Update thewantCountandwantReqassertions in the test cases to3.
📍 Affects 2 files
src/genkit/flows/campaign_assistant/tools.go#L533-L535(this comment)src/genkit/flows/campaign_assistant/tools_test.go#L200-L201
🤖 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/campaign_assistant/tools.go` around lines 533 - 535, Change
the fallback in the requested-count handling to default to 3 instead of 1.
Update the related wantCount and wantReq assertions in
src/genkit/flows/campaign_assistant/tools_test.go lines 200-201 to expect 3.
Merged
This was referenced Aug 15, 2026
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.
CON-114: honor an exact post count in generatePosts
Summary
Asking the Campaign Assistant to "generate 1 post" produced 3. This fixes the assistant's targeted
generatePoststool so an explicit number is honored exactly, while vague requests ("a few", "some") still default to a sensible count.Root cause
Not the per-call cap (10) and not the generation engine —
runGeneratePostsalready generates exactlyreq.Count, and the tool honors any positivein.Count. The problem was the model-facing instructions: both thecountschema ("omit to infer from the request (a few = 3)") and the system prompt ("Infer count from the request (\"a few\" = 3)") told the planner to infer the count rather than pass the number the user named. So "generate 1 post" → the model omittedcount→ the tool fell back to itsrequested <= 0 → 3default. (That default is exactly why it was consistently 3.)Changes
src/genkit/flows/campaign_assistant/tools.gocountschema now: "Exact number of posts to add. When the user names a quantity, set count to that number ("add 1 post"→1,"5 articles"→5). Omit only when the user gives no number; an omitted count defaults to 3." Also extracted the count bleresolveGenerateCount(requested, maxN)helper (behavior unchanged).src/genkit/flows/campaign_assistant/prompts/claced "Infer count… (a few = 3)" with explicit"set count to the EXACT number the user names; omit only when no number is given (defaults to 3)"; added a"write 1 post"`src/genkit/flows/campaign_assistant/tools_test.goTestResolveGenerateCount: exact1→1/5→5, omitted/negative→3, over-cap→clamped.Behavior after
GENERATE_POSTS_MAX, default 10).Test plan
gofmtclean,go build,go vetgo test ./src/genkit/flows/campaign_assistResolveGenerateCount)Notes / limitations
The deterministic parts (engine honors the count; helper defaults + clamps) are unit-tested. Whether the planner reliably emits
count=1is a model-behavior change dions — not unit-testable. If it everunder/over-generates in practice, the fully deterministic backstop is engine-level enforcement (cap persisted posts atCountin the targeted path); happy to add as a follow-Summary by CodeRabbit