266 lines
12 KiB
Markdown
266 lines
12 KiB
Markdown
# ICD Audit: API Contract vs ROADMAP / ARCHITECTURE
|
||
|
||
**Date:** 2026-03-04 | **Baseline:** v0.23.0
|
||
**Scope:** Cross-reference ICD-API.md against ROADMAP.md, ARCHITECTURE.md, and live codebase.
|
||
**Status:** §2 (naming) fully resolved — preset→persona rename complete across all JS, templates, CSS, Go handlers, and ICD. §3 (cleanup) resolved. §4.1 (chat_id alias) resolved. §4.2 (legacy list messages) open. ICD updated to target state including multi-participant channels.
|
||
|
||
---
|
||
|
||
## 1. ARCHITECTURE.md — Staleness Report
|
||
|
||
ARCHITECTURE.md is stamped **v0.20** — six minor versions behind. The following sections are materially stale:
|
||
|
||
### 1.1 Frontend File Structure (§ Frontend Architecture)
|
||
|
||
ARCHITECTURE lists 19 JS files. Codebase has **34**. Fifteen files are undocumented:
|
||
|
||
| Missing from ARCHITECTURE | Added In | Purpose |
|
||
|---------------------------|----------|---------|
|
||
| `admin-scaffold.js` | v0.22.7 | Admin section DOM scaffolding |
|
||
| `channel-models.js` | v0.20.0 | Multi-model channel roster UI |
|
||
| `chat-pane.js` | v0.22.7 | Reusable ChatPane component |
|
||
| `editor-mode.js` | v0.21.5 | Editor surface (listed but not in file tree) |
|
||
| `extensions.js` | v0.11.0 | Extension loader |
|
||
| `notification-prefs.js` | v0.20.0 | Notification preference UI |
|
||
| `notifications.js` | v0.20.0 | Bell dropdown + badge |
|
||
| `pages.js` | v0.22.5 | Page-level auth + routing |
|
||
| `pages-splash.js` | v0.22.7 | Login/register surface |
|
||
| `panels.js` | v0.18.1 | Side panel architecture |
|
||
| `persona-kb.js` | v0.17.0 | Persona-KB binding UI |
|
||
| `repl.js` | v0.21.3 | REPL surface |
|
||
| `surface-nav.js` | v0.22.8 | Surface navigation overrides |
|
||
| `tools-toggle.js` | v0.13.1 | Per-tool enable/disable UI |
|
||
| `ui-format.js` | v0.22.7 | Message formatting helpers |
|
||
| `ui-primitives.js` | v0.10.5 | Shared render primitives |
|
||
| `ui-primitives-additions.js` | v0.22.7 | Toast, badge, icon extensions |
|
||
|
||
ARCHITECTURE also lists `knowledge.js` — **renamed** to `knowledge-ui.js`.
|
||
|
||
### 1.2 SPA Shell References
|
||
|
||
ARCHITECTURE §Deployment Modes still says:
|
||
|
||
> "The frontend entrypoint (`docker-entrypoint-fe.sh`) handles `BASE_PATH` injection into `index.html`"
|
||
|
||
And the file tree lists:
|
||
|
||
> `├── index.html # Single-page app shell`
|
||
|
||
Both are wrong since v0.22.6. `index.html` is now a 16-line redirect stub. The real entry points are Go-rendered surface templates. The entrypoint now generates `proxy_pass` blocks to the backend for surface routes.
|
||
|
||
### 1.3 Deployment Diagram
|
||
|
||
The split-deployment description is functionally correct but the mechanism is wrong. Frontend no longer serves an SPA — it reverse-proxies page routes to the backend, which renders Go templates. The diagram should show the backend serving both API and HTML.
|
||
|
||
### 1.4 Backward Compatibility Section
|
||
|
||
The section says:
|
||
|
||
> "JSON field rename: `api_config_id` → `provider_config_id`... Frontend updated to match."
|
||
|
||
This is now ancient history (v0.9). The rename is complete everywhere. This section should either be removed or reduced to a single sentence noting historical context.
|
||
|
||
### 1.5 Missing Sections
|
||
|
||
ARCHITECTURE has **no documentation** for:
|
||
|
||
- Go template engine (`server/pages/`)
|
||
- Surface architecture (5 surfaces: chat, editor, notes, admin, settings)
|
||
- Template components (`model-select`, `team-select`, `file-upload`, `chat-pane`)
|
||
- Page data loaders (`loaders.go`)
|
||
- Provider Extensions / hook system (v0.22.1)
|
||
- Routing policies (v0.22.2)
|
||
- Export system (v0.22.4)
|
||
- Git integration (v0.21.4)
|
||
- The `treepath/` package (cursor, path, siblings)
|
||
|
||
**Recommendation:** Bump to v0.22 and rewrite §Frontend Architecture, §Deployment Modes, and §Backward Compatibility. Add §Template Engine, §Surface Architecture, §Provider Extensions sections.
|
||
|
||
---
|
||
|
||
## 2. ROADMAP.md — Housekeeping
|
||
|
||
### 2.1 Naming Inconsistency: Presets vs Personas
|
||
|
||
The ROADMAP uses "Persona" consistently. The API routes use `/personas`. The Go handler type is `PersonaHandler`. The JSON response sends **both** `"personas"` and `"presets"` keys (backward compat). The frontend reads the `presets` key exclusively.
|
||
|
||
This is a naming barnacle that bleeds into every layer:
|
||
|
||
| Layer | Name Used |
|
||
|-------|-----------|
|
||
| ROADMAP / ARCHITECTURE | Persona |
|
||
| Go types | `Persona`, `PersonaPatch` |
|
||
| Go handler | `PersonaHandler` |
|
||
| API routes | `/personas`, `/admin/personas` |
|
||
| JSON response keys | `"personas"` AND `"presets"` (dual) |
|
||
| Frontend JS | `presets` (reads `data.presets`) |
|
||
| UI labels | Mixed ("Preset" in some places, "Persona" in others) |
|
||
|
||
**Recommendation:** Pick one. The domain concept is "Persona" (it carries identity, system prompt, model, KB bindings — it's not just a preset). Two options:
|
||
|
||
- **Option A — Route rename**: `/personas` → `/personas`. Update FE. Remove dual JSON keys. This is a clean break and pre-1.0 is the time to do it.
|
||
- **Option B — Accept "presets" everywhere**: Rename Go types to `Preset`, update docs. Simpler but loses semantic precision.
|
||
|
||
### 2.2 `chat_id` Backward Compat Alias
|
||
|
||
`completionRequest` has:
|
||
|
||
```go
|
||
ChatID string `json:"chat_id"` // deprecated alias — maps to channel_id
|
||
```
|
||
|
||
The frontend doesn't send `chat_id` anymore. This is dead code in the handler that adds 3 lines of aliasing logic. Safe to remove.
|
||
|
||
### 2.3 Go Field Name Barnacle: `APIConfigID`
|
||
|
||
Throughout `channels.go` and `completion.go`, the Go struct field is named `APIConfigID` with json tag `"provider_config_id"`. The JSON tag is correct, so this is invisible to the API — but it's a source of confusion for anyone reading the Go code. Rename the Go field to `ProviderConfigID` for consistency.
|
||
|
||
### 2.4 Dual JSON Keys on Persona List
|
||
|
||
Three `PersonaHandler` methods return:
|
||
|
||
```go
|
||
gin.H{"personas": personas, "presets": personas}
|
||
```
|
||
|
||
The `"presets"` key exists solely for backward compat that was needed in v0.9. The FE only reads `"presets"`. If we go with Option A (§2.1), remove the `"presets"` key. If Option B, remove the `"personas"` key. Either way, stop sending both.
|
||
|
||
### 2.5 Route Alias: `/models` → `/models/enabled`
|
||
|
||
`main.go` has:
|
||
|
||
```go
|
||
protected.GET("/models/enabled", modelH.ListEnabledModels)
|
||
protected.GET("/models", modelH.ListEnabledModels) // alias
|
||
```
|
||
|
||
Plus the admin has its own `/admin/models`. Having `/models` as an alias for `/models/enabled` is confusing when `/admin/models` is a completely different thing. The ICD documents both, but one should go away. The FE uses `listEnabledModels()` → `/models/enabled`. The alias is unused.
|
||
|
||
### 2.6 ROADMAP v0.22.8 — Status vs Reality
|
||
|
||
The ROADMAP shows v0.22.8 as the next incomplete version with checkboxes for ChatPane Bridge, Theme Integration, Admin Surface, and Settings Surface. The last two sessions partially implemented some of these (admin scaffold, surface-nav, prototype overrides) but things broke and were rolled back/lost. The ROADMAP doesn't reflect the partial state — it still shows all items as unchecked.
|
||
|
||
**Recommendation:** After the ICD is finalized and the FE rebuild begins properly, update v0.22.8 checkboxes to reflect actual completion. Consider splitting v0.22.8 into smaller increments since it's become a catch-all.
|
||
|
||
---
|
||
|
||
## 3. ICD Gaps (things the backend implements but ICD doesn't fully cover)
|
||
|
||
### 3.1 Admin Models — Specific Endpoints
|
||
|
||
The ICD §24.5 lists admin model routes but doesn't document request/response shapes for:
|
||
|
||
- `POST /admin/models/fetch` — triggers provider sync, returns updated catalog
|
||
- `PUT /admin/models/bulk` — bulk visibility updates (request shape)
|
||
- `PUT /admin/models/:id` — single model update (what fields?)
|
||
|
||
**Fix:** Extract shapes from `admin.go` handler methods and add to ICD.
|
||
|
||
### 3.2 Admin Stats Response Shape
|
||
|
||
ICD §24.3 says "returns aggregate counts" but doesn't specify the shape. Should document:
|
||
|
||
```json
|
||
{
|
||
"users": 42,
|
||
"channels": 156,
|
||
"messages": 12847,
|
||
...
|
||
}
|
||
```
|
||
|
||
### 3.3 Admin Global Config — configWithKey Shape
|
||
|
||
ICD §24.4 says admin configs include `has_key` flag but doesn't show the full shape. The actual `configWithKey` response differs from the user-facing `safeConfig` — it includes `config`, `headers`, `settings` objects.
|
||
|
||
### 3.4 Team Provider Models
|
||
|
||
`GET /teams/:teamId/providers/:id/models` is in `main.go` but not clearly called out in the ICD §18.2 team admin section.
|
||
|
||
### 3.5 Team Audit Endpoints
|
||
|
||
`GET /teams/:teamId/audit` and `GET /teams/:teamId/audit/actions` are registered but not in the ICD team section.
|
||
|
||
### 3.6 Knowledge Base — Missing Detail
|
||
|
||
ICD §14 doesn't document:
|
||
|
||
- `GET /channels/:id/knowledge-bases` response shape (array of `ChannelKB`)
|
||
- `PUT /channels/:id/knowledge-bases` request shape
|
||
- `GET /knowledge-bases-discoverable` vs `GET /knowledge-bases` distinction
|
||
- KB document status progression: `pending → chunking → embedding → ready | error`
|
||
|
||
### 3.7 Export Endpoint Shape
|
||
|
||
ICD §21.2 has the right route but should note that `pandoc` must be available in the container. Also missing: supported format list beyond pdf/docx (the handler may accept more).
|
||
|
||
---
|
||
|
||
## 4. ICD Barnacles (things in the ICD that should be cleaned up)
|
||
|
||
### 4.1 `chat_id` Alias
|
||
|
||
The ICD §5.1 notes `chat_id` as a deprecated alias in the completion request. Once the dead code is removed from the handler, remove from ICD.
|
||
|
||
### 4.2 Legacy ListMessages
|
||
|
||
ICD §4.2 documents `GET /channels/:id/messages` as "Legacy/Debug." If it's truly not for frontend use, consider either removing the route or moving it to the admin namespace. Currently it's in the protected (user) namespace, which means any user can dump all messages (including branches) for their channels.
|
||
|
||
---
|
||
|
||
## 5. ROADMAP/ARCHITECTURE Reorganization Suggestions
|
||
|
||
### 5.1 ARCHITECTURE Should Reference the ICD
|
||
|
||
Instead of maintaining API route tables in multiple places, ARCHITECTURE should say "See ICD-API.md for the complete endpoint reference" and focus on:
|
||
|
||
- Design principles, scope model, resolution chains (what it does well today)
|
||
- Package structure (update to v0.22)
|
||
- Frontend architecture (rewrite for surfaces/templates)
|
||
- Security model (mostly current)
|
||
- New: Template engine, provider extensions, routing
|
||
|
||
### 5.2 ROADMAP Completed Versions — Collapse
|
||
|
||
The ROADMAP has 900+ lines of completed release details (v0.9 through v0.22.7). These are duplicated in CHANGELOG.md. The ROADMAP should collapse completed versions to one-liners (as it does for v0.9.x through v0.16.0) and push detailed history to CHANGELOG.
|
||
|
||
Currently v0.17.0 through v0.22.7 have full checkbox-level detail in the ROADMAP — that's useful while in-progress but noise once shipped. Collapse them.
|
||
|
||
### 5.3 Three-Document Model
|
||
|
||
Post-audit, the documentation should be:
|
||
|
||
| Document | Audience | Content |
|
||
|----------|----------|---------|
|
||
| `ARCHITECTURE.md` | Developers | Design principles, package structure, data model, resolution chains, security, deployment. **No API details.** |
|
||
| `ICD-API.md` | FE developers, integrators | Complete API contract: routes, shapes, auth, streaming, WebSocket. |
|
||
| `ROADMAP.md` | Stakeholders, contributors | Version plan, dependency graph, future features. Completed versions collapsed to one-liners with CHANGELOG refs. |
|
||
|
||
Future additions:
|
||
|
||
| Document | Audience | Content |
|
||
|----------|----------|---------|
|
||
| `ICD-SURFACE.md` | FE developers | Surface contract: templates, PageData, DOM IDs, JS globals, lifecycle. |
|
||
| `ICD-EXTENSION.md` | Extension authors | Extension contract: manifest, tiers, hooks, tool bridge, asset serving. |
|
||
|
||
### 5.4 ROADMAP Dependency Graph — Update
|
||
|
||
The ASCII dependency graph stops at v0.22.7. The v0.22.8 → v0.23.0 → v0.24.0 path should be added.
|
||
|
||
---
|
||
|
||
## 6. Cleanup Punch List
|
||
|
||
Priority-ordered actions that could all happen in a single commit:
|
||
|
||
| # | Action | Risk | Lines Changed |
|
||
|---|--------|------|---------------|
|
||
| 1 | Remove `chat_id` alias from `completionRequest` | None — FE doesn't use it | ~5 |
|
||
| 2 | Rename Go field `APIConfigID` → `ProviderConfigID` in channels.go + completion.go | None — json tags unchanged | ~30 |
|
||
| 3 | Pick persona vs preset naming, remove dual JSON keys | Low — FE reads one key | ~10 |
|
||
| 4 | Remove `/models` alias (keep `/models/enabled` only) | Low — check FE uses | ~1 |
|
||
| 5 | Bump ARCHITECTURE.md to v0.22, rewrite §Frontend | Medium — documentation | ~100 |
|
||
| 6 | Collapse ROADMAP completed versions (v0.17–v0.22.7) to one-liners | None — CHANGELOG has detail | ~600 removed |
|
||
| 7 | Add missing ICD shapes (§3 above) | None — documentation | ~80 added |
|
||
| 8 | Update ICD to remove chat_id and document any cleanup from #1–#4 | None | ~10 |
|