Docs/db redesign v2 perf findings

app/blueprints/rule_blueprint.rb

@@ -12,9 +12,22 @@ class RuleBlueprint < Blueprinter::Base
   identifier :id
 
   # === Default view: fields shared by ALL views ===
-  fields :rule_id, :title, :version, :status, :rule_severity, :locked,
+  fields :rule_id, :version, :status, :locked,
          :review_requestor_id, :changes_requested
 
+  # title + rule_severity resolve through DisplayFallback: the rule's own value
+  # when customized, otherwise the SRG template. A no-op while rules still copy
+  # template content, but lets later phases (7+) null duplicated columns without
+  # breaking the API. Reads the eager-loaded :srg_rule association only when the
+  # rule's own value is blank, so it stays N+1-free for loaded collections.
+  field :title do |rule, _options|
+    rule.display_title
+  end
+
+  field :rule_severity do |rule, _options|
+    rule.display_severity
+  end
+
   # per-rule comment summary surfaced on the navigator + section
   # icon badges so triagers + commenters can spot rules with active
   # work without drilling in. Computed in-memory against the eager-
@@ -59,11 +72,20 @@ class RuleBlueprint < Blueprinter::Base
 
   # === Viewer view: read-only detail ===
   view :viewer do
-    fields :rule_weight, :fixtext, :fixtext_fixref, :ident, :ident_system,
+    fields :rule_weight, :fixtext_fixref, :ident_system,
            :vendor_comments, :vuln_id, :legacy_ids,
            :component_id, :status_justification, :artifact_description,
            :locked_fields
 
+    # fixtext + ident resolve through DisplayFallback (see default view).
+    field :fixtext do |rule, _options|
+      rule.display_fixtext
+    end
+
+    field :ident do |rule, _options|
+      rule.display_ident
+    end
+
     field :nist_control_family do |rule, _options|
       rule.nist_control_family
     end

app/controllers/application_controller.rb

@@ -10,6 +10,10 @@ class ApplicationController < ActionController::Base
   before_action :check_access_request_notifications
   before_action :check_locked_user_notifications
 
+  # Upper bound on the pending-access-requests list rendered into the navbar
+  # dropdown on every HTML request. See check_access_request_notifications.
+  NAVBAR_ACCESS_REQUESTS_CAP = 50
+
   # AC-8: Determines if the current user must acknowledge consent.
   # Returns true when consent is enabled and the session has no valid acknowledgment.
   def consent_required?
@@ -380,9 +384,15 @@ def check_access_request_notifications
 
     # Single query: find all access requests for projects where current user is admin.
     # Replaces N+1 loop that called can_admin_project? + eager_load per project.
+    # `.limit(NAVBAR_ACCESS_REQUESTS_CAP)` bounds the per-request cost on the super-admin
+    # branch, which previously loaded the whole table on every HTML page (Problem 12
+    # in docs/plans/DATABASE-COMPLETE-REDESIGN-v2.md). Newest-first so the recent
+    # asks the admin would actually action stay visible at the cap.
     pending_requests = if current_user.admin?
                          # Super admins see all pending requests — no need to pluck project IDs
                          ProjectAccessRequest.eager_load(:user, :project)
+                                             .order(created_at: :desc)
+                                             .limit(NAVBAR_ACCESS_REQUESTS_CAP)
                        else
                          admin_project_ids = Membership.where(user_id: current_user.id, role: 'admin',
                                                               membership_type: 'Project')
@@ -391,6 +401,8 @@ def check_access_request_notifications
 
                          ProjectAccessRequest.where(project_id: admin_project_ids)
                                              .eager_load(:user, :project)
+                                             .order(created_at: :desc)
+                                             .limit(NAVBAR_ACCESS_REQUESTS_CAP)
                        end
 
     @access_requests = pending_requests.map do |ar|

app/controllers/components_controller.rb

@@ -550,7 +550,7 @@
   # banner (CommentPeriodBanner) and any per-rule callouts have the
   # accurate count. Without this, the blueprint defaults to zero.
   def blueprint_render_options
     review_ids = @component ? Review.joins(:rule).merge(Rule.where(component_id: @component.id)).pluck(:id) : []
     {
       pending_comment_counts: Component.pending_comment_counts([@component.id]),
       reactions_summary: Reaction.summary(review_ids, current_user&.id)
@@ -583,10 +583,14 @@
 
   # Defines the set_component method.
   def set_component
-    # Loads a Component object with associated rules, reviews,
-    # descriptions, checks and additional answers where ID is equal to params id.
-    @component = Component.eager_load(
-      rules: [:reviews, :disa_rule_descriptions, :rule_descriptions, :checks,
+    # preload (separate queries) NOT eager_load (single LEFT OUTER JOIN). Multiple
+    # sibling has_many's on Rule create a cartesian product per rule — N reviews
+    # x M checks x ... — that scales with comment volume. Nested review authors
+    # (user/triage_set_by/adjudicated_by) avoid an N+1 in ReviewBlueprint via
+    # RuleBlueprint :editor. See docs/plans/DATABASE-COMPLETE-REDESIGN-v2.md Problem 11.
+    @component = Component.preload(
+      rules: [{ reviews: %i[user triage_set_by adjudicated_by] },
+              :disa_rule_descriptions, :rule_descriptions, :checks,
               :additional_answers,
               { satisfies: :srg_rule },
               { satisfied_by: :srg_rule },

app/models/component.rb

@@ -89,6 +89,34 @@
 
   COMMENT_PHASES = %w[open closed].freeze
   CLOSED_REASONS = %w[adjudicating finalized].freeze
+
+  # Aggregate pending-comment counts per component, union over rule-scoped
+  # (commentable_type='BaseRule') AND component-scoped (commentable_type=
+  # 'Component') reviews. Two ? placeholders, one per UNION branch. See
+  # .pending_comment_counts.
+  PENDING_COMMENT_COUNTS_UNION_SQL = <<~SQL.squish.freeze
+    SELECT component_id, COUNT(*) AS cnt FROM (
+      SELECT base_rules.component_id
+      FROM reviews
+      INNER JOIN base_rules ON base_rules.id = reviews.commentable_id
+      WHERE reviews.commentable_type = 'BaseRule'
+        AND reviews.action = 'comment'
+        AND reviews.responding_to_review_id IS NULL
+        AND reviews.triage_status = 'pending'
+        AND base_rules.component_id IN (?)
+      UNION ALL
+      SELECT reviews.commentable_id AS component_id
+      FROM reviews
+      WHERE reviews.commentable_type = 'Component'
+        AND reviews.action = 'comment'
+        AND reviews.responding_to_review_id IS NULL
+        AND reviews.triage_status = 'pending'
+        AND reviews.commentable_id IN (?)
+    ) AS pending
+    GROUP BY component_id
+  SQL
+  private_constant :PENDING_COMMENT_COUNTS_UNION_SQL
+
   validates :comment_phase, inclusion: { in: COMMENT_PHASES }
   validates :closed_reason, inclusion: { in: CLOSED_REASONS }, allow_nil: true
   validate  :closed_reason_only_when_closed
@@ -582,9 +610,20 @@
   end
 
   def reviews
-    rule_names = rules.pluck(:id, :rule_id).to_h.transform_values { |rid| "#{prefix}-#{rid}" }
-    Review.where(rule_id: rule_names.keys).order(created_at: :desc).limit(20).as_json.map do |review|
-      review['displayed_rule_name'] = rule_names[review['rule_id'].to_i]
+    rule_id_to_displayed = rules.pluck(:id, :rule_id).to_h.transform_values { |rid| "#{prefix}-#{rid}" }
+    # Polymorphic union: reviews on this component's rules OR on the component
+    # itself (commentable_type='Component'). The pre-polymorphic filter
+    # `rule_id IN (...)` silently dropped component-scoped reviews shipped in
+    # #729. See docs/plans/DATABASE-COMPLETE-REDESIGN-v2.md Problem 13.
+    rule_id_subquery = rules.select(:id)
+    rule_scoped = Review.where(commentable_type: 'BaseRule', commentable_id: rule_id_subquery)
+    component_scoped = Review.where(commentable_type: 'Component', commentable_id: id)
+    rule_scoped.or(component_scoped).order(created_at: :desc).limit(20).as_json.map do |review|
+      review['displayed_rule_name'] = if review['commentable_type'] == 'Component'
+                                        '(component)'
+                                      else
+                                        rule_id_to_displayed[review['rule_id'].to_i]
+                                      end
       review
     end
   end
@@ -602,13 +641,15 @@
   def self.pending_comment_counts(component_ids)
     return {} if component_ids.blank?
 
-    Review.where(action: 'comment',
-                 responding_to_review_id: nil,
-                 triage_status: 'pending')
-          .joins(:rule)
-          .merge(Rule.where(component_id: component_ids))
-          .group('base_rules.component_id')
-          .count
+    # Polymorphic union: pending top-level comments on this component's rules
+    # OR on the component itself. The pre-polymorphic `.joins(:rule)` INNER
+    # JOIN dropped commentable_type='Component' rows (rule_id IS NULL); see
+    # docs/plans/DATABASE-COMPLETE-REDESIGN-v2.md Problem 13. Mirrors the
+    # REVIEW_COMPONENT_UNION_BODY pattern on Project — single SQL statement
+    # so callers' query-count assertions stay green.
+    sql = sanitize_sql_array([PENDING_COMMENT_COUNTS_UNION_SQL, component_ids, component_ids])
+    rows = connection.exec_query(sql)
+    rows.each_with_object({}) { |r, h| h[r['component_id']] = r['cnt'] }
   end
 
   # Backs GET /components/:id/comments — the triage table.
@@ -616,7 +657,7 @@
   # On-the-wire vocabulary is DISA-native: triage_status keys (concur,
   # non_concur, ...) and XCCDF section keys (check_content, fixtext, ...).
   # The frontend translates to friendly labels via triageVocabulary.js.
   def paginated_comments(triage_status: 'all', section: nil, rule_id: nil, # rubocop:disable Metrics/ParameterLists
                          author_id: nil, query: nil, page: 1, per_page: 25,
                          resolved: 'all', commentable_type: nil)
     page = [page.to_i, 1].max
@@ -641,7 +682,7 @@
     scope = scope.where(commentable_type: 'BaseRule', commentable_id: rule_id) if rule_id.present?
     scope = scope.where(user_id: author_id) if author_id.present?
 
     case resolved.to_s
     when 'true'  then scope = scope.where.not(adjudicated_at: nil)
     when 'false' then scope = scope.where(adjudicated_at: nil)
     end

app/models/concerns/display_fallback.rb

@@ -0,0 +1,86 @@
+# frozen_string_literal: true
+
+# DisplayFallback implements the "prefer the rule's own value, fall back to the
+# SRG template" pattern central to the DB 3NF redesign
+# (docs/plans/DATABASE-COMPLETE-REDESIGN-v2.md, Phase 1).
+#
+# A Rule starts life as a copy of its SRG template. As later phases stop copying
+# template content, a rule's own column may be NULL — meaning "unchanged from the
+# template." These display_* methods resolve that transparently so views,
+# blueprints, and exports never need to know whether a value was customized.
+#
+# Resolution order for every field:
+#   1. the rule's own column value (a user override), if present
+#   2. an explicit `<field>_override` accessor, if the model defines one (Phase 3)
+#   3. the SRG template value via the :srg_rule association
+#
+# Always load collections through `.with_display_fallbacks` to avoid an N+1 on
+# :srg_rule when calling display_* over many rules.
+module DisplayFallback
+  extend ActiveSupport::Concern
+
+  # Fields whose canonical value may live on the SRG template.
+  OVERRIDABLE_FIELDS = %i[title fixtext ident rule_severity].freeze
+
+  included do
+    scope :with_display_fallbacks, -> { includes(:srg_rule) }
+  end
+
+  def display_title
+    display_field(:title)
+  end
+
+  def display_fixtext
+    display_field(:fixtext)
+  end
+
+  def display_ident
+    display_field(:ident)
+  end
+
+  def display_severity
+    display_field(:rule_severity)
+  end
+
+  # Generic resolver. Reads the rule's own attribute first, then an optional
+  # `<field>_override` accessor (added in Phase 3), then the SRG template.
+  #
+  # Uses respond_to? guards rather than `rescue nil` so a genuine programming
+  # error (e.g. a typo'd field) surfaces instead of being silently swallowed.
+  def display_field(field)
+    own = self[field] if has_attribute?(field.to_s)
+    return own if own.respond_to?(:presence) ? own.presence : own
+
+    override_method = "#{field}_override"
+    if respond_to?(override_method, true)
+      override = public_send(override_method)
+      return override if override.present?
+    end
+
+    return nil unless srg_rule.respond_to?(field)
+
+    srg_rule.public_send(field)
+  end
+
+  # True if any overridable field diverges from the SRG template.
+  def has_overrides?
+    return false if srg_rule.nil?
+
+    OVERRIDABLE_FIELDS.any? do |field|
+      own = self[field]
+      own.present? && own != srg_rule.public_send(field)
+    end
+  end
+
+  # Compact description of what this rule customizes from its template.
+  def override_summary
+    {
+      rule_id: id,
+      srg_requirement: srg_rule&.version,
+      overridden: OVERRIDABLE_FIELDS.index_with do |field|
+        own = self[field]
+        own.present? && srg_rule.present? && own != srg_rule.public_send(field)
+      end
+    }
+  end
+end

app/models/rule.rb

@@ -6,6 +6,7 @@
 # Benchmark XCCDF.
 class Rule < BaseRule
   include PgSearch::Model
+  include DisplayFallback
 
   attr_accessor :skip_update_inspec_code
 
@@ -156,7 +157,7 @@
   #    audit_id (integer) - A specific ID for an audited record
   #    field (string) - A specific field to revert from the audit record
   #
   def self.revert(rule, audit_id, fields, audit_comment)
     audit = rule.own_and_associated_audits.find(audit_id)
 
     # nil check for audit