wulf-pulse/docs/wulf-pulse-ticket-analyzer-build-notes.md

429 lines
21 KiB
Markdown
Raw Normal View History

# AI Ticket Analyzer — Build Notes
A running journal of the multi-phase build for the AI Ticket Analyzer feature.
Captures what was delivered each phase, design decisions worth flagging, and
what was deliberately left out. Spec lives in
`wulf-pulse-ticket-analyzer-prompt.md`.
This file is updated after each phase ships.
---
## Phase 1 — Migration + Zod schemas
**Delivered**
- `migrations/069_create_analyzer_tables.sql` — three tables
(`analyzer_analyses`, `analyzer_shares`, `analyzer_jobs`) with `pgcrypto`
extension guard, indexes per spec, status `CHECK` constraints, and FK types
corrected to `TEXT` (not `UUID`) to match Better Auth's `user.id`.
- `lib/types/analyzer.ts` — Zod schemas for every LLM-stage parsed JSON
(`TaggedEvent`, `TriageResponse`, `DeepAnalysisResponse`, `OpusResponse`),
persisted row shapes, job status, API request bodies, and the internal
`PreprocessedTicket` payload that flows through the pipeline.
**Decisions worth flagging**
- `triggered_by_user_id` is **nullable + `ON DELETE SET NULL`** (not `NOT NULL`).
An analysis should still be readable in history if the triggering user is
later deleted. `analyzer_shares.shared_by_user_id` is `NOT NULL +
ON DELETE CASCADE` (audit-log style — share rows go with the user).
- `evidence_timestamps` typed as `string().datetime({offset: true})` (ISO
timestamps), not numeric indices. More robust to model hallucination and
reads better in the UI.
- `TaggedEvent` does NOT support a "two separate events" form for time entries
with both Summary + Internal Notes — single event with both fields, per the
spec's preference for a cleaner timeline.
**Deliberately left out**
- Migration is committed but **not applied** to any running DB. Postgres only
re-applies migrations on first init of a fresh volume; the existing DB needs
a manual run of this migration when phase 5 is exercised.
---
## Phase 2 — Stage 0 pre-processor + IT Glue redaction
**Delivered**
- `lib/services/analyzer/itglue-redact.ts` — recursive walk; matches keys
against `/password|secret|key|token|credential|api[_-]?key/i`; replaces
values with `[REDACTED]`. Subtree redaction (key matching `auth` → no leaves
leak), defensive copy, cycle guard (`[CIRCULAR]` marker).
- `lib/services/analyzer/preprocessor.ts` — filters workflow noise + Service
Desk Notification rows, tags ticket_create/notes/time entries with
`actor / actor_type / source / visibility / summary_notes / internal_notes /
hours`, sorts chronologically, computes `sha256` content hash over canonical
JSON of `(events, status, priority, queue)`.
- `vitest@^4.1.5` added as devDep with `vitest.config.ts` setting up the
`@/` alias. Two test files at this phase: 32 redaction tests + 36
preprocessor tests including the regression run against the
`T20260424.0045` fixture.
- Type-aliasing fix in `lib/types/analyzer.ts`: added `export type X =
z.infer<typeof X>` for `ActorType`, `EventSource`, `Visibility`, `Severity`,
`ComplexityTier`, `TicketType` — the Zod-enum const exports alone don't
produce a usable TypeScript type.
**Decisions worth flagging**
- `actor_type` is classified by **email domain, not author text**, per the
spec. `lorentz@wulfconsulting.com` is `wulf_tech` regardless of how the
message reads. Sonnet handles the role nuance at Stage 3.
- Time entries with no narrative content (no summary, no internal notes) are
dropped — a purely numeric entry adds nothing.
- Vendor-domain allowlist is intentionally short and conservative (Vertafore,
Datto, Microsoft, etc.). Misclassifying a customer domain as "vendor" is
worse than the default `client_contact`.
**Deliberately left out**
- No real protection for secrets embedded in **free text** (e.g. a notes field
containing the literal string "the password is hunter2"). The redaction
guarantee is on field **keys**, not values. The IT Glue search test
documents this contract explicitly so it isn't "fixed" without thought.
---
## Phase 3 — Anthropic SDK setup + Stage 1 (Haiku triage)
**Delivered**
- `lib/services/llm/{models,pricing,client,call}.ts` — model ID constants,
per-model rate table (Haiku $1/$5, Sonnet $3/$15, Opus $5/$25 per 1M tokens
+ cache read/write tiers), lazy SDK singleton, generic
`callLLMStage<T>({model, system, user, schema, maxTokens, client?})`
helper that:
- Marks the system prompt with `cache_control: {type: "ephemeral"}`
- Sends NO `temperature`/`top_p`/`top_k` (Opus 4.7 would 400)
- Strips a single ` ```json ` fence before parsing
- On parse failure, retries **once** with prior-attempt + error in a
follow-up user turn
- Returns `{data, usage, estimated_cost_usd, attempts, raw_response}`
- `lib/services/analyzer/stages/stage1-triage.ts` — Haiku caller with the
spec's verbatim system prompt and a 50KB user-payload cap that drops oldest
internal-only events first when oversized.
- `lib/services/analyzer/itglue-search.ts` — search facade that runs every
result through `redact()` before returning. Snippets capped at 2000 chars,
doc count capped at 10. `itglue-aliases.json` skeleton for known fuzzy
org-name mappings.
- 32 new tests across pricing/call/Stage 1/IT Glue search.
**Decisions worth flagging**
- **Manual JSON.parse + Zod validate, not `output_config.format` /
`client.messages.parse()`.** The spec said retry-once on Zod parse failure,
and Zod 4 ↔ JSON Schema conversion has edge cases I didn't want to depend
on (e.g. `.datetime({offset: true})` → JSON Schema `format`). Manual parse
is what the spec asks for and is more transparent.
- **Prompt caching probably won't fire on these stages.** Our system prompts
are ~1-2 KB (~250-500 tokens); minimum cacheable prefix is 2048 (Sonnet) or
4096 (Haiku/Opus) tokens. The `cache_control` marker is a no-op below
threshold and incurs no cost — left in defensively, but caching is not a
meaningful lever for this workload.
- **Redaction is on KEY names, not free text** — re-stated for emphasis. The
IT Glue search test asserts this contract.
- **`itglue-aliases.json` is a skeleton** with `_comment` / `_example` keys
documenting the format. Real org-id entries get added when phase 5 wires
this into the pipeline.
**Deliberately left out**
- No live API integration test. All Stage 1 tests use a mocked `Anthropic`
client. A real-API smoke test belongs in phase 5+ when we run end-to-end.
- Did **not** wrap `lib/services/itglue-client.ts` in a redacting search
facade for non-LLM callers. The redaction primitive is ready; non-LLM
callers (sync service, data browsers) intentionally have full data — they
are not the path that needs protection.
---
## Phase 4 — Pipeline + Job worker (Stages 3, 4, 5)
**Delivered**
- `lib/services/analyzer/stages/stage3-deep-analysis.ts` — Sonnet caller,
spec's verbatim prompt, 80KB payload cap.
- `lib/services/analyzer/stages/stage4-deep-reasoning.ts` — Opus caller plus
pure helpers `shouldRunDeepReasoning()` and `applyOpusUpdates()`.
- `lib/services/analyzer/data-access.ts``loadTicketBundle(ticketNumber)`
joins `tickets` / `statuses` / `priorities` / `queues` / `companies` /
`contacts` / `resources` / `ticket_notes` / `time_entries` and returns the
exact `RawTicketBundle` shape the preprocessor expects. Throws typed
`TicketNotFoundError`.
- `lib/services/analyzer/persistence.ts``getNextAnalysisVersion`,
`findExistingAnalysisByContentHash`, `insertAnalysis`, plus job
`claimQueuedJob` / `updateJobStatus` / `completeJob` / `failJob` /
`queueJob` / `getJob`. Job claim uses `FOR UPDATE SKIP LOCKED` so multiple
Next.js workers can poll safely.
- `lib/services/analyzer/pipeline.ts` — composes Stage 0 → idempotency check
→ Stage 1 → (Stage 2 if `itglue_lookup_needed`) → Stage 3 → (Stage 4 if
trigger fires AND cost ceiling not reached) → result. Cost circuit breaker
trips at $2.00 before Opus, sets `needs_human_review=true` with a
reason. Returns full `model_traces` for debugging.
- `lib/services/analyzer/worker.ts` — singleton with 2-second poll loop;
auto-starts in production; opt-in in dev via
`ANALYZER_WORKER_AUTOSTART=1`; **skipped** under vitest. `runJob()`
exposed for tests + manual triggers. `TicketNotFoundError` produces a
user-facing job error message that fingers the sync as the culprit.
- 28 new tests bringing the total to 128.
**Decisions worth flagging**
- **Worker auto-start is more conservative than `sync-scheduler.ts`.** That
one auto-starts on any non-browser import (including tests). I gated this
one because the worker hits the database AND runs LLM calls — much higher
blast radius. To run locally, set `ANALYZER_WORKER_AUTOSTART=1`.
- **Cost circuit breaker only fires before Opus.** Sonnet runs unconditionally
even if it would push past $2. Spec wording matches; if the team wants
stricter control, the natural place is next to `COST_CEILING_USD` in
`pipeline.ts`.
- **Idempotency check matches `status='complete'` only.** Failed runs don't
poison the cache.
- **IT Glue failures are tolerated.** Search throws → analysis continues
without context, doesn't fail the run.
- `worker.test.ts` casts `mockImplementationOnce` to `as never` because
vitest's overload resolution fights us when the mocked function has multiple
call signatures. Functional, just ugly.
**Deliberately left out**
- No live API integration test (still). Phase 5+ is the natural place for an
end-to-end smoke test.
- Stage 3 doesn't have a fixture-driven happy-path test like Stage 1; it's
exercised at the orchestration layer via `pipeline.test.ts` only. Worth
adding direct Stage 3 tests later.
- Migration `069` still not applied — same as phase 1.
---
## Phase 5 — API routes
**Delivered**
- Persistence read paths added to `lib/services/analyzer/persistence.ts`:
`getAnalysisById`, `listAnalysesByTicketNumber`, `listNeedsReview`,
`createShare`, plus a shared `rowToPersistedAnalysis` row mapper.
- 6 routes under `app/api/analyzer/`:
| Route | Method | Returns |
|---|---|---|
| `tickets/[ticketNumber]/analyze` | POST | `{status, jobId?, existingAnalysisId?}` |
| `tickets/[ticketNumber]/analyses` | GET | `{analyses: PersistedAnalysis[]}` |
| `jobs/[jobId]` | GET | `{job}` |
| `analyses/[id]` | GET | `{analysis}` |
| `analyses/[id]/share` | POST | `{share}` |
| `needs-review?limit=&offset=` | GET | `{analyses}` |
**Decisions worth flagging**
- **Analyze runs preprocess inline.** The route does
`loadTicketBundle → preprocessTicket → findExistingAnalysisByContentHash`
synchronously to support the spec's `existingAnalysisId?` immediate
response. The worker also runs this — duplicated work, but preprocess is
fast (deterministic, one DB load) and the alternative (always queue,
frontend polls to discover the short-circuit) is worse UX.
- **Email send deferred to phase 8** per the spec's delivery order. The share
route persists the audit row and validates the recipient domain against
`ALLOWED_SHARE_DOMAINS`. Until phase 8, share rows have `viewed_at = null`
indefinitely.
- **`requireAuth()` everywhere — not `requireAdmin()`.** Any authenticated
user can analyze a ticket they have access to. If `/needs-review` should
be admin-only later, swap that one to `requirePermission('analyzer',
'review')` once the permission map is decided.
- **No new permission entries added to `lib/permissions.ts`.** Adding scoped
permissions for a feature still under build risks getting them wrong.
- Routes are NOT in `middleware.ts` `publicRoutes` — they require a session.
**Deliberately left out**
- No API route tests. The repo has zero `app/api/**/*.test.ts` files; the
routes are thin orchestration on top of already-tested persistence. Adding
integration tests means setting up a test harness for the auth helpers +
Postgres, which is a separate effort.
---
## Phase 6 — Frontend pages
**Delivered**
- `components/analyzer/analyze-button.tsx``<AnalyzeButton>` with the full
state machine: POSTs to the analyze endpoint, navigates straight to an
existing analysis if `existingAnalysisId` came back, otherwise polls
`/api/analyzer/jobs/:jobId` every 2s and renders stage labels (Queued →
Fetching → Triaging → Searching IT Glue → Analyzing → Deep review → Done).
5-minute hard timeout. Failures surface as toast errors. Supports `force`
for explicit re-run.
- `components/analyzer/share-modal.tsx``<ShareModal>` with a shadcn Dialog
+ email field + optional note (max 2000 chars). POSTs to the share
endpoint and surfaces server-side validation errors (domain not allowed →
toast).
- `components/analyzer/analysis-view.tsx` — the full 10-section analysis
layout per spec. Header with model-tier badges (Haiku/Sonnet/Opus pills),
confidence score, total cost, Share + Re-analyze buttons. Summary, Next
Step (with rationale collapsed), Timeline (vertical list with 🟢 / 🔒 /
🔄 markers, click to expand), What Was Done / Should Have Been Done
side-by-side on wide screens, Gaps colored by severity with "Evidence:"
links that scroll-and-expand the matching timeline event,
Post-Resolution (only if present), Human Review Flags (only if needed),
IT Glue References.
- `app/analyzer/ticket/[ticketNumber]/page.tsx` — ticket detail with the
Analyze button and a list of historical versions; latest is badged.
- `app/analyzer/analysis/[id]/page.tsx` — fetches one analysis and renders
it via `<AnalysisView>`.
- `app/analyzer/queue/page.tsx` — needs-review queue, shows ticket number,
version, summary preview, top reasons, confidence badge, and a `high gap`
badge if any gap is high severity.
**Decisions worth flagging**
- **Imperative `useState` + `useEffect` + `fetch`**, no SWR / react-query —
matches CLAUDE.md and the rest of the repo. Don't refactor to a global
cache layer for these three pages; if it becomes a real pain, that's a
whole-app concern.
- **`'use client'` everywhere.** The pages use `use(params)` (the React
hook) to unwrap Next.js 16's `params: Promise<...>` shape on the client.
Server components weren't appropriate here — every page does interactive
state (analyze flow, expand events, share modal).
- **Stage labels render directly from `JobStatus` enum values**, not a
separate label list, so any new statuses added to the enum auto-render
with their default name.
- **Re-analyze banner not implemented yet.** The spec calls for "New
activity since last analysis · Re-analyze" when the live content_hash
drifts from the persisted one. That requires a live-preprocess endpoint
(or running preprocess on the page render). Skipped for now — the user
can always click Re-analyze. Worth adding once we have real-world signal
on whether activity drift is common.
- **No autocomplete on the share-recipient field.** Spec says "autocomplete
from existing wulf-pulse user list if available". Skipped — the existing
user list is in Better Auth's `user` table; exposing it requires a
small API endpoint. Easy follow-up.
- **No navigation entry yet.** `components/navigation/app-navigation.tsx`
doesn't yet have an "Analyzer" link. Adding that is a one-line edit; I
left it for the operator to opt in once the feature is staged.
- **Printable analysis view** — the spec mentions "printable" for the
analysis page. The current layout is print-friendly by accident (no
fixed sidebars, sectioned cards), but no explicit `@media print` styles
yet. Add when someone asks.
**Deliberately left out**
- No frontend tests. vitest is configured for `lib/**/*.test.ts` only; the
pages and components are visually verified. Component tests with
testing-library would be a separate setup decision.
- Analysis view doesn't auto-refresh while a job is running on a
*different* version. If a user navigates to an old version while a new
one is in progress, they don't see the in-progress state. Acceptable —
the queue view + ticket history give that signal.
---
## Phase 7 — Share-via-email integration
**Delivered**
- `sendAnalysisShareEmail()` added to `lib/services/email.ts` — re-uses the
existing nodemailer SMTP transport that already serves magic-link and
invitation mail. Subject `Pulse analysis · <ticket> v<n><summary
excerpt>`, gradient-header HTML body matching the other Pulse emails, plain
text fallback, `replyTo` set to the sharer so a recipient reply lands with
the right person.
- Share route `app/api/analyzer/analyses/[id]/share/route.ts` now resolves
the sharer's session, builds the analysis URL from `BETTER_AUTH_URL`
(falling back to `NEXT_PUBLIC_BETTER_AUTH_URL`, then `localhost:3100`),
persists the audit row, then attempts the email send.
- Share-modal frontend handles the new `emailSent`/`emailError` fields:
success → green toast; row-saved-but-send-failed → orange `toast.warning`
carrying the SMTP error.
**Decisions worth flagging**
- **Audit row persists even if email send fails.** The `analyzer_shares`
row is the audit log, not just a delivery receipt. SMTP outages should
not erase the record that the user attempted a share.
- **Response is HTTP 200 on email failure.** The route returns
`{share, emailSent: false, emailError}`. A non-2xx would imply the share
itself failed; surfacing `emailSent: false` is the more honest signal.
- **No `email_sent_at` column added.** Adding it requires a migration and
a way to retry — neither is asked for by the spec. Send state lives only
in the response and the server log line `[ANALYZER-SHARE] email send
failed for share <id>`. If retries become a real need, a dedicated
`analyzer_share_email_attempts` table is the natural shape.
- **Used existing nodemailer SMTP, not Graph sendMail.** Spec said "via
M365 Graph using existing wulf-pulse mail integration if one exists;
otherwise use a new module" — `email.ts` *is* the existing module. The
Graph integration in this codebase is read-only (mailbox search, user
reports), not send-capable.
- **HTML escaping is hand-rolled.** Five-replace function for
amp/lt/gt/quote/apos. The user-controlled fields (sender name, note,
analysis summary, next step) all flow through it. The repo has no HTML
escaper utility and the magic-link/invitation emails don't need one
because their inputs are URLs and admin-set names.
**Deliberately left out**
- **No retries.** A failed send is logged once and surfaced to the user.
They can re-share if they want — that creates a fresh audit row, which
is correct behavior.
- **No tests.** `lib/services/email.ts` has no existing tests, mocking
nodemailer fully would add a non-trivial test scaffold for one
function, and the share route is route-handler thin. Consistent with
phases 5/6.
- **No `viewed_at` tracking yet.** The migration has the column but
nothing writes to it. A `/share/:id/viewed` endpoint with an
unguessable token would be the smallest addition; spec didn't ask, so
skipped.
---
## Phase 8 — Operator runbook
**Delivered**
- `docs/wulf-pulse-ticket-analyzer-runbook.md` — cost monitoring queries
(daily spend, top expensive analyses, multi-run tickets), the cost
ceiling explanation (`COST_CEILING_USD = 2.00` in `pipeline.ts`), the
IT Glue alias workflow (where to find org IDs, how to verify via
`model_traces.itglue.alias_used`), failure triage tables (failed jobs
vs. `needs_human_review` flagged completes, with the actual error
strings the worker emits), and a manual-ops section for queueing jobs
from psql.
- README.md "Documentation" section links the runbook so operators
finding the project for the first time discover it.
**Decisions worth flagging**
- **Runbook lives in `docs/`, not the README.** Per CLAUDE.md, long-form
per-feature docs go in `docs/` and the README points to them. Matches
the existing pattern (workflow-editor-guide, sync guides, etc.).
- **Manual migration step is documented.** Postgres only re-applies
migrations on first init — operators applying analyzer to an existing
DB need to run `psql -f migrations/069_*.sql`. The runbook leads with
this.
- **Listed what's *not* implemented.** Retries, `viewed_at`, share
`email_sent_at`, pre-Sonnet cost gate, share-recipient autocomplete.
Better to advertise the gaps than have an operator trip over them.
**Deliberately left out**
- No production-monitoring dashboard (Grafana etc.) — Pulse doesn't have
one for any other feature, and the SQL queries cover the same ground.
- No README expansion for the analyzer feature itself. Pulse's README is
intentionally short; the runbook + spec + build notes are the deep
docs.
---
## Status after each phase
| Phase | Tests | tsc | Notes |
|---|---|---|---|
| 1 | 0 | clean | migration + types only |
| 2 | 68 | clean | redaction + preprocessor regression |
| 3 | 100 | clean | + LLM scaffolding + Stage 1 |
| 4 | 128 | clean | + pipeline + worker |
| 5 | 128 | clean | API routes (no route tests) |
| 6 | 128 | clean | frontend (no FE tests) |
| 7 | 128 | clean | share email via existing SMTP transport |
| 8 | 128 | clean | operator runbook + README link |