Create the foundational Rails engine structure for ActivePrompt #243 #277

sarbadaj · 2025-11-17T06:37:38Z

feat(active_prompt): add and finalize mountable Rails engine (#243)

Add ActivePrompt engine (engine.rb, version.rb, module)
Isolate namespace via `isolate_namespace`
Wire asset pipeline (JS/CSS + manifest) and precompile hooks
Add engine routes with GET /health (no optional format)
Add minimal controller/helper stubs
Mount engine in dummy app and load engine before routes
Define `ActiveAgent::TestCase` to satisfy Zeitwerk
Add Minitest coverage: engine load/isolation, routes, assets
Stabilize route tests (assert named route and exact path)
Test hygiene: clean tmp/generators and remove lingering constants to avoid generator collisions

…gents#243) - Add ActivePrompt engine (engine.rb, version.rb, module) - Isolate namespace via \`isolate_namespace\` - Wire asset pipeline (JS/CSS + manifest) and precompile hooks - Add engine routes with GET /health (no optional format) - Add minimal controller/helper stubs - Mount engine in dummy app and load engine before routes - Define \`ActiveAgent::TestCase\` to satisfy Zeitwerk - Add Minitest coverage: engine load/isolation, routes, assets - Stabilize route tests (assert named route and exact path) - Test hygiene: clean tmp/generators and remove lingering constants to avoid generator collisions

…rator; make tests auto-migrate - Add ActiveRecord models: - app/models/active_prompt/prompt.rb - app/models/active_prompt/message.rb - app/models/active_prompt/action.rb - app/models/active_prompt/context.rb - Add agent DSL: - lib/active_agent/has_context.rb - enables: `has_context prompts: :prompts, messages: :messages, tools: :actions` - instance helpers: `add_prompt`, `remove_prompt` - Add engine migration (SQLite-friendly: uses `t.json`): - db/migrate/20251123000000_create_active_prompt_core.rb - Add install generator for host apps: - lib/generators/active_prompt/install/install_generator.rb - (optional) lib/active_prompt/railtie.rb + `require "active_prompt/railtie"` - Wire dummy app for verification: - test/dummy/app/models/application_agent.rb - test/dummy/db/migrate/20251123001000_create_application_agents.rb (uses `t.json`) - Tests: - test/active_prompt/models_test.rb (persistence + DSL smoke tests) - test/active_prompt/* (engine load, routes, assets) kept - test(active_prompt): add validations + message ordering coverage - Test boot: proactively migrate both dummy and engine paths: - test/test_helper.rb: construct MigrationContext with paths (no `connection.migration_context`) - still calls `ActiveRecord::Migration.maintain_test_schema!` Notes: - Accepts `rails g active_prompt:install` in host apps (or `railties:install:migrations FROM=active_prompt`).

Copilot

Pull request overview

This PR establishes the foundational structure for ActivePrompt as a mountable Rails engine, introducing models for managing prompts, messages, actions, and contexts, along with database migrations, routes, test infrastructure, and asset pipeline integration.

Key Changes:

Creates a Rails engine with isolated namespace for ActivePrompt
Adds core models (Prompt, Message, Action, Context) with associations and validations
Implements ActiveAgent::HasContext concern for polymorphic prompt attachment
Sets up comprehensive test suite with dummy app integration

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
lib/active_prompt.rb	Main entry point for the ActivePrompt engine module
lib/active_prompt/engine.rb	Engine configuration with asset pipeline and i18n setup
lib/active_prompt/version.rb	Version constant for the engine (0.1.0)
config/routes.rb	Defines engine routes including health check endpoint
app/models/active_prompt/*.rb	Core models for prompts, messages, actions, and contexts
db/migrate/20251127000000_create_active_prompt_core.rb	Migration creating all ActivePrompt tables with indexes
lib/active_agent/has_context.rb	Concern providing polymorphic prompt association methods
lib/active_agent/test_case.rb	Minimal test case base class for Zeitwerk compatibility
lib/generators/active_prompt/install/install_generator.rb	Generator for installing engine migrations into host app
test/test_helper.rb	Enhanced test setup with migration handling and test hygiene
test/active_prompt/*.rb	Comprehensive test coverage for engine, models, routes, and assets
test/dummy/config/application.rb	Dummy app configuration loading the engine
test/dummy/config/routes.rb	Mounts ActivePrompt engine at /active_prompt
test/dummy/db/migrate/*.rb	Dummy app migration for test agent model
app/assets/*/	JavaScript and CSS assets with manifest configuration
app/controllers/active_prompt/application_controller.rb	Base controller with CSRF protection
app/helpers/active_prompt/application_helper.rb	Empty helper module for future use
docs/parts/examples/*.md	Updated documentation with corrected example output

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-08T19:59:34Z

app/models/active_prompt/prompt.rb

+    def to_runtime
+      {
+        name: name,
+        description: description,
+        template: template,
+        messages: messages.order(:position).map(&:attributes),
+        actions: actions.map(&:attributes),
+        metadata: metadata || {}
+      }
+    end


Calling .map(&:attributes) on associations will trigger N+1 queries if the associations aren't preloaded. Consider documenting that callers should eager-load associations, or add a scope/class method that handles preloading. For example:

def to_runtime { name: name, description: description, template: template, messages: messages.order(:position).as_json, # or use .pluck with specific columns actions: actions.as_json, metadata: metadata || {} } end

Or provide a scope like Prompt.with_runtime_associations that preloads the data.

Suggested change

def to_runtime

{

name: name,

description: description,

template: template,

messages: messages.order(:position).map(&:attributes),

actions: actions.map(&:attributes),

metadata: metadata || {}

}

end

# NOTE: For best performance, callers should eager-load :messages and :actions

# using the .with_runtime_associations scope to avoid N+1 queries.

def to_runtime

{

name: name,

description: description,

template: template,

messages: messages.order(:position).as_json,

actions: actions.as_json,

metadata: metadata || {}

}

end

# Scope to eager-load associations needed for to_runtime

scope :with_runtime_associations, -> { includes(:messages, :actions) }

Copilot · 2025-12-08T19:59:35Z

app/models/active_prompt/message.rb

+
+    belongs_to :prompt, class_name: "ActivePrompt::Prompt", inverse_of: :messages
+
+    enum :role, { system: "system", user: "user", assistant: "assistant", tool: "tool" }, prefix: true


The enum defines string values that are identical to their keys. Rails enums can infer string values automatically when using the array syntax, which would be cleaner:

enum :role, %i[system user assistant tool], prefix: true

This produces the same behavior but is more concise. However, if explicit string values are intentional for clarity, the current approach is fine.

Suggested change

enum :role, { system: "system", user: "user", assistant: "assistant", tool: "tool" }, prefix: true

enum :role, %i[system user assistant tool], prefix: true

Copilot · 2025-12-08T19:59:35Z

config/routes.rb

+# frozen_string_literal: true
+
+ActivePrompt::Engine.routes.draw do
+  get "health", to: proc { [200, { "Content-Type" => "text/plain" }, ["ok"]] }, as: :health


[nitpick] Using a proc for a simple health check route works but is unconventional. For better maintainability and consistency with Rails patterns, consider creating a dedicated controller:

# In app/controllers/active_prompt/health_controller.rb module ActivePrompt class HealthController < ApplicationController def show render plain: "ok" end end end # Then in routes: get "health", to: "health#show", as: :health

This makes testing easier and follows Rails conventions.

Suggested change

get "health", to: proc { [200, { "Content-Type" => "text/plain" }, ["ok"]] }, as: :health

get "health", to: "health#show", as: :health

Copilot · 2025-12-08T19:59:36Z

test/dummy/config/application.rb

+$LOAD_PATH.unshift File.expand_path("../../../lib", __dir__)
+require "active_prompt"


Manually manipulating $LOAD_PATH is fragile and can lead to hard-to-debug issues. Rails engines should be loaded via proper gem mechanisms or by requiring the file directly.

Consider changing:

$LOAD_PATH.unshift File.expand_path("../../../lib", __dir__) require "active_prompt"

To:

require_relative "../../../lib/active_prompt"

This is more explicit and doesn't modify the global load path.

Suggested change

$LOAD_PATH.unshift File.expand_path("../../../lib", __dir__)

require "active_prompt"

require_relative "../../../lib/active_prompt"

Copilot · 2025-12-08T19:59:36Z

app/assets/config/active_prompt_manifest.js

+//= link_directory ../javascripts/active_prompt .js
+//= link_directory ../stylesheets/active_prompt .css


The manifest file uses link_directory which will link all files in those directories. If you only need specific files (as configured in the engine initializer), consider using link directives for explicit control:

//= link active_prompt/application.js //= link active_prompt/application.css

This makes the manifest consistent with the precompile configuration in engine.rb and prevents accidentally including unwanted files.

Suggested change

//= link_directory ../javascripts/active_prompt .js

//= link_directory ../stylesheets/active_prompt .css

//= link active_prompt/application.js

//= link active_prompt/application.css

Copilot · 2025-12-08T19:59:38Z

app/models/active_prompt/context.rb

+    belongs_to :prompt, class_name: "ActivePrompt::Prompt", inverse_of: :contexts
+
+    validates :agent, :prompt, presence: true
+    validates :label, length: { maximum: 255 }, allow_nil: true


The validation validates :label, length: { maximum: 255 }, allow_nil: true sets a maximum length of 255, but the migration defines label as a string type which typically has a database-level limit of 255 characters anyway in most databases.

While this validation is not harmful, it's somewhat redundant with the database constraint. Consider either:

Removing the validation if relying on database constraints

Or keeping it for better error messages before hitting the database

Copilot · 2025-12-08T19:59:38Z

db/migrate/20251127000000_create_active_prompt_core.rb

+      t.json      :metadata,   null: false, default: {}
+      t.timestamps
+    end
+    add_index :active_prompt_contexts, [:agent_type, :agent_id, :prompt_id], unique: true, name: "idx_ap_contexts_agent_prompt"


The custom index name idx_ap_contexts_agent_prompt is good practice for keeping index names short. However, for consistency, consider also providing custom names for the other composite indexes in this migration (lines 21 and 32) to maintain a uniform naming convention throughout the schema.

Copilot · 2025-12-08T19:59:38Z

lib/generators/active_prompt/install/install_generator.rb

+module ActivePrompt
+  module Generators
+    class InstallGenerator < Rails::Generators::Base
+      source_root File.expand_path("../../../../..", __dir__) # engine root


The source_root is set to File.expand_path("../../../../..", __dir__) which goes up 5 levels from the generator file location. This would point to the parent of the engine root, not the engine root itself.

From lib/generators/active_prompt/install/install_generator.rb:

__dir__ = lib/generators/active_prompt/install

Up 1: lib/generators/active_prompt

Up 2: lib/generators

Up 3: lib

Up 4: engine root ← Should stop here

Up 5: parent of engine root ← Currently pointing here

The correct path should be:

source_root File.expand_path("../../../..", __dir__)

Suggested change

source_root File.expand_path("../../../../..", __dir__) # engine root

source_root File.expand_path("../../../..", __dir__) # engine root

Copilot · 2025-12-08T19:59:38Z

app/models/active_prompt/message.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+module ActivePrompt
+  class Message < ApplicationRecord


Models in the engine inherit from ApplicationRecord, but the engine doesn't define its own ApplicationRecord class. This creates a dependency on the host application's ApplicationRecord, which may not exist or may have different configurations.

Consider adding app/models/active_prompt/application_record.rb:

module ActivePrompt class ApplicationRecord < ActiveRecord::Base self.abstract_class = true end end

Then have all engine models inherit from ActivePrompt::ApplicationRecord instead.

Copilot · 2025-12-08T19:59:39Z

test/test_helper.rb

+rescue => e
+  warn "Warning: failed to clean #{generated_dir}: #{e.message}"


The bare rescue clause catches all exceptions including SystemExit and SignalException, which is an anti-pattern. It should rescue a specific exception type instead.

Change to:

rescue StandardError => e warn "Warning: failed to clean #{generated_dir}: #{e.message}" end

sarbadaj changed the title ~~Create Rails Engine structure for ActivePrompt #243~~ Create the foundational Rails engine structure for ActivePrompt #243 Nov 17, 2025

TonsOfFun requested a review from Copilot December 8, 2025 19:50

Copilot started reviewing on behalf of TonsOfFun December 8, 2025 19:50 View session

Copilot AI reviewed Dec 8, 2025

View reviewed changes

TonsOfFun force-pushed the main branch from 9864a60 to f727c32 Compare December 11, 2025 18:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Create the foundational Rails engine structure for ActivePrompt #243 #277

Create the foundational Rails engine structure for ActivePrompt #243 #277

Uh oh!

sarbadaj commented Nov 17, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 8, 2025

Uh oh!

Copilot AI Dec 8, 2025

Uh oh!

Copilot AI Dec 8, 2025

Uh oh!

Copilot AI Dec 8, 2025

Uh oh!

Copilot AI Dec 8, 2025

Uh oh!

Copilot AI Dec 8, 2025

Uh oh!

Copilot AI Dec 8, 2025

Uh oh!

Copilot AI Dec 8, 2025

Uh oh!

Copilot AI Dec 8, 2025

Uh oh!

Copilot AI Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		belongs_to :prompt, class_name: "ActivePrompt::Prompt", inverse_of: :messages

		enum :role, { system: "system", user: "user", assistant: "assistant", tool: "tool" }, prefix: true

	enum :role, { system: "system", user: "user", assistant: "assistant", tool: "tool" }, prefix: true
	enum :role, %i[system user assistant tool], prefix: true

	get "health", to: proc { [200, { "Content-Type" => "text/plain" }, ["ok"]] }, as: :health
	get "health", to: "health#show", as: :health

		$LOAD_PATH.unshift File.expand_path("../../../lib", __dir__)
		require "active_prompt"

	$LOAD_PATH.unshift File.expand_path("../../../lib", __dir__)
	require "active_prompt"
	require_relative "../../../lib/active_prompt"

		//= link_directory ../javascripts/active_prompt .js
		//= link_directory ../stylesheets/active_prompt .css

	source_root File.expand_path("../../../../..", __dir__) # engine root
	source_root File.expand_path("../../../..", __dir__) # engine root

		rescue => e
		warn "Warning: failed to clean #{generated_dir}: #{e.message}"

Uh oh!

Create the foundational Rails engine structure for ActivePrompt #243 #277

Are you sure you want to change the base?

Create the foundational Rails engine structure for ActivePrompt #243 #277

Uh oh!

Conversation

sarbadaj commented Nov 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants