From 38148a97f961c4d8985341f08b90a99e02214640 Mon Sep 17 00:00:00 2001 From: saqib mir Date: Fri, 21 Aug 2026 17:35:32 +0530 Subject: [PATCH] docs: add architect's additions doc (gaps & recommendations) Production concerns not covered by the original spec (00-10): idempotency on ticket creation, bi-directional webhook callbacks, row-level-security tenant isolation, AI prompt-injection defense, optimistic concurrency on shared mutable state, RAG implementation specifics, AI cost/token governance, knowledge effectiveness feedback, CSAT capture, data retention/PII, API versioning/error contract, localization, and a lower-urgency list. Indexed in docs/00-INDEX.md as doc 11. Co-Authored-By: Claude Sonnet 5 --- docs/00-INDEX.md | 1 + ...tect-additions-gaps-and-recommendations.md | 88 +++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 docs/11-architect-additions-gaps-and-recommendations.md diff --git a/docs/00-INDEX.md b/docs/00-INDEX.md index 77c452c..01afa16 100644 --- a/docs/00-INDEX.md +++ b/docs/00-INDEX.md @@ -20,6 +20,7 @@ | 08 | [Frontend Architecture](./08-frontend-architecture.md) | Customer, agent, admin frontends; real-time UX | | 09 | [Testing, Observability & CI/CD](./09-testing-observability-cicd.md) | Test strategy, critical E2E flows, logging/metrics, Jenkins pipeline | | 10 | [Implementation Roadmap](./10-implementation-roadmap.md) | 11-phase delivery plan, success criteria, open business decisions | +| 11 | [Architect's Additions: Gaps & Recommendations](./11-architect-additions-gaps-and-recommendations.md) | Production concerns not in the original spec — idempotency, webhooks, RLS, prompt injection, AI cost governance, CSAT, data retention, and more | --- diff --git a/docs/11-architect-additions-gaps-and-recommendations.md b/docs/11-architect-additions-gaps-and-recommendations.md new file mode 100644 index 0000000..f8b3d56 --- /dev/null +++ b/docs/11-architect-additions-gaps-and-recommendations.md @@ -0,0 +1,88 @@ +# 11 — Architect's Additions: Gaps & Recommendations + +Everything in files 00–10 is a direct organization of the original specification. **Everything below is added by me** — things a production enterprise support platform needs that the original spec didn't call out, or only mentioned in passing. I've grouped them by how costly they are to bolt on later. + +--- + +## A. Critical — expensive to retrofit, cheap to design in now + +### A1. Idempotency on ticket creation +The spec defines the inbound product→SupportHub request but never addresses **retries**. If DocuQube's client times out waiting for a response and retries the same "PDF conversion failed" report, you'll get duplicate tickets for one problem unless the caller sends an idempotency key. + +- Add `idempotencyKey` (client-generated, e.g. hash of `productId + referenceIds + timestamp-bucket`) to the inbound contract in [02](./02-integration-and-security.md). +- Store it on `Ticket` with a unique constraint scoped to `productId`; a repeat request within a configurable window returns the existing ticket instead of creating a new one. + +### A2. Bi-directional integration (SupportHub → product callbacks) +The spec only defines product → SupportHub. But the product's own UI (e.g., DocuQube's "Help & Support" widget) needs to know the ticket's status without polling. Add: + +- A registered **webhook URL per `ProductIntegration`**, signed the same way inbound requests are (HMAC or mTLS), firing on key events: `ticket.status_changed`, `ticket.resolved`, `ticket.escalated`. +- Delivery must be async (via BullMQ), retried with backoff, and logged — a failed webhook delivery should never block or roll back the underlying ticket state change. + +### A3. Multi-tenant data isolation enforced at the data layer, not just the app layer +Section 52 says "a customer must never access another customer's tickets," but the spec only implies application-level scoping. For an enterprise platform, add: + +- **Postgres Row-Level Security (RLS)** policies on `Ticket`, `TicketMessage`, `TicketAttachment` keyed on `externalTenantId`, so a bug in one service-layer query can't leak cross-tenant data. Application-level scoping remains the primary control; RLS is the belt-and-suspenders layer. + +### A4. AI prompt-injection defense +The spec covers AI tool permissioning and hallucination control (section 58) but not **adversarial customer input**. A customer's problem description, or content inside an uploaded attachment (e.g., a PDF with embedded text), is untrusted input that reaches the LLM. Add explicit handling: + +- Treat retrieved knowledge, customer messages, and any attachment-derived text as **data, not instructions** — the system prompt must state this and the orchestration layer should never let content from these sources alter tool permissions or escalation policy. +- Tool-invocation requests coming out of the model are validated against the deterministic policy layer regardless of what the model claims justifies them (already implied by section 58/11, but worth stating as an explicit adversarial-input test case in [09](./09-testing-observability-cicd.md)). + +### A5. Optimistic concurrency on mutable shared state +`Ticket.status`, `SLARun.status`, and `AgentAvailability.currentLoad` are all written by multiple actors (customer actions, AI, agents, background jobs) concurrently. Add a `version` column (optimistic locking) to these three tables specifically, on top of the general concurrency guidance already in [06](./06-database-schema.md) — a plain "last write wins" update is not sufficient for SLA/assignment correctness under load. + +--- + +## B. Important — real gaps, moderate cost to retrofit + +### B1. RAG implementation specifics +The spec says "use a RAG architecture" and "vector database as appropriate" but leaves the actual retrieval design open. Decide and document: +- Embedding model + chunking strategy per knowledge entry type (a `KnowledgeEntry` with distinct `problem/symptoms/cause/solution` fields probably wants field-aware chunking, not one blob embedding). +- Retrieval filters must apply **before** the vector search (product scope, status=published, validationStatus) — never filter after, or you'll retrieve fewer results than the limit implies. +- Re-ranking step before knowledge reaches the LLM context, prioritizing `validationStatus: validated` and recency. + +### B2. AI cost and token governance +Nothing in the spec addresses LLM cost/latency control at scale. Add: +- Per-session token budget and a hard step-count cap on the reasoning loop (diagnosis → tool call → re-diagnosis) to prevent runaway sessions. +- Model routing/fallback (e.g., a smaller/faster model for classification, a stronger one for diagnosis) as a configurable policy, not a hardcoded model name. +- Track cost per ticket as a reportable metric alongside the AI dashboard metrics in [09](./09-testing-observability-cicd.md). + +### B3. Knowledge effectiveness feedback loop +The spec has a "knowledge effectiveness" metric (section 60) but no mechanism to actually compute it. Add: +- Link `AIKnowledgeReference` → ticket outcome (`AI_RESOLVED` vs `HUMAN_ESCALATION`) so each knowledge entry accumulates a resolution-contribution rate. +- Surface low-performing knowledge entries to admins for review — this closes the loop the spec's admin knowledge management (section 59) otherwise leaves open-ended. + +### B4. Customer satisfaction (CSAT) capture +Not in the original spec at all. Add a lightweight, optional post-resolution prompt ("Was this helpful?") captured against the ticket, reportable per product/agent/AI — this is standard for any support platform and materially informs whether "AI resolved" actually meant the customer was satisfied, not just that verification evidence existed. + +### B5. Data retention, deletion, and PII handling +Section 52 covers security but not data lifecycle. For an enterprise platform touching customer data across many tenants, define explicitly (as an `OPEN BUSINESS DECISION` per [10](./10-implementation-roadmap.md)): +- Retention period for tickets/messages/attachments/AI session transcripts. +- A deletion path when the SaaS reports a user/tenant deletion (SupportHub must purge or anonymize its `externalUserId`-linked records — it can't wait indefinitely holding data the SaaS no longer has consent for). +- Whether attachments or AI transcripts may contain PII that needs redaction before being used as RAG training/eval data. + +### B6. API versioning and error contract +The spec mentions OpenAPI but not a versioning scheme or a standard error shape. Add: +- URL or header-based versioning (`/v1/...`) from day one — retrofitting this after external product integrations exist is painful. +- A single error envelope (`{ error: { code, message, requestId, details? } }`) used by every endpoint, so integrating products write one error handler, not one per endpoint. + +### B7. Localization / multi-language customer input +Customers may report problems in a language other than the knowledge base's authoring language. Not addressed anywhere in the spec. At minimum, decide: does the AI reason and search knowledge in English regardless of input language and translate the response back, or is knowledge itself multi-language? This affects the RAG design in B1 and should be an explicit early decision, not discovered mid-build. + +--- + +## C. Worth deciding early, lower urgency + +- **Feature flags** for gradual AI capability rollout per product (e.g., enable tool execution for DocuQube before enabling it for a newer, less-tested product). +- **Full-text/ticket search** for agents (section 38's "All Tickets" view will need this quickly) — Postgres full-text search is likely sufficient before reaching for a separate search engine. +- **Bulk admin operations** (bulk reassign on agent offboarding, bulk close on stale tickets) — not mentioned, but every real deployment needs it within the first quarter. +- **Disaster recovery / backup cadence and RPO/RTO targets** — absent from the spec's otherwise thorough operations coverage. +- **Incident-management integration** (e.g., paging on `SLABreached` for critical severity) — the spec defines the breach event but not what happens operationally when one fires outside business hours. +- **Sandbox integration environment** — a product team integrating with SupportHub needs a way to test the full inbound/outbound contract without touching production tenants; worth a dedicated `environment: sandbox` flag on `ProductIntegration`. + +--- + +## What I did *not* add + +I deliberately didn't invent: specific SLA minute values, specific confidence thresholds, specific retention periods, or specific vector database/embedding model choices — those are exactly the kind of "final business/technical policy values" the original spec (and [10](./10-implementation-roadmap.md)) says must come from the business/team, not be guessed. Where I raised something above that implies a concrete choice, treat it as `REQUIRES BUSINESS CONFIRMATION` or `OPEN BUSINESS DECISION`, consistent with the rest of this guide.