docs(investigations): consolidate findings for #1097, #1096, #1094 - #1179
Merged
vjuliaife merged 1 commit intoAug 29, 2026
Merged
Conversation
…uliaife#1094 Adds docs/investigations/*.md for the three performance-investigation issues left open after vjuliaife#1173, which linked them as related context but did not close them. Consolidates the findings already posted as GitHub comments on each issue into the same docs/investigations/ format used for vjuliaife#1090 and vjuliaife#1093. - compliance-flags-listing-at-scale.md (vjuliaife#1097): EXPLAIN ANALYZE at 1x/10x shows flat latency (1.05ms to 1.16ms); existing composite index already covers the query shape. No action needed. - compliance-dashboard-aggregation-at-scale.md (vjuliaife#1096): all 8 cold-cache sub-queries profiled at 1x/10x; cost scales linearly with table size as expected for unfiltered aggregates, worst case ~12ms at 10x. No urgent action; two candidate indexes documented for future consideration once real production selectivity can be checked. - importers-fulltext-search-at-scale.md (vjuliaife#1094): confirms the migration's tsvector column and GIN index are correctly built and perform well, but no route in the codebase actually queries them yet. The search feature described by the issue was never wired up. Recommends a follow-up feature issue rather than bundling a new endpoint into this investigation.
|
@Martha-code-dev Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
closes #1097
closes #1096
closes #1094
Completes the investigation-only follow-up for the three issues PR #1173 linked as "relates to" but did not close (#1173 fixed the one issue with an actual code bug, #1095, and left #1094/#1096/#1097 as investigation-only since no code changes were warranted for them at the time — see that PR's body). Each of these three issues' acceptance criteria only asked to benchmark, measure, document, and report findings — no code fix was requested. All three were already fully investigated (findings posted as comments on each issue); this PR consolidates those findings into
docs/investigations/*.md, matching the format already used for #1090 and #1093, so there's a durable doc rather than only a comment thread.No functional code changes in this PR — docs only.
Findings
#1097 —
GET /compliance/flagslisting at 10x volumedocs/investigations/compliance-flags-listing-at-scale.mdEXPLAIN (ANALYZE, BUFFERS)at 1x (1500 flags) vs 10x (15000 flags): latency essentially flat, 1.05ms → 1.16ms. The existing composite indexidx_compliance_flags_surety(surety_id, created_at DESC)already matches this query'sWHERE+ORDER BYshape exactly, so Postgres never scans more than the requested page. Filtered variants (severity/resolution_status/importer_id) can only be faster, verified directly forresolution_status = 'open'. Payload size is bounded by the enforcedlimit <= 100. No action needed at current or 10x volume; cursor-based pagination is flagged only as a future consideration if a singlesurety_idever accumulates tens of thousands of flags (OFFSETdepth is the only theoretical degradation mode, and it isn't hit at these volumes).#1096 —
GET /compliance/dashboardaggregation cost as volume growsdocs/investigations/compliance-dashboard-aggregation-at-scale.mdProfiled all 8 cold-cache sub-queries at 1x vs 10x. All are
Seq Scans (structurally unavoidable — several filter onresolution_statusalone, which isn't covered by anysurety_id-scoped index). Buffers scale ~linearly with table size (~10x for a ~10x row increase), confirming O(n) cost per sub-query. Worst measured case at 10x: ~12ms. Linear extrapolation to 100x suggests ~100-150ms cold-miss latency — not measured directly, disclosed as extrapolation, not a load test. The 5-minute per-admin cache TTL bounds cold-misses to at most one per admin per 5 minutes regardless of traffic, so total query load stays low regardless of hit-ratio (which isn't independently measurable without a running server — disclosed rather than guessed). No urgent action needed. Two candidate follow-ups documented for before ~50-100x volume: an incremental KYC-count summary table (reusing theimporter_metrics_mvpattern already in this codebase) and a plain index oncompliance_flags.resolution_status— both left as documented recommendations rather than implemented here, since their value depends on production data's actual open/resolved ratio, and a low-selectivity index that goes unused just adds write overhead for nothing (the same failure mode already documented foridx_importers_created_atin #1095/#1173).#1094 — importers full-text search at 10x volume
docs/investigations/importers-fulltext-search-at-scale.mdPrimary finding: the search feature this issue describes doesn't exist in the codebase. Migration
006_importers_fulltext_search.sqlcorrectly addedlegal_name_tsv(aGENERATED ALWAYS AS (...) STOREDcolumn) and its GIN index, but no route anywhere inapps/api/srcactually queries it —GET /has no search parameter at all. So "trigger-based tsvector maintenance" (per the issue's AC) doesn't apply either — it's a stored generated column, not trigger-maintained; confirmed no tsvector-related trigger fires on insert. Benchmarked the query the feature would use once built: at 10x (5000 rows), the planner correctly prefers aSeq Scan(1.02ms) over the GIN index (2.83ms when forced) for a ~4%-selectivity two-word search — expected, correct cost-based behavior, not a bug. Index size is ~40% of table size (600KB / 1.46MB), a normal ratio for GIN. Recommendation: building the actual search endpoint is a new feature, not a performance fix, so it's out of scope here — flagging it as a separate feature-issue candidate rather than bundling a new endpoint into this investigation PR.What's not in this PR
No index or code changes are made for any of the three issues. Every investigated query already performs acceptably at 10x, and the two speculative index candidates for #1096 are documented as recommendations pending real production selectivity data, not implemented — see that doc's reasoning.
Test plan
EXPLAIN (ANALYZE, BUFFERS)captured for every query analyzed; the underlying migrations (006_importers_fulltext_search.sql, the compliance-flags/dashboard indexes) were not modified.