Repository navigation
feat(rails): replace Rails rules with per-layer MVC conventions - #15
Conversation
ecaca7f to
38d4059
Compare
tms-minhtang1
left a comment
There was a problem hiding this comment.
🤖【AI REVIEW】Overview
Cảm ơn bạn 🙏 Toàn bộ thay đổi của PR đã được review tại commit 2929571. Bạn trả lời ngay trong thread của từng finding nếu đồng ý hoặc không đồng ý.
🔵 SUGGESTION
🔵 Hai tài liệu ngoài diff vẫn mô tả bộ rule Rails cũ.
knowledge/README.md:144: bảngagent-stack catalogvẫn ghi dòng rails là3rule,29skill; sau PR này là 25 rule, 30 skill (26 global + 4).README.md:183-192: ví dụ checklist liệt kêrails-activerecord,rails-conventions,rails-ruby(cả ba bị xoá ở PR này) dướiRules (3).
Cách sửa — chạy agent-stack catalog và npm run try -- --framework rails, chép số thật vào hai chỗ trên, và đổi ví dụ checklist sang id đang có (ví dụ rails-models, rails-structure, rails-style).
tms-minhtang1
left a comment
There was a problem hiding this comment.
🤖【AI REVIEW】Overview
Cảm ơn 🙏 Toàn bộ thay đổi của PR đã được review tại commit 40657c8. Bạn trả lời ngay trong từng comment nếu không đồng ý hoặc đã sửa.
- validations: forms use custom validators for existence and uniqueness; uniqueness: is Active Record only. The 422 error carries the form. - controllers: HTML create/update re-render new/edit from the error's form. - lib: the core_ext initializer declares CoreExt namespaces before require. - models, structure: models never enqueue jobs; the service does after commit. - authorization: skip_authorization / skip_authorization_check for actions whose permission is the lookup or authentication alone. - migrations, models, validations: SQLite bounds strings by check constraint. - errors: a message is for the log and tracker, never rendered. - security: concurrent tests turn transactions off for their group only. - queries, performance: loading rules live once, in queries. - enumerize: configs read with fetch. rails-doctor checklist: Gemfile. - README, knowledge/README: current Rails rule ids and counts.
|
Về finding |
tms-minhtang1
left a comment
There was a problem hiding this comment.
LGTM 🌟 (commit d28f574)
tms-minhtang1
left a comment
There was a problem hiding this comment.
🤖【AI REVIEW】Overview
Cảm ơn bạn 🙏 Toàn bộ thay đổi của PR đã được review tại commit deb28e4. Bạn trả lời ngay trong thread của từng finding nếu đã sửa hoặc không đồng ý.
- security, services: a constraint-guarded insert runs in a savepoint, tries the next slot and raises sold-out when none is left; the lock bullet accepts a constraint. The concurrent spec releases its threads through a gate, with records created before. - specs: system specs typed by location, no hand-written type:. - tailwind: entry file for both install paths; app/assets/builds/ is build output. - rails-doctor checklist: gems count from the Gemfile, never the lock.
tms-minhtang1
left a comment
There was a problem hiding this comment.
LGTM 🌟 (commit ed539d6)
Rails projects got no conventions from agent-stack beyond the imported global layer. This adds 29 rules under knowledge/rules/framework/rails/ (structure, server layers, cross-cutting concerns, Hotwire, JS packages, and gem-gated rules for decorators, view components, enumerize, soft delete, Tailwind and Bootstrap) and four skills: rails-feature, rails-migration, rails-debug-n-plus-one and rails-doctor, which fills a project's AGENTS.md from its own code and reports rules that conflict with it. Every rule but structure is path-scoped, so it loads only when Claude touches a matching file and adds nothing to session start.
ed539d6 to
eeb4a7d
Compare
Purpose
The three generic Rails rules gave advice our Rails projects don't follow. This PR replaces them with 29 rules, one per layer or gem, and four procedure-only skills, so Claude writes code the way the team does: thin controllers, service objects, form objects, Hotwire on the front end. Markdown only;
tests/pipeline.test.tsuses the new rule ids.What changes
rails-structure: where each kind of code goes, the call flow, read a layer's rule before writing it, ask the user via AskUserQuestioncontrollers,services,validations,authorization,errors,models,queries,search,migrations,jobs,json,views,i18n,config,libsecurity,performance,specs,style,commentshotwire(Turbo, Stimulus),javascript-packages(importmap or jsbundling)decorators,view-components,enumerize,soft-delete,tailwind,bootstraprails-feature,rails-migration,rails-debug-n-plus-one(procedure only, conventions stay in the rules),rails-doctor(typed only: fillsAGENTS.md, reports rules that don't fit the project)Cost
Every rule except
rails-structurehaspaths:and loads once per session, when Claude first touches a matching file.Result
Rules generated into fresh Rails 8.1 apps; each task ran in a clean
claude -psession and its diff was graded against the rules by a separate session.claude-opus-5-5)ApplicationService, form objects, specs split by model and endpoint, scopedshared_context, locales split by scope, Rails-named indexes,frozen_string_literal, helpers by topic, a concurrent spec for a limited shared resource.data-action, broadcasts from the service, no interpolated or!importantclasses, a baseApplicationController,dom_id(Model, :list)for collection frames, browser-formatted text throughIntl.Not exercised: jsbundling and the
tailwindcss-railsgem path (both front-end apps use importmap, and Tailwind from npm).Verification
npm run check: typecheck, 120/120 tests, bundle.dist/unchanged.npm run try -- --framework railsemits 29 Rails rules and 4 Rails skills.