CON-188: add per-post Notes entity (post_notes) - #103
Conversation
A Note is a small standalone record attached to a Post (title?/body/type/ origin/created_by/timestamps) in a tenant-scoped post_notes table, so ancillary content lives outside the post body. - content-plan now leaves the post body empty and stores the generated bullet thesis as a draft_thesis note (origin=content_plan) - post assistant gains a createNote tool (image_prompt/note), reads a post's existing notes as context, and supports a "noted" action — standalone or alongside an edit in the same turn - REST CRUD under /api/posts/:post_id/notes backed by a shared notes.Service (also used by the assistant tool) with draft_thesis-first ordering - http-client file + unit/integration tests
|
Warning Review limit reached
Next review available in: 49 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (1)
WalkthroughAdds tenant-scoped post notes with shared persistence, authenticated CRUD endpoints, content-plan thesis storage, and post-assistant note creation and context support. ChangesPost Notes
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant PostNotesHandler
participant NotesService
participant PostNoteRepository
participant Database
Client->>PostNotesHandler: Submit authenticated note request
PostNotesHandler->>NotesService: Validate and execute operation
NotesService->>PostNoteRepository: Create, list, update, or delete
PostNoteRepository->>Database: Read or write post_notes
Database-->>PostNoteRepository: Return operation result
PostNoteRepository-->>NotesService: Return note data or status
NotesService-->>PostNotesHandler: Return service result
PostNotesHandler-->>Client: Return HTTP response
Possibly related PRs
Suggested labels: 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/content_plan/generate.go`:
- Around line 546-550: Update the DraftPost.Body JSON schema description in the
content-plan types and the associated content-plan prompt to request a concise
bullet-point thesis rather than complete platform-adapted post copy. Keep the
contract aligned with persistOne, which stores Body as draft_thesis while
leaving Post.Content empty.
In `@src/genkit/flows/post_assistant/context.go`:
- Around line 230-240: Bound aggregate note context in the note-summary
construction after repos.Notes.ListByPostID by enforcing a total note-count or
rune budget while iterating in repository order, stopping once the budget is
reached; retain per-body truncation and existing error handling. Add a boundary
test covering the limit and confirming ordering.
- Around line 220-242: Invalidate the cached context entry for the post
immediately after a successful note persistence in the createNote tool flow.
Update the contextCache entry keyed by post.ID after the Notes repository create
operation completes, while preserving existing error handling and leaving
buildNoteSummaries unchanged.
In `@src/genkit/flows/post_assistant/prompts/post_assistant.tmpl`:
- Around line 38-41: Update the notes-mode instructions in the post assistant
prompt and the response schema/tool guidance they conflict with: allow action
"noted" in the response schema and permit createNote for ordinary note requests.
Preserve the existing createNote type mapping, empty updatedContent requirement,
and edited-post-plus-note behavior.
In `@src/genkit/flows/post_assistant/run.go`:
- Around line 405-419: Update the action assignment in the note-results handling
around result.NotesCreated so the notes-only fallback executes only when
result.UpdatedContent is empty, in addition to the existing action and result
checks. Preserve non-empty UpdatedContent and avoid setting result.Action to
"noted" or clearing edited content for truncated combined edit-and-note
responses.
In `@src/handlers/post_notes.go`:
- Around line 49-59: Update PostNotesHandler.loadPostOrErr and the repository
lookup it uses to distinguish an absent post from a post belonging to another
tenant: preserve 404 for sql.ErrNoRows when the post does not exist, but
propagate a distinct tenant-authority error and map it to fiber.StatusForbidden
in List, Get, and Create. Add an integration test covering a note endpoint
request using another tenant’s post and assert a 403 response.
🪄 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: d36aa4f9-e527-4b04-baaf-648f0f0162ca
📒 Files selected for processing (22)
http-client/posts/notes.httpsrc/database/migrations/20260806000001_post_notes.down.sqlsrc/database/migrations/20260806000001_post_notes.up.sqlsrc/genkit/flows/content_plan/flow.gosrc/genkit/flows/content_plan/generate.gosrc/genkit/flows/content_plan/generate_test.gosrc/genkit/flows/post_assistant/context.gosrc/genkit/flows/post_assistant/post_note_tool_test.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/handlers/post_notes.gosrc/integration/post_notes_test.gosrc/models/post_note.gosrc/notes/service.gosrc/notes/service_test.gosrc/repository/post_notes.gosrc/server/genkit_runtime.gosrc/server/post_assistant.gosrc/server/server.go
| // | ||
| // CON-188: the model's bullet-point thesis (dp.Body) is no longer written into | ||
| // the post body. The post is created with an empty body and the thesis is | ||
| // stored as a draft_thesis note, so the assistant can later expand it into copy. | ||
| func persistOne(ctx context.Context, dp DraftPost, campaign *models.Campaign, postRepo repository.PostRepository, noteRepo repository.PostNoteRepository, usedAssetIDs []string) (string, error) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align the generated-field contract.
persistOne now treats DraftPost.Body as a bullet-point thesis. src/genkit/flows/content_plan/types.go still instructs the model to generate “Complete post copy adapted to the platform”.
The model can generate publishable copy, but this flow stores it as draft_thesis and leaves Post.Content empty. Update the DraftPost.Body JSON schema description and the content-plan prompt to request a thesis consistently.
Proposed contract update
- Body string `json:"body" jsonschema:"description=Complete post copy adapted to the platform"`
+ Body string `json:"body" jsonschema:"description=Bullet-point thesis for a future post expansion"`🤖 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/content_plan/generate.go` around lines 546 - 550, Update the
DraftPost.Body JSON schema description in the content-plan types and the
associated content-plan prompt to request a concise bullet-point thesis rather
than complete platform-adapted post copy. Keep the contract aligned with
persistOne, which stores Body as draft_thesis while leaving Post.Content empty.
| // loadPostOrErr fetches the parent post, returning 404 when it is missing or | ||
| // belongs to another tenant. | ||
| func (h *PostNotesHandler) loadPostOrErr(c *fiber.Ctx) (*models.Post, error) { | ||
| post, err := h.postRepo.GetByID(c.Context(), c.Params("post_id")) | ||
| if err != nil { | ||
| if errors.Is(err, sql.ErrNoRows) { | ||
| return nil, fiber.NewError(fiber.StatusNotFound, "post not found") | ||
| } | ||
| return nil, err | ||
| } | ||
| return post, nil |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect post lookup and tenant authorization behavior.
fd -e go . src/repository -x rg -n -C 5 'func \(.*\) GetByID\(' {}
rg -n -C 5 'TenantScoped|tenant_id|ErrNoRows|StatusForbidden' src/models src/repository src/handlersRepository: ogen-app/ogen
Length of output: 4802
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)src/(handlers|models|repository).*\.go$|(^|/)src/.*\.go$' | sed -n '1,160p'
echo
echo "== post handlers and routes =="
fd -e go post_notes post router . src | sed -n '1,120p'
for f in $(fd -e go 'post_notes|notes' src/handlers); do
echo "--- $f"
wc -l "$f"
sed -n '1,180p' "$f"
done
echo
echo "== repository getByID implementations mentioning posts =="
rg -n -C 12 'func \(.*\) GetByID\([^)]*\*\s*models\.Post|GetByID.*Post|scopeTenantRead|tenant_id|TenantScoped' src -g '*.go'
echo
echo "== route auth setup =="
rg -n -C 6 'RequireAuth|Register.*Notes|post_notes|PostNotesHandler|NewPostNotesHandler' srcRepository: ogen-app/ogen
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== targeted file list =="
git ls-files 'src/handlers/post_notes.go' 'src/repository/posts.go' 'src/models/*.go' 'src/*.go' | sed -n '1,120p'
echo
echo "== src/handlers/post_notes.go =="
wc -l src/handlers/post_notes.go
sed -n '1,180p' src/handlers/post_notes.go
echo
echo "== src/repository/posts.go focused =="
wc -l src/repository/posts.go
sed -n '80,135p' src/repository/posts.go
echo
echo "== models.TenantScoped / Post =="
rg -n -C 8 'type Post struct|TenantScoped|tenant_id' src/models -g '*.go'
echo
echo "== router/auth registration focused =="
rg -n -C 8 'RequireAuth\(|Register.*Notes|PostNotesHandler|NewPostNotesHandler|Notes' src -g '*.go'Repository: ogen-app/ogen
Length of output: 50370
Return 403 for cross-tenant post lookups.
loadPostOrErr returns the same sql.ErrNoRows path for a missing post and a tenant-denied post, and List/Get/Create convert it to 404. Keep 404 only when the post is absent. Add a distinct tenant-authority failure and return 403 for it.
Also add an integration test that calls a note endpoint with a post from another tenant and expects 403.
🤖 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/handlers/post_notes.go` around lines 49 - 59, Update
PostNotesHandler.loadPostOrErr and the repository lookup it uses to distinguish
an absent post from a post belonging to another tenant: preserve 404 for
sql.ErrNoRows when the post does not exist, but propagate a distinct
tenant-authority error and map it to fiber.StatusForbidden in List, Get, and
Create. Add an integration test covering a note endpoint request using another
tenant’s post and assert a 403 response.
Source: Path instructions
- content_plan: fix DraftPost.Body schema description to match the prompt and persistOne (a bullet-point thesis stored as a draft_thesis note, not copy) - post_assistant context: bound the aggregate note section (count + rune budget) in repository order so draft_thesis is never the entry dropped; add a boundary/ordering test - post_assistant: invalidate the context cache after createNote persists a note (notes aren't part of the cache fingerprint) - post_assistant prompt: allow action "noted" in the response schema and scope the "don't call tools" note to the asset-retrieval tools so createNote is permitted for ordinary note requests - post_assistant run: guard the notes-only fallback on empty updatedContent so a truncated combined edit-and-note turn keeps its edit
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/genkit/flows/post_assistant/post_note_tool_test.go (1)
85-102: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd a rune-budget boundary test.
This test only reaches
maxNotesInContext. It cannot detect removal or regression ofmaxNotesContextRunes.Add 20 notes with 800-rune bodies. Assert that
buildNoteSummariesreturns five entries and retains the firstdraft_thesis.🤖 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/post_note_tool_test.go` around lines 85 - 102, Add a rune-budget boundary case to TestBuildNoteSummaries_LimitAndOrdering using 20 notes whose bodies are 800 runes each, so the maxNotesContextRunes limit is exercised independently of maxNotesInContext. Assert that buildNoteSummaries returns exactly five entries and that the first entry remains the draft_thesis with its pinned body.
🤖 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/context.go`:
- Around line 117-120: Add per-post generation tracking protected by
contextCacheMu around assembleContextCached: capture the current generation
before assembling and only write actx to contextCache if it is still unchanged
afterward. Update invalidateContextCache to increment the post’s generation
while deleting its cached entry, preventing in-flight stale assemblies from
repopulating the cache.
---
Nitpick comments:
In `@src/genkit/flows/post_assistant/post_note_tool_test.go`:
- Around line 85-102: Add a rune-budget boundary case to
TestBuildNoteSummaries_LimitAndOrdering using 20 notes whose bodies are 800
runes each, so the maxNotesContextRunes limit is exercised independently of
maxNotesInContext. Assert that buildNoteSummaries returns exactly five entries
and that the first entry remains the draft_thesis with its pinned body.
🪄 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: 571b8412-2697-4e07-b154-019fe4e97e41
📒 Files selected for processing (6)
src/genkit/flows/content_plan/types.gosrc/genkit/flows/post_assistant/context.gosrc/genkit/flows/post_assistant/post_note_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/run.go
- src/genkit/flows/post_assistant/prompts/post_assistant.tmpl
- src/genkit/flows/post_assistant/tools.go
Track a per-post generation bumped by invalidateContextCache. Capture it before the unlocked assemble and only write the result back if unchanged, so an invalidation racing an in-flight assembly can't be undone by the stale result repopulating the cache (notes aren't in the fingerprint).
Adds a first-class Note attached to a Post — a small standalone record (
title?,body,type,origin,created_by, timestamps) that captures ancillary content (draft theses, image prompts, side notes) outside the post body.Closes CON-188.
Why
Today the content-plan flow dumps a bullet-point "draft thesis" straight into
posts.content, and any assistant-produced artifact (e.g. an image prompt) either overwrites the body or is lost. Notes give these a proper home and let the assistant read/write them without touching the copy.What's included
post_notestable (tenant-scoped,ON DELETE CASCADEfromposts) +models.PostNotewithPostNoteType {draft_thesis, image_prompt, note}(Go-validated so the set can grow without a migration) andPostNoteOrigin {manual, assistant, content_plan}(DBCHECK).notes.Service(validation +originstamping) reused by the REST CRUD and the assistant tool, so the two entry points can't drift./api/posts/:post_id/notes(PostNotesHandler, mirrorspost_attachments): list/create/get/patch/delete, wrong-post → 404, validation → 400, CON-125 activity events.draft_thesisnote (origin=content_plan, author = campaign owner). Empty thesis → no note. A note-write failure is logged, not fatal (preserves the CON-66 "persisted work survives" guarantee).createNotetool (image_prompt/noteonly —draft_thesisis reserved for content-plan), existing notes injected into the LLM context, response gainsNotesCreated+ a newAction="noted". Notes are additive — a single turn can edit the body and create notes; a notes-only turn isnoted. Includes prompt "Notes mode", anote_createdSSE event, prewarm tool-list update, andnoteCountin the persisted turn history.http-client/posts/notes.http+ unit and integration tests.Locked product decisions
originenum{manual, assistant, content_plan};created_byis always a real userimage_promptnotesAPI
Behavior change to call out
Content-plan drafts now render with an empty
posts.contentuntil auts in thedraft_thesisnote. Anything readingposts.content(posts list,quality assessment CON-184, publish-readiness) will see empty drafts. This is the intended new workflow, not a regression.Testing
go build ./...andgo vet ./src/...clean.notes.Servicevalidation, content-planpersistOn note, empty-thesis skip, nil-repo safety), assistantcreateNote` tool(origin/reserved-type/unavailable).post_notes_test.gocovers CRUD, ordering, validation,st — run with-tags integrationagainst a Postgres instance.Migration
20260806000001_post_notes.{up,down}.sql— additive, auto-discovered. `doSummary by CodeRabbit