From 239c8596a7fbd840c9829bf3f83c76ab7183761a Mon Sep 17 00:00:00 2001
From: Christian Hood <christianhood@gmail.com>
Date: Fri, 20 Mar 2026 02:48:32 -0400
Subject: [PATCH] Fix review issues: partial index, middleware scope, add
 timing tests

- Change idx_financing_plans_active to partial index (WHERE active = true)
  to avoid useless B-tree index on low-cardinality boolean column
- Scope timingMiddleware to /api routes only (was globally registered,
  which caused noise from static asset requests)
- Add unit tests for timing middleware (5 tests covering: next() call,
  timing log format, SLOW warning threshold, fast-request path, status code)

Nightshift-Task: perf-regression
Nightshift-Ref: https://github.com/marcus/nightshift
---
 db/migrations/005_performance_indexes.sql     |  2 +-
 .../src/__tests__/timing.middleware.test.js   | 98 +++++++++++++++++++
 server/src/app.js                             |  3 +-
 3 files changed, 100 insertions(+), 3 deletions(-)
 create mode 100644 server/src/__tests__/timing.middleware.test.js

diff --git a/db/migrations/005_performance_indexes.sql b/db/migrations/005_performance_indexes.sql
index 76f5afd..b4376c7 100644
--- a/db/migrations/005_performance_indexes.sql
+++ b/db/migrations/005_performance_indexes.sql
@@ -4,4 +4,4 @@ CREATE INDEX IF NOT EXISTS idx_paycheck_bills_paycheck_id ON paycheck_bills(payc
 CREATE INDEX IF NOT EXISTS idx_actuals_paycheck_id ON actuals(paycheck_id);
 CREATE INDEX IF NOT EXISTS idx_one_time_expenses_paycheck_id ON one_time_expenses(paycheck_id);
 CREATE INDEX IF NOT EXISTS idx_financing_payments_plan_id ON financing_payments(plan_id);
-CREATE INDEX IF NOT EXISTS idx_financing_plans_active ON financing_plans(active);
+CREATE INDEX IF NOT EXISTS idx_financing_plans_active ON financing_plans(active) WHERE active = true;
diff --git a/server/src/__tests__/timing.middleware.test.js b/server/src/__tests__/timing.middleware.test.js
new file mode 100644
index 0000000..f834219
--- /dev/null
+++ b/server/src/__tests__/timing.middleware.test.js
@@ -0,0 +1,98 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+
+const timingMiddleware = require('../middleware/timing');
+
+function makeResMock() {
+  const listeners = {};
+  return {
+    statusCode: 200,
+    on(event, cb) {
+      listeners[event] = cb;
+    },
+    emit(event) {
+      if (listeners[event]) listeners[event]();
+    },
+  };
+}
+
+describe('timingMiddleware', () => {
+  let consoleSpy;
+  let warnSpy;
+
+  beforeEach(() => {
+    consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+    warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    consoleSpy.mockRestore();
+    warnSpy.mockRestore();
+    vi.useRealTimers();
+  });
+
+  it('calls next()', () => {
+    const req = { method: 'GET', path: '/api/health' };
+    const res = makeResMock();
+    const next = vi.fn();
+
+    timingMiddleware(req, res, next);
+    expect(next).toHaveBeenCalledOnce();
+  });
+
+  it('logs timing on response finish', () => {
+    const req = { method: 'GET', path: '/api/health' };
+    const res = makeResMock();
+
+    timingMiddleware(req, res, vi.fn());
+    res.emit('finish');
+
+    expect(consoleSpy).toHaveBeenCalledOnce();
+    const msg = consoleSpy.mock.calls[0][0];
+    expect(msg).toMatch(/\[timing\] GET \/api\/health 200 \d+ms/);
+  });
+
+  it('emits SLOW warning when duration exceeds 200ms threshold', () => {
+    vi.useFakeTimers();
+
+    const req = { method: 'POST', path: '/api/paychecks' };
+    const res = makeResMock();
+
+    timingMiddleware(req, res, vi.fn());
+
+    // Advance time past the threshold
+    vi.advanceTimersByTime(250);
+    res.emit('finish');
+
+    expect(warnSpy).toHaveBeenCalledOnce();
+    const msg = warnSpy.mock.calls[0][0];
+    expect(msg).toMatch(/\[SLOW\] POST \/api\/paychecks/);
+    expect(consoleSpy).not.toHaveBeenCalled();
+  });
+
+  it('does not emit SLOW warning for fast requests', () => {
+    vi.useFakeTimers();
+
+    const req = { method: 'GET', path: '/api/financing' };
+    const res = makeResMock();
+
+    timingMiddleware(req, res, vi.fn());
+
+    vi.advanceTimersByTime(50);
+    res.emit('finish');
+
+    expect(consoleSpy).toHaveBeenCalledOnce();
+    expect(warnSpy).not.toHaveBeenCalled();
+  });
+
+  it('includes status code in the log message', () => {
+    const req = { method: 'GET', path: '/api/bills' };
+    const res = makeResMock();
+    res.statusCode = 404;
+
+    timingMiddleware(req, res, vi.fn());
+    res.emit('finish');
+
+    const msg = consoleSpy.mock.calls[0][0];
+    expect(msg).toContain('404');
+  });
+});
diff --git a/server/src/app.js b/server/src/app.js
index 2deda15..072d487 100644
--- a/server/src/app.js
+++ b/server/src/app.js
@@ -15,9 +15,8 @@ const app = express();
 
 app.use(cors());
 app.use(express.json());
-app.use(timingMiddleware);
-
 // API routes
+app.use('/api', timingMiddleware);
 app.use('/api', healthRouter);
 app.use('/api', configRouter);
 app.use('/api', billsRouter);