-
Notifications
You must be signed in to change notification settings - Fork 1
feat: agent/user profile Creatives + system prompt sourced from profile (Subsystem A) #1412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
dec0da3
742ece3
afb6a7a
f63559c
ac8bff8
9051f15
11a20c4
49f7f24
cb70bc1
eeeed34
b5d59bc
e57ba58
ea5f94b
85619a0
0b20884
f1ecee5
1d6e5a8
591e5b5
b2359a5
6a1bb73
95c573b
947f073
4a5f101
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,11 +5,23 @@ module Collavre | |
| class Creative < ApplicationRecord | ||
| self.table_name = "creatives" | ||
|
|
||
| # Promote the `kind` discriminator (stored in `data`) to a real column so the | ||
| # profile-uniqueness index is a plain-column index that dumps identically on | ||
| # SQLite and PostgreSQL, instead of a JSON-expression index that serializes | ||
| # per-adapter and crashes `db:schema:load` on Postgres (see | ||
| # docs/engine_development.md and 20260721000010_add_unique_index_to_profile_creatives). | ||
| # `data` stays the source of truth; `kind` is re-derived from it on every save. | ||
| include Collavre::IndexedJsonColumns | ||
| indexed_json_columns json: :data, columns: { kind: "kind" } | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Reserved metadata key registry — engines register their own namespaces so | ||
| # `update_metadata` preserves them without core naming vendor-specific keys. | ||
| # "kind" is the discriminator (`inbox`/`profile`/`skill`) that scopes like | ||
| # `profiles` query on (`data->>'kind'`); it must be preserved or a metadata | ||
| # save that omits it makes the row undiscoverable and a duplicate is created. | ||
| # --------------------------------------------------------------------------- | ||
| BUILTIN_RESERVED_METADATA_KEYS = %w[markdown_source content_type editor].freeze | ||
| BUILTIN_RESERVED_METADATA_KEYS = %w[markdown_source content_type editor kind].freeze | ||
|
|
||
| class << self | ||
| def registered_reserved_metadata_keys | ||
|
|
@@ -69,6 +81,17 @@ def to_partial_path | |
| # --- Inbox --- | ||
| scope :inboxes, -> { where("data->>'kind' = 'inbox'") } | ||
|
|
||
| # --- Profile --- | ||
| PROFILE_KIND = "profile" | ||
| SKILL_KIND = "skill" | ||
|
|
||
| scope :profiles, -> { where("data->>'kind' = ?", PROFILE_KIND) } | ||
|
|
||
| # A profile creative is an agent's system prompt, never a tool source. | ||
| def profile? | ||
| data&.dig("kind") == PROFILE_KIND | ||
| end | ||
|
|
||
| SYSTEM_TOPIC_NAME = "System" | ||
| MAIN_TOPIC_NAME = "Main" | ||
| CONTENT_TOPIC_NAME = "Content" | ||
|
|
@@ -135,6 +158,25 @@ def self.inbox_for(user) | |
| ) | ||
| end | ||
|
|
||
| # Find or create the profile creative for a given user. | ||
| # Places it as a root creative (no parent) owned by the user. | ||
| # | ||
| # Guarantees a single profile creative per user. A partial unique index | ||
| # (index_creatives_on_user_id_profile_unique) prevents a duplicate at the DB, | ||
| # and create_or_find_by! resolves a concurrent insert to the surviving row — | ||
| # so there is never a second, separately-editable profile to split-brain on. | ||
| # The read-first fast path keeps the common (already-exists) case cheap. | ||
| def self.profile_for(user) | ||
| existing = profiles.where(user: user).order(:id).first | ||
| return existing if existing | ||
|
Comment on lines
+169
to
+171
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fresh evidence beyond the earlier duplicate-profile thread: deterministic Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified — real, and fixed at the root in Rather than hide/merge duplicates, this now prevents them: a partial unique index on Correcting the record from the earlier duplicate-profile thread: I had declined this exact index claiming Migration collapses any pre-index duplicate onto the oldest profile first. Regression tests: the DB rejects a second profile creative for the same user; |
||
|
|
||
| profiles.create_or_find_by!(user: user) do |creative| | ||
| creative.description = user.name.to_s | ||
| creative.data = { "kind" => PROFILE_KIND } | ||
| creative.progress = 0.0 | ||
|
Comment on lines
+173
to
+176
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When this runs for an AI agent created by another user, the profile Creative is owned by the agent account itself, but the Creative permission path only grants access to the owner or to users with CreativeShare cache entries, and Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified the mechanism — accurate: But "the directly edited profile prompt flow ... is effectively unreachable" overstates it for this PR:
The genuine forward-looking need you've identified — the creator needs access to the agent's profile Creative — is foundational to the two card requirements this PR intentionally does not yet implement: skills created/linked under the profile Creative (B), and the settings UI that surfaces the profile for direct editing (C). Granting the share, and deciding its permission level (admin vs write) and whether the skills subtree inherits it, is a coherent piece of that work rather than a data/functional defect in this A-only PR — and doing it here would prematurely commit those semantics while touching the share-cache invalidation path. Concrete fix location for when B/C lands: in |
||
| end | ||
| end | ||
|
|
||
| attr_accessor :filtered_progress | ||
|
|
||
| belongs_to :user, class_name: Collavre.configuration.user_class_name, optional: true | ||
|
|
@@ -352,6 +394,10 @@ def progress_service | |
| end | ||
|
|
||
| def mark_mcp_tools_sync_pending | ||
| # Profile creatives hold an agent's system prompt, so a fenced | ||
| # `extend ToolMeta` example in the prompt must not register a real tool. | ||
| return if profile? | ||
|
|
||
| @mcp_tools_sync_pending = true | ||
| end | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,11 @@ module Describable | |
| # textarea. Stored in data["editor"]; decoupled from the storage format | ||
| # (content_type), which is now Markdown for both surfaces. | ||
| attr_accessor :markdown_editor | ||
| # When true, convert_markdown_to_html stores markdown_source verbatim | ||
| # instead of rewriting inline data-URI images into Active Storage blobs. | ||
| # Used by prompt sync, where markdown_source is the canonical text sent | ||
| # to the LLM and must stay byte-for-byte what the user entered. | ||
| attr_accessor :skip_data_uri_rewrite | ||
| attr_reader :markdown_source | ||
|
|
||
| def markdown_source=(value) | ||
|
|
@@ -147,7 +152,17 @@ def convert_markdown_to_html | |
| # FIRST, then persist the rewritten source. Subsequent edits | ||
| # around the same image carry the blob path instead of the | ||
| # data URI, so re-renders no longer create duplicate blobs. | ||
| rewritten_source = Collavre::MarkdownConverter.rewrite_data_uri_images(new_source) | ||
| # skip_data_uri_rewrite callers (prompt sync) keep the source | ||
| # verbatim; the derived HTML still renders the data URI inline. | ||
| # Profile creatives are agent prompts: markdown_source is sent | ||
| # verbatim to the LLM, so the rewrite must never fire for them | ||
| # regardless of caller — a direct Creative-editor edit reaches this | ||
| # path without the skip flag, but its source is equally LLM-bound. | ||
| rewritten_source = if skip_data_uri_rewrite || data["kind"] == PROFILE_KIND | ||
| new_source | ||
| else | ||
| Collavre::MarkdownConverter.rewrite_data_uri_images(new_source) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fresh evidence beyond the earlier sync-path data-URI fix: when an agent’s canonical prompt is edited directly through its profile Creative in Markdown/source mode, no caller sets Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified in code — real, fixed in The caller-level flag was the wrong altitude. A profile creative’s rewritten_source = if skip_data_uri_rewrite || data["kind"] == PROFILE_KIND
new_source
else
Collavre::MarkdownConverter.rewrite_data_uri_images(new_source)
endProfile creatives are now exempt from the rewrite regardless of caller (sync, direct editor, or any future write path); non-profile creatives still rewrite to avoid duplicate blobs. Regression test:
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified — real, and already fixed in |
||
| end | ||
| self.data["markdown_source"] = rewritten_source | ||
| self.description = Collavre::MarkdownConverter.markdown_to_html(rewritten_source) | ||
| else | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ class User < ApplicationRecord | |
| self.table_name = "users" | ||
|
|
||
| include HasInboxCreative | ||
| include HasProfileCreative | ||
|
|
||
| has_many :user_themes, class_name: "Collavre::UserTheme", dependent: :destroy | ||
|
|
||
|
|
@@ -207,6 +208,45 @@ def ai_user? | |
| llm_vendor.present? | ||
| end | ||
|
|
||
| # Mirror the agent's system prompt into its profile creative as Markdown, | ||
| # so the raw prompt lands losslessly in data["markdown_source"] — the | ||
| # canonical home for the prompt. Writing the prompt straight into | ||
| # `description` would run it through the HTML sanitizer (Describable), which | ||
| # strips <thinking>/<tool_call> tags and entity-escapes < > &, corrupting | ||
| # every prompt; the derived `description` is only a rendered display view. | ||
| # The system_prompt column is legacy, pending removal. | ||
| # No-op for humans and for prompt-less agents (profile keeps its name seed). | ||
| def sync_profile_system_prompt! | ||
| return unless ai_user? | ||
| creative = profile_creative | ||
| if system_prompt.present? | ||
| return if creative.data&.dig("markdown_source") == system_prompt | ||
| # Store the prompt verbatim: markdown_source is the canonical text sent | ||
| # to the LLM, so an inline data-URI image must NOT be rewritten into a | ||
| # blob path (which would mutate the prompt the agent receives). | ||
| creative.skip_data_uri_rewrite = true | ||
| creative.update!(content_type_input: "markdown", markdown_source: system_prompt) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an agent prompt contains Markdown data-URI image syntax, such as an instruction/example Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified in code — real, fixed in Fix keeps Regression test: a prompt with an inline data-URI image round-trips through There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an agent prompt contains a fenced example or instruction block with Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified in code — real, fixed in Gated at the model (the right altitude, matching the profile data-URI fix): added Regression tests: (1)
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified — real, fixed in Fixed at two layers:
Regression tests cover the service guard, the enqueue gate, and end-to-end sync/direct-edit paths. |
||
| elsif creative.data&.dig("markdown_source").present? | ||
| # The prompt was cleared. Demote out of markdown mode so the stale | ||
| # markdown_source is dropped and render falls back to the (now-blank) | ||
| # column — matching the pre-profile behavior where blanking the prompt | ||
| # took effect immediately. Also reseed `description` back to the name | ||
| # seed so the old rendered prompt doesn't stay visible/searchable on the | ||
| # owned profile root. Without this, the old prompt stays authoritative. | ||
| creative.update!(content_type_input: "html", description: name.to_s) | ||
| end | ||
| end | ||
|
|
||
| # The effective system prompt for all read consumers. The canonical prompt | ||
| # lives in the profile creative's data["markdown_source"]; the legacy | ||
| # `system_prompt` column is the fallback for rows not yet backfilled. Every | ||
| # prompt reader (orchestration matching, type classification, typo agent, | ||
| # admin view) must route here so that editing the profile creative directly | ||
| # takes effect everywhere, not just in AiAgentService. | ||
| def effective_system_prompt | ||
| profile_creative_if_present&.data&.dig("markdown_source").presence || system_prompt | ||
|
Comment on lines
+246
to
+247
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an agent's profile Creative is edited directly, this new method makes Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified in code — real, fixed in Swept for the same class while here and found one more: Regression test: collaborator description follows a directly-edited profile prompt, not the stale column. 53 runs across the touched reader + profile suites, 0 failures.
Comment on lines
+246
to
+247
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Now that this method makes Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified — real, and already fixed in Now it reads
Comment on lines
+246
to
+247
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an agent already has a backfilled/synced profile prompt, any non- Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Real gap, but deferring it rather than adding an auto-sync callback here — with reasoning. The A model-level |
||
| end | ||
|
|
||
| def claude_channel_agent? | ||
| llm_model == "claude-code" | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Collavre | ||
| module HasProfileCreative | ||
| extend ActiveSupport::Concern | ||
|
|
||
| included do | ||
| after_create_commit :create_profile_creative | ||
| end | ||
|
|
||
| # Returns the user's profile creative, creating one if it doesn't exist. | ||
| # Use only from write paths (e.g. sync_profile_system_prompt!) — never from | ||
| # a hot read path, since creating a profile creative also creates a Topic. | ||
| def profile_creative | ||
| Collavre::Creative.profile_for(self) | ||
| end | ||
|
|
||
| # Read-only lookup: the existing profile creative or nil, never creating one. | ||
| # Read paths (effective_system_prompt) must use this so a hot-path read never | ||
| # writes, and so rendering never fails when the in-memory user is invalid | ||
| # (e.g. re-rendering the AI form after a failed save leaves name blank). | ||
| def profile_creative_if_present | ||
| return nil unless persisted? | ||
|
|
||
| # A partial unique index enforces one profile per user, so at most one row | ||
| # matches; order(:id) is defensive (matches profile_for's selection) for | ||
| # any pre-index legacy duplicate, keeping reads and writes on the same row. | ||
| Collavre::Creative.profiles.where(user: self).order(:id).first | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def create_profile_creative | ||
| Collavre::Creative.profile_for(self) | ||
| rescue StandardError => e | ||
| Rails.logger.error("[HasProfileCreative] Failed to create profile for user #{id}: #{e.message}") | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,8 +136,13 @@ def prepare_rendering_context | |
| end | ||
|
|
||
| def render_system_prompt(rendering_context) | ||
| # The prompt lives losslessly in the profile creative's markdown_source | ||
| # (data["markdown_source"]); `description` is the sanitized rendered view | ||
| # and would corrupt tags/angle-brackets, so never read it here. Fall back | ||
| # to the legacy system_prompt column for rows not yet backfilled. | ||
| template = @agent.effective_system_prompt | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a stateful agent has already replied in a topic, Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified in code — real, fixed in Now the check also includes the profile creative's profile_updated_at = @agent.profile_creative_if_present&.updated_at
prompt_changed = profile_updated_at.present? && profile_updated_at > last_reply_at
creative_changed || agent_changed || prompt_changed
|
||
| rendered = AiSystemPromptRenderer.new( | ||
| template: @agent.system_prompt, | ||
| template: template, | ||
| context: rendering_context | ||
| ).render | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When this code is deployed to a database that still has pre-20260720235959 migrations pending, this callback is installed on the current
Creativemodel before thecreatives.kindcolumn has been added. Older migrations that run before that point still save Creative records — for exampleengines/collavre/db/migrate/20251230113607_refactor_labels.rbcallsCreative.create!— sosync_indexed_json_columnswill try to writeself["kind"]against a schema where the column does not exist and abort the migration. Guard the sync when the promoted column is absent, or keep those older migrations on a migration-local model that does not include the new callback.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified — real, fixed in
947f073d. Confirmed the mechanism:IndexedJsonColumns#sync_indexed_json_columns(installed as abefore_saveonCreativeviaindexed_json_columns json: :data, columns: { kind: "kind" }) wroteself["kind"]unconditionally. On a full migration replay,20251230113607_refactor_labels(Dec 2025) runs long before20260720235959_add_kind_column_to_creativesand callsCreative.create!, so the sync hitMissingAttributeErroragainst a table without thekindcolumn and aborted the migration.Rather than pin old migrations to a migration-local model (fragile — every future record-saving migration would have to remember), the guard moves into the concern itself:
next unless has_attribute?(column). The JSON attribute stays the source of truth, so a promoted column that has not been added yet is simply skipped and re-derived on the next save once it exists. This makes everyIndexedJsonColumnsuser (Creativekind, Channelrepo_full_name/pr_number/worktree_id) robust to mid-replay saves, not just this one migration.Regression test (
skips promoted columns absent from the table instead of raising) points the column map at a not-yet-added column and asserts the save does not raise while still syncing the existing promoted columns — it errors without the guard, passes with it. FullIndexedJsonColumnsTest5 runs /creative_profile10 /user_profile_system_prompt11, 0 failures; rubocop clean.