3,853 Insertions Across 20 Files and Not a Single New Framework

One feature branch, 3,853 insertions across 20 files, one merge. That commit added five analytics surfaces to our SaaS admin: a Package Report, a Conversion Report, a Renewal Report, a Conversion-Renewal Report, and a Sales-Lifetime view. The admin they live in is a long-running legacy application on IIS over MSSQL. No React, no BI tool bolted on the side, no separate analytics service. Just more pages, written to the same conventions as the pages that shipped when the app launched.

You can grow a real business-intelligence surface inside an application like this and keep it maintainable, on two conditions. Commit to one page pattern, and stop treating review SQL as throwaway query-window snippets.

One three-file shape for every report

Every report here has the same three files, and that consistency is the whole trick.

A data endpoint runs the query and returns rows. A client-side script, around 580 lines, fetches from the endpoint, builds the table, wires the drill-downs, and handles the filter UI. A CSS source file holds the layout. The page wrapper is thin: it pulls in the includes and hands off to the client.

All of it follows a convention we lean on hard across this admin, the page-object pattern. Each page is a Page.prototype object with its init, its event handlers, and its render methods hanging off one prototype. It is plain object-oriented JavaScript, nothing fancy, but every report in the suite reads the same way. If you have touched one, you can find your way around the next in about thirty seconds. New report? Copy the shape, swap the query and the columns.

That makes “add another analytics page” a bounded task instead of a research project. The biggest enemy of a legacy codebase is five different ways to do the same thing, each invented by whoever was tired that week. One enforced pattern is worth more than any framework.

The exclusion filters every query needs

The hard part of reporting is never the SQL syntax. It is working out which rows are real.

Revenue and expiry analytics here run over the customers table joined to the org-structure table. The naive version of “how many active paid accounts do we have” is a COUNT(*) with a WHERE Expires > GETDATE(). That number is wrong, and wrong in ways that make you look foolish in a meeting.

Every report walks the same gauntlet of exclusions:

• Exclude trial accounts (a trial flag on the customer row). Trials inflate every count and none of them are revenue.
• Exclude internal test groups (a test-group flag). We have a pile of test orgs made over the years, and they are indistinguishable from real customers unless you filter them out explicitly.
• Exclude non-active org-structure nodes. The join is to org structure, and only nodes with active status are live customers. Inactive ones are deleted, suspended, or half-migrated.
• Only then apply Expires > today for the “currently paid” cut.

Miss any one of those and the conversion or renewal figure can be materially wrong. Those four filters are the institutional knowledge the SQL encodes, and they belong in every query that touches this data. That is why we stopped throwing the queries away.

Committing the review query next to the report code

The thing we are most glad we did on this branch is a hand-written review query committed to the repo right next to the report code.

It answers a specific question from the renewal analysis: accounts that expired in a given window and have at least one paid transaction. A correlated subquery pulls a last_paid_before_expires date per account, so we can see who lapsed despite having paid us before. That is the population that matters for renewal outreach, and it is fiddly SQL with the same trial/test/active gauntlet baked in.

It took us too many years to internalize that this query is not scratch work. It is the definition of “lapsed paying customer” for this business, written in the one language that cannot hand-wave. In a query window on a desktop, it dies when the tab closes, and the next person (or us, in four months) reinvents it slightly differently and gets a slightly different number. In the repo it is version-controlled, diffable, re-runnable, and reviewable. When someone asks “wait, how did you count that,” the answer is a file path, not our memory.

Checked-in review SQL turns “we ran a query once and got a number” into “here is the exact, reproducible definition that produced the number.” For internal analytics, where the whole value is trusting the number, that separates a report people act on from a report people second-guess.

We now treat every non-trivial analytics query this way. If we had to think about the filter gauntlet to write it, it gets checked in.

Corrupt rows in old production data

Years of production mean years of edge cases that no constraint caught at the time.

The org-structure table has corrupt rows. Nodes with broken parent references, nodes with priority values that should never exist, nodes that are half-deleted in a way the schema permits but the application logic does not expect. In day-to-day use these are invisible, because the normal pages either never touch them or fail silently on one record. A report does not have that luxury. It aggregates across everything, so a corrupt node lands in your SUM, and your endpoint either crashes or returns a garbage total.

So the report endpoint threads a defensive corruption-repair routine through its processing. It is not glamorous. It detects the known-bad shapes and repairs or skips them before they hit the aggregation, so a single bad row cannot take down the whole report or quietly poison the math. This is the unspoken tax on reporting over old data: a meaningful chunk of the code exists to survive the inputs rather than compute the answer. On a profitable old app the corrupt rows are not going anywhere, so the report has to cope with them.

Why this shipped as one feature branch

The release shape was a deliberate choice. This admin ships through direct branch merges and a build server, with no pull requests, and on a normal day we cut tightly scoped hotfix versions, one ticket per tag, so a rollback is a single-ticket rollback.

This was not a normal day. Five net-new pages, zero changes to existing production behavior, nothing an existing customer flow could regress. So it rode a feature branch into develop and shipped on the next minor version. Hotfix-per-ticket fits when you are touching live behavior and a bad merge could break checkout. One feature branch fits when the work is purely additive and the worst case is a new admin page that renders wrong. The release unit should follow the blast radius, and a rule that ignores what the change can actually break is just superstition.

What to check before trusting a report number

When a report number drives a decision, find out which exclusions it applies (trial accounts, internal test groups, non-active org-structure nodes, the expiry cut) and where the query that defines it is stored. A definition kept in a version-controlled file can be reproduced and reviewed, and one left in a query window cannot. If the report runs over old production data, budget for the code that repairs or skips corrupt rows too.

dxdev LESSON - paste into your AI coding agent
LESSON: Check in your review SQL - it is the canonical definition, not scratch work
SOURCE: dxdev.com/blog/reporting-suite-inside-legacy-asp-checked-in-review-sql

WHAT HAPPENED:
An analytics suite was built on top of a long-running legacy codebase.
Every query had to walk the same multi-filter gauntlet (trial accounts, internal test rows, inactive nodes, expiry date) to produce a number anyone could trust.
One-off queries in a query window die when the tab closes; the next reader re-derives them slightly differently and gets a slightly different number.
Committing the review SQL next to the report code turns "we ran a query once" into a version-controlled, reproducible definition that can be diffed, cited by file path, and re-run by anyone.

THE RULE: If you had to think carefully about the filter logic to write a query, that query is a business definition and belongs in the repo.

CHECK MY CODE, then report PASS / FAIL with file:line for each:
  1. Any non-trivial analytics or reporting query lives only in a query window, a comment, or a chat log rather than a checked-in file.
  2. Aggregation queries that touch user or revenue data omit one or more exclusion filters (test accounts, trial accounts, soft-deleted or inactive rows).
  3. A report endpoint aggregates over a table that has known data-quality issues without a defensive pre-processing or skip step before the aggregation.

THEN PRINT: a table (check, PASS/FAIL, evidence, fix) + a verdict (applies / partially / OUT_OF_SCOPE / no) + the single most important next action.