Skip to content

Prosopite N+1 detection is inert: fix the harness and enforce it per directory #7101

Description

@suttondemlong

Summary

Prosopite is in the Gemfile and wired into the test suite, but it currently detects nothing, and there are a number of real N+1 queries behind that. This issue proposes fixing the detection harness and then clearing the N+1s directory by directory.

Why nothing is detected today

spec/.prosopite_ignore lists every spec directory (spec/models, spec/services, spec/lib, spec/system, spec/requests, spec/controllers, spec/views, spec/decorators, spec/policies, spec/datatables), so no example is enforced.

There is also a mismatch between what the harness documents and what it does. The comment in spec/.prosopite_ignore says ignored directories are "scanned but won't raise, only log", but in spec/support/prosopite.rb an ignored example runs with Prosopite.enabled = false, so it is not scanned at all and nothing is written to log/prosopite.log either.

Two further gaps stop enforcement from being usable as soon as a directory is un-ignored:

  1. The factory pause targets the wrong module. It patches FactoryBot::SyntaxRunner, but spec/support/factory_bot.rb does config.include FactoryBot::Syntax::Methods, so create called from a spec or a let block never goes through the patch. Every created record's uniqueness check then looks like an N+1.
  2. Specs get blamed for their own loops. Prosopite reports every repeated query in an example, including ones the spec itself causes by iterating records to build an expected value. Those are not application N+1s but they still fail the example.

Separately, some operations write one record at a time by design (CSV import, org default contact types/topics/hearing types, bulk supervisor assignment, copying a draft contact to several cases). Each iteration runs its own INSERT plus that row's belongs_to presence checks, so they read as an N+1 no matter how much is preloaded — there is no collection to eager load. These need to be declared as intentional rather than "fixed".

N+1s found

Measured by running the suite with detection forced on and attributing each report to the app frame that caused it. The most significant:

  • VolunteerDatatable runs three queries per row (made_contact_with_all_cases_in_days?, hours_spent_in_days, languages).
  • CasaCase#next_court_date and #most_recent_past_court_date are scoped queries on the court_dates association, so they re-run per case and silently defeat any includes(:court_dates). This also affects the missing-data report.
  • CaseContactsContactDates walks a joins-only relation calling #contact_type and #case_contact, costing two queries per interviewee row during court report generation.
  • Missing eager loads on collections rendered per row: the case group form (assigned volunteers for every case in the org), the court report case picker, Supervisor#volunteers (Volunteer#supervisor is its own has_one :through, so preloading the join row does not satisfy it), notifications' patch note types, the all-CASA dashboard's per-org counts, the contact type "last logged" hint, and the new contacts table's row policy (casa_org / creator_casa_org).

Proposed approach

Fix the harness first, then un-ignore directories as they are cleaned, so the ignore list shrinks over time instead of hiding everything.

  • Part 1 (this issue): fix the harness; clear spec/datatables and spec/requests; drop those two from the ignore list.
  • Later parts: the remaining directories. spec/system, spec/services and spec/models still have known N+1s, including the CSV export services (placement, mileage, followup, learning hours) and the scheduled reminder jobs.

Out of scope, worth separate issues

Two pre-existing bugs turned up while measuring. Both are behaviour changes, so they should not ride along in a performance change:

  • Volunteer#cases_where_contact_made_in_days(...).count counts contact rows rather than distinct cases, so a volunteer with more contacts than active cases reads as not having reached everyone in made_contact_with_all_cases_in_days?.
  • The volunteers index sorts on a 60-day, creator-keyed hours figure that ignores case activity, while displaying a 30-day active-case figure under a header that reads "Hours (30 days)".

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions