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 <noreply@anthropic.com>
This commit is contained in:
saqib mir
co-authored by
Claude Sonnet 5
Claude Sonnet 5
parent
2dffe58496
commit
38148a97f9
@@ -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 |
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -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.
|
||||
Reference in New Issue
Block a user