M001: Baseline Quality — TS fixes, refactoring, UI consistency, tests, performance audit

[TerrifiedBug]

Summary

Establishes a clean, maintainable codebase baseline before building new features.

What Changed

S01: TypeScript Fixes & Shared Utilities

• Extracted 7 duplicated utility functions from 10 consumer files into 3 shared modules (src/lib/pipeline-status.ts, src/lib/format.ts, src/lib/status.ts)
• Zero inline duplicate definitions remain

S02: Router & Component Refactoring

• Split 5 over-target files: alerts page (1910→45 lines), pipeline router (1318→847), dashboard router (1074→652), team-settings (865→747), users-settings (813→522)
• Created 2 service modules: pipeline-graph.ts (5 exports), dashboard-data.ts (3 exports)
• Extracted alert page into 4 section components + constants
• Extracted settings dialogs into dedicated files

S03: UI Consistency Sweep

• Created shared EmptyState and QueryError components
• Wired into 30 dashboard pages — every data-fetching page now has consistent loading, empty, and error states
• Zero inline border-dashed empty state patterns remain

S04: Foundational Test Suite

• Set up Vitest 4.1.0 + vitest-mock-extended
• 105 tests across 7 files: auth (TOTP + crypto), pipeline CRUD (graph + dashboard data), deploy operations, alert evaluation, pipeline utilities
• Established Prisma deep-mock pattern for service-level testing

S05: Performance Audit & Optimization

• Installed @next/bundle-analyzer
• Fixed Prisma client runtime leak into browser bundle (import type for enums)
• Scoped allComponentNodes query to user pipeline IDs (eliminates full-table scan)
• Produced formal audit report with deferred recommendations

Cleanup

• Removed .gsd/ and .bg-shell/ agent artifacts from git tracking (belong in global gitignore)

Verification

Check	Result
tsc --noEmit	✅ exit 0
eslint src/	✅ exit 0
pnpm test (105 tests)	✅ all passing
No non-exempt file over ~800 lines	✅
No inline duplicate utilities	✅
EmptyState in 17 files, QueryError in 27 files	✅
Bundle analysis report exists	✅

Stats

• 61 source files changed (excluding .gsd/ cleanup)
• +7,662 / −3,578 lines
• 105 tests added
• 9 requirements validated (R001–R008, R010)

[greptile-apps]

Greptile Summary

This PR establishes a clean maintainability baseline across 61 source files: extracting duplicated utilities into shared modules, splitting over-target files into focused components and services, wiring consistent EmptyState/QueryError UI across 30 dashboard pages, bootstrapping a 105-test Vitest suite, and running a formal bundle/performance audit.

The large majority of the work is a faithful lift-and-shift of existing logic into new service modules (pipeline-graph.ts, dashboard-data.ts) and the new tests validate the extracted functions well. One minor behavioral difference was found in the refactoring:

• detectConfigChanges null-version case — the new function returns true (has changes) when latestVersion is null, whereas the original pipeline.list handler left hasUndeployedChanges = false in that same path. This only matters for deployed pipelines with no version records (an abnormal state), but it is an unintended behavioral change for a baseline-quality PR.

Everything else — middleware correctness, encryption round-trips, audit patterns, team-scoping, and the performance scope fix on allComponentNodes — looks correct and well-covered by the new test suite.

Confidence Score: 4/5

• Safe to merge; one minor behavioral edge case in detectConfigChanges warrants a quick look before landing.
• The refactoring is faithful and well-tested (105 passing tests, tsc clean, eslint clean). The single finding is a subtle behavioral difference that only surfaces in an abnormal database state (deployed pipeline with no version records) and does not cause data loss or a security issue. All security-sensitive paths (withTeamAccess, withAudit, encryption, audit logging) are preserved correctly.
• src/server/services/pipeline-graph.ts — specifically the detectConfigChanges null-version return value and how listPipelinesForEnvironment calls it.

Important Files Changed

Filename	Overview
src/server/services/dashboard-data.ts	New service extracting chart metrics computation, node cards, and pipeline cards assembly from dashboard.ts. Faithful extraction with one minor behavioral difference in assemblePipelineCards (null latestVersion case). The PipelineMetricRow interface declares bytesIn/bytesOut but they are unused in the sparkline assembly.
src/server/services/pipeline-graph.ts	New service extracting saveGraphComponents, promotePipeline, discardPipelineChanges, detectConfigChanges, and listPipelinesForEnvironment from pipeline.ts. detectConfigChanges returns true when latestVersion is null, which differs from the original list endpoint's behavior (returned false). All other extracted logic is a faithful 1:1 extraction.
src/server/routers/pipeline.ts	Significantly trimmed (1318→847 lines) by delegating to new pipeline-graph service. All tRPC procedures retain correct middleware (withTeamAccess, withAudit). Import of Prisma namespace removed from this file since it moved to pipeline-graph.ts.
src/server/routers/dashboard.ts	Cleaned up by delegating to new dashboard-data service functions. Performance fix: allComponentNodes query is now scoped to user's pipeline IDs rather than a full-table scan. Delegation to assembleNodeCards, assemblePipelineCards, and computeChartMetrics is correct.
src/server/services/tests/crypto.test.ts	Solid test suite for AES-256-GCM encrypt/decrypt — round-trips, randomization, error handling, and missing-secret behavior are all covered. Import hoisting works correctly because the crypto module reads NEXTAUTH_SECRET at call time, not at module import time.
src/server/services/tests/pipeline-graph.test.ts	Thorough test coverage for the five exported functions in pipeline-graph.ts using Prisma deep-mocks. Transaction and error paths are exercised.
src/server/services/tests/dashboard-data.test.ts	Covers groupBy pipeline/node/aggregate, downsampling, and the node/pipeline card assembly. Fixture helpers keep tests readable.
src/lib/pipeline-status.ts	Clean extraction of aggregateProcessStatus and derivePipelineStatus into a shared module; priority ordering is correct and well-tested.
src/components/empty-state.tsx	New shared EmptyState component; straightforward and uses shadcn/ui Button primitive correctly.
src/components/query-error.tsx	New shared QueryError component; clean and consistent with EmptyState styling.
vitest.config.ts	Standard Vitest configuration with correct path alias resolution and proper exclusion of generated Prisma files.
next.config.ts	Bundle analyzer added correctly behind the ANALYZE=true env guard; does not affect normal builds.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Routers["tRPC Routers (trimmed)"]
        PR[pipeline.ts\n847 lines]
        DR[dashboard.ts\n652 lines]
    end

    subgraph Services["New Service Modules"]
        PG[pipeline-graph.ts\n• saveGraphComponents\n• promotePipeline\n• discardPipelineChanges\n• detectConfigChanges\n• listPipelinesForEnvironment]
        DD[dashboard-data.ts\n• computeChartMetrics\n• assembleNodeCards\n• assemblePipelineCards]
    end

    subgraph Libs["New Shared Libs"]
        PS[lib/pipeline-status.ts\n• aggregateProcessStatus\n• derivePipelineStatus]
        FMT[lib/format.ts\n• formatTime\n• formatTimeWithSeconds\n• formatTimestamp ✏️]
        ST[lib/status.ts\n• STATUS_COLORS\n• statusColor]
    end

    subgraph UI["New Shared UI"]
        ES[components/empty-state.tsx]
        QE[components/query-error.tsx]
    end

    subgraph Tests["New Test Suite (105 tests)"]
        T1[pipeline-graph.test.ts]
        T2[dashboard-data.test.ts]
        T3[alert-evaluator.test.ts]
        T4[deploy-agent.test.ts]
        T5[crypto.test.ts]
        T6[totp.test.ts]
        T7[pipeline-status.test.ts]
    end

    PR -->|delegates to| PG
    DR -->|delegates to| DD

    PG -->|uses| FMT
    DD -->|uses| FMT

    PG -.->|tested by| T1
    DD -.->|tested by| T2
    PS -.->|tested by| T7

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/server/services/pipeline-graph.ts
Line: 525-526

Comment:
**`detectConfigChanges` returns `true` for a null version — behavioral change from original**

The original `pipeline.list` logic in the router treated a missing `latestVersion` as _no changes_ (`hasUndeployedChanges` was left `false`). The extracted `detectConfigChanges` now returns `true` when `latestVersion` is `null`:

```typescript
if (!latestVersion) return true;
```

This means a deployed pipeline that somehow has no version records will now surface as "has undeployed changes" rather than "no changes". While this edge case shouldn't occur in normal operation (deploying always creates a version), it is a silent behavioral regression from the original code introduced during the refactoring.

If the intent is to keep parity with the pre-refactor behavior, the caller in `listPipelinesForEnvironment` should guard the call:

```typescript
hasUndeployedChanges = latestVersion
  ? detectConfigChanges({ ..., latestVersion: { ... } })
  : false;
```

Alternatively, add a comment to `detectConfigChanges` noting that the `null` → `true` behavior is intentional (the more conservative "no snapshot means state is unknown").

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "chore: remove .planning/ from git tracki..." | Re-trigger Greptile}

[greptile-apps]

detectConfigChanges returns true for a null version — behavioral change from original

The original pipeline.list logic in the router treated a missing latestVersion as no changes (hasUndeployedChanges was left false). The extracted detectConfigChanges now returns true when latestVersion is null:

if (!latestVersion) return true;

This means a deployed pipeline that somehow has no version records will now surface as "has undeployed changes" rather than "no changes". While this edge case shouldn't occur in normal operation (deploying always creates a version), it is a silent behavioral regression from the original code introduced during the refactoring.

If the intent is to keep parity with the pre-refactor behavior, the caller in listPipelinesForEnvironment should guard the call:

hasUndeployedChanges = latestVersion
  ? detectConfigChanges({ ..., latestVersion: { ... } })
  : false;

Alternatively, add a comment to detectConfigChanges noting that the null → true behavior is intentional (the more conservative "no snapshot means state is unknown").

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/server/services/pipeline-graph.ts
Line: 525-526

Comment:
**`detectConfigChanges` returns `true` for a null version — behavioral change from original**

The original `pipeline.list` logic in the router treated a missing `latestVersion` as _no changes_ (`hasUndeployedChanges` was left `false`). The extracted `detectConfigChanges` now returns `true` when `latestVersion` is `null`:

```typescript
if (!latestVersion) return true;
```

This means a deployed pipeline that somehow has no version records will now surface as "has undeployed changes" rather than "no changes". While this edge case shouldn't occur in normal operation (deploying always creates a version), it is a silent behavioral regression from the original code introduced during the refactoring.

If the intent is to keep parity with the pre-refactor behavior, the caller in `listPipelinesForEnvironment` should guard the call:

```typescript
hasUndeployedChanges = latestVersion
  ? detectConfigChanges({ ..., latestVersion: { ... } })
  : false;
```

Alternatively, add a comment to `detectConfigChanges` noting that the `null` → `true` behavior is intentional (the more conservative "no snapshot means state is unknown").

How can I resolve this? If you propose a fix, please make it concise.