Adopt Rails render-partial conventions across the codebase

Created: 2026-05-28 Origin: Spun out of the AppSignal #5422 / #5532 fix on the appsignal-fix branch. The Rails partial features blog post (railsdesigner.com/rails-partial-features) surfaced three convention-based shorthands we’re not using consistently. Status: In progress. First pass landed on feat/render-partial-conventions-sweep branch: 4 additional overrides (ShippingCost, SalesRepQueueEntry, Printer, PrintProfile) plus shorthand callsite cleanup in 4 view files. Triage script (/tmp/partial_triage2.rb in branch history) reclassifies the bulk of the remaining candidates as verbose-required because the partials take a FormBuilder (f:) or other renamed locals — those can’t use the shorthand by design. Remaining safe overrides exhausted; this PR also wires cross-links from the documentation-conventions and view-components skill docs so the convention is discoverable from the usual model/view documentation flow. Scope: View templates + model to_partial_path overrides. No controller or routing changes.

Background

render has three convention-based shorthands that let you drop the explicit partial:, collection:, and locals: arguments:

Verbose form	Convention-based shorthand	What Rails does
render partial: 'sms_message', collection: @sms_messages	render @sms_messages	Looks up to_partial_path on the first element; passes each element as a local matching the partial’s basename
render partial: 'sms_message', locals: { sms_message: @sms_message }	render @sms_message	Same lookup; passes the object as a local matching the partial’s basename
render partial: 'crm/sms_messages/sms_message' (explicit absolute path)	render @sms_messages + to_partial_path override on the model	Move the path-knowledge from every callsite to one place on the model

Adopting these aggressively makes views shorter, decouples view location from caller-controller namespace, and surfaces “this partial wants a weird shape” cases (which become explicit partial:/locals: calls instead of hiding inside a long argument list).

The article also notes that any class that responds to to_partial_path can be passed — not just ActiveRecord models. Useful for presenters, value objects, and Data.define structs that already wrap a domain concept.

Why now

• The AppSignal #5422 fix exposed that render partial: '/crm/sms_messages/sms_message', collection: @sms_messages is a smell — the absolute path is callsite knowledge that belongs on the model. Six models on the appsignal-fix branch already got a to_partial_path override; the other ~45 single-location candidates (see “Inventory” below) are still on the verbose form.
• The tab-content-* streamed-mode sweep (PR #886 → #907) touched 220 tab views. Many of those still pass partial: + locals: explicitly where shorthand would do — easier to clean up now while the diff context is fresh.

Non-goals

• Don’t override to_partial_path blindly on every model whose partial isn’t at the conventional path. Skip when:
  • Multiple partials with the same basename exist at different paths (e.g. SerialNumber has 3 — RMA-context, receipt-context, store-context). The override would pin to one and silently break the others.
  • The partial requires locals beyond the object itself (e.g. locals: { rma: ..., serial_number: ... }). The shorthand only passes the object.
• Don’t rewrite verbose calls just to make them shorter. The verbose form is correct when:
  • The partial isn’t model-named (render partial: 'shared/spinner').
  • Extra locals are needed (render partial: 'sms_message', locals: { sms_message: m, highlight: true }).
  • The collection is empty and a custom as: is needed for a polymorphic render.

Inventory of single-location custom-path candidates (51 models)

Models whose partial lives at exactly one non-conventional path. As of the feat/render-partial-conventions-sweep branch, 10 have the override:

PR #915 (initial 6, shipped with the AppSignal #5422 fix):

• SmsMessage → crm/sms_messages/sms_message
• ActivityAgenda → crm/activity_agendas/activity_agenda
• AssortmentInstruction → crm/assortment_instructions/assortment_instruction
• CspReport → admin/csp_reports/csp_report
• ElementPoleAssignment → crm/element_pole_assignments/element_pole_assignment
• Packing → crm/packings/packing

This sweep (PR #917, 4 more, after refined triage):

• ShippingCost → deliveries/shipping_cost
• SalesRepQueueEntry → sales_rep_queues/sales_rep_queue_entry
• Printer → crm/printers/printer
• PrintProfile → crm/print_profiles/print_profile

Follow-up sweep — DEFERRED partials refactored to take locals instead of reading controller ivars, one of which then qualified for the override:

• Showcase → shared/showcase — partial refactored from @showcase ivar (38 refs) to showcase local; override added because the partial only needs the object itself, so render @showcase works from both crm/showcases/show and www/showcases/show.
• FloorPlanDisplay — partial refactored from 5 ivars to 5 locals (floor_plan_display, assets_by_room, hero_assets, main_room, room_line_items); both callsites updated. No override because the partial needs more than just the object — callers must keep the explicit locals: {…} form, but the ivar dependence is gone.
• BudgetGroup — recursive partial refactored from 2 ivars (@dimensions_by_group, @budget_groups_by_parent) to locals, threaded through the recursion. No override for the same reason as FloorPlanDisplay.

Why “only” 7 distinct changes (5 model overrides + 3 ivar→locals refactors, with Showcase counted in both)

The naïve discovery script flagged ~45 remaining single-location candidates. A refined triage (see /tmp/partial_triage2.rb in branch history — looks at the partial body AND the callsites’ locals: keys) reclassified them into four buckets:

• OVERRIDE (5): Partial uses (or was refactored to use) the conventional local name only; callsites don’t pass extra locals:. ShippingCost, SalesRepQueueEntry, Printer, PrintProfile, and Showcase (after the ivar→local refactor). All landed above.
• VERBOSE (36 + 2): Partial takes a FormBuilder (f:), or other non-conventional locals (a:, of:, is_snowmelt:, …). These cannot use render @x shorthand by design — every caller must pass the form builder/extra locals explicitly. Skip the override. Includes FloorPlanDisplay and BudgetGroup after their refactor — they no longer read ivars but they still take multiple locals. Also ProductCategory (recursive + takes f:) and Kpi (the partial is a report-wide table that uses a plural kpis local, not a single kpi; the path collision is coincidental).
• DEAD (0): All 4 were verified truly unrendered and deleted: _contact_training_topic.html.erb, _warranty_towel_warmer.html.erb, _shipping_account_number.html.erb, _storage_location.html.erb.

To re-run the discovery script:

mise exec -- bundle exec rails runner '
Rails.application.eager_load!
ActiveRecord::Base.descendants.each do |klass|
  next if klass.abstract_class?
  begin
    default_collection = klass.model_name.collection
    default_element    = klass.model_name.element
  rescue
    next
  end
  partial_name = "_#{default_element}.html.erb"
  conventional = Rails.root.join("app/views/#{default_collection}", partial_name)
  next if File.exist?(conventional)
  alternatives = Dir.glob(Rails.root.join("app/views/**/#{partial_name}"))
  next if alternatives.empty? || alternatives.size > 1
  puts "#{klass.name} → #{alternatives.first.sub(Rails.root.to_s + "/app/views/", "").sub(".html.erb", "").sub(%r{/_}, "/")}"
end
'

For each candidate, before adding the override:

1. Grep for render partial: calls that target the partial — the override only saves work if the partial is the canonical one for the model.
2. Check the partial body for required locals beyond the object name.
3. If both pass, add the override with a one-line Why: comment matching the pattern on SmsMessage.

Inventory of verbose-form render callsites

These are calls of the shape:

render partial: 'foo', collection: @foos
render partial: 'foo', locals: { foo: bar }

…that could collapse to render @foos / render bar once the model’s to_partial_path is in place. Find them with:

rg "render partial: ['\"][^/]*['\"]," app/views | rg "(collection|locals):"

The sweep is opportunistic — convert when you’re already touching the file. Don’t open a PR that only does mechanical render-conversions; bundle the cleanup with whatever feature/fix brought you to the file.

Out of scope

• tag.foo(...) vs content_tag(:foo, ...) — covered by the tag-helpers skill, separate sweep.
• Inline view scripts → Stimulus — covered by no-inline-view-scripts skill.
• render Component.new(...) ViewComponent calls — different API surface, not part of the partial conventions.

Done when

• ✅ All 51 single-location custom-path candidates either have a to_partial_path override or are documented as verbose-required / context-specific / dead (see the bucket breakdown above).
• ⏳ A grep for render partial: ['"][^/]*['"] returns only calls that legitimately need extra locals: or are not model-named. Opportunistic cleanup as files are touched for other reasons.
• ✅ The documentation-conventions and view-components skill docs link here from their “see also” section.
• ✅ The 4 DEAD partials confirmed truly unrendered via grep against every render form + every model/controller reference — deleted.
• ✅ The 5 DEFERRED partials each addressed:
  • Showcase — ivar → local refactor (38 refs); override added.
  • FloorPlanDisplay — 5-ivar → 5-local refactor; no override (needs extras beyond the object).
  • BudgetGroup — recursive partial’s 2 ivar lookups → locals threaded through recursion; no override (same reason).
  • Kpi — partial isn’t a model partial (uses plural kpis local for a report-wide table); path collision coincidental, leave as-is.
  • ProductCategory — recursive partial takes a FormBuilder; can’t use shorthand by design, leave verbose.
• ✅ A render-partials skill at .agents/skills/render-partials/SKILL.md documents the conventions so future agents adopt them without needing to read this task doc.