Repository navigation
feat(docs): render every page and ship a runnable image - #88
Merged
Merged
Conversation
Fidasek009
commented
Oct 7, 2026
Fidasek009
force-pushed
the
feat/docs-deployment
branch
from
October 8, 2026 09:03
7f1fff9 to
d86afa4
Compare
Fidasek009
marked this pull request as ready for review
October 8, 2026 09:25
Contributor
Code Review Roast 🔥Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. The incremental diff is substantial — widget CSS consolidation (inlined + scoped via 📊 Overall: Like a developer who actually reads code review comments AND knows how to use a bundler plugin — I'm not crying, you're crying. Files Reviewed (25 files)
Previous Review Summaries (2 snapshots, latest commit 29e6d67)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 29e6d67)Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. All 9 previous review findings are resolved in this incremental diff:
New code (widget markdown, embed form agent removal, DialogFooter simplification, theme toggle default flip, migration edits) is well-structured — no bugs, no security issues, no logic errors. The markdown component correctly strips 📊 Overall: Like a developer who actually reads code review comments — rare, appreciated, and slightly suspicious. I'm going to need a stronger drink. Files Reviewed (28 files)
Previous review (commit d86afa4)Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. 📊 Overall: Like finding a unicorn in production — I didn't think clean PRs existed anymore, but here we are. The splat route, shared problem codes, port validation, CSS adjustments, and the Files Reviewed (22 files, 4 generated/skip)
Reviewed by deepseek-v4-flash · Input: 135.9K · Output: 20.2K · Cached: 1M |
Fildass1
reviewed
Oct 10, 2026
Fildass1
left a comment
Fildass1
left a comment
Contributor
There was a problem hiding this comment.
Thorough pass on 03b0cbd8, verified empirically rather than by reading: tsc (docs/api/shared), bun test, oxlint, oxfmt, knip all pass; vite build succeeds and the SSR bundle has zero external imports; the runtime stage was reproduced without node_modules (dist + serve.ts only) and works; the image builds and the container reports healthy with all routes behaving as designed (/, all 5 content pages, /problems → 200; unknown paths and /docs/* → 404; hashed assets → 200; TALQO_DOCS_PORT="" fails fast). The contract-parity claim holds: PROBLEM_CODES key order is byte-identical to main, openapi.json and both generated clients are unchanged, and the three new catalog statuses match the API's actual responses (403 / 409 / 400).
Findings are inline: one shipped link that 404s (should fix before merge), malformed slugs rendering 500s with an internal message, hashed assets served without a cache policy, and the problems page no longer showing the type URI pattern, plus three nits.
Fildass1
approved these changes
Oct 11, 2026
Three of four content pages 404'd and nothing ran the built site. The sidebar is rendered from the page tree, so it advertised three dead links. docs.index.tsx imported one MDX file directly, which cannot cover a slug. Underneath, the compile-ahead collection glob-imported MDX eagerly and server-only, so client-side navigation had no way to resolve a page body. Both are replaced by a splat route over the Fumadocs macro collection, which compiles one lazy chunk per page. source.config.ts, .source/, and the @content alias are gone. docs.talqo.chat gets its own image: ADR-0016 scopes the single image to the self-hosted product, no operator needs the docs site, and the published problem type URIs pin it to that origin's root. The SSR bundle is built with noExternal so the runtime stage carries only dist and the entry point. src/serve.ts is deliberately not src/server.ts: TanStack Start resolves src/server.ts as its SSR build entry and overwrites dist/server/server.js with it. Prerendering stays off. It writes HTML nothing would serve without more server logic, and static versus server-rendered is a deployment call rather than a routing fix. Co-Authored-By: Space Bunny Free <noreply@t3.chat>
The only published URL on the docs origin was prefixed with /docs while /problems was not, so the origin exposed two conventions. Nothing else lives there, so the pages drop the prefix: /, /deployment, /embed-chat-integration, /ai-provider-configuration. A root splat now renders every page, which folds the /docs layout into it and removes a route file. /problems stays a static sibling and still outranks the splat. The integration test now asserts that no rendered link 404s, which is the invariant the prefixed layout got wrong. The container healthcheck probes / rather than /docs, which dropping the prefix turned into a 404 and a permanently failing image. Co-Authored-By: Space Bunny Free <noreply@t3.chat>
The problem catalog was missing agent-file-not-retryable and embed-disabled while the API emits both. The codes are now covered by a static parity check against apps/api/src/http/problem.ts, which fails in either direction without a server or a build. TALQO_DOCS_PORT="" and "0" silently bound a random port while the healthcheck probed 3002. Non-integer or out-of-range ports now fail fast with the received value. A client collection miss rendered a 500; it is a 404. The doc server fn now validates slug shape and size instead of passing input through. The serve integration test is gone; manual verification per the PR steps replaces it, and the parity check above keeps the published problem URIs honest. Co-Authored-By: Space Bunny Free <noreply@t3.chat>
…ecture The parity check read API source as text from a docs test, which is a dependency wearing a trenchcoat. PROBLEM_CODES now lives in @talqo/shared/problem-codes, consumed by both apps; generated contracts are byte-identical before and after. The architecture guide no longer documents the docs app beyond the one line it always had. The image-separation rationale lives in the Dockerfile, and NFR-1.3 is rewritten short to match what ships: one image for API, dashboard, and widget, its own image for docs. Co-Authored-By: Space Bunny Free <noreply@t3.chat>
Co-Authored-By: Space Bunny Free <noreply@t3.chat>
main already sets agentGuidance: false; this branch forked before that, so turbo kept re-adding its managed block here. Aligning stops it. Co-Authored-By: Space Bunny Free <noreply@t3.chat>
Resolving all SSR dependencies into the bundle breaks the dev module runner (`module is not defined` from CJS React, every page 500s). The runtime image still needs the self-contained bundle because it ships no node_modules, so the option now applies to `vite build` only. Co-Authored-By: Space Bunny Free <noreply@t3.chat>
Each problem entry now reads what happened, why, and how to fix it, with code, status, and type URI demoted to one muted line. Anchors and URIs are untouched. The page also links back to the documentation index; it stays chrome-free otherwise because API errors anchor straight into it. Sidebar 268px to 220px on desktop via the theme variable, verified by screenshot on desktop and mobile (drawer untouched below the breakpoint). Co-Authored-By: Space Bunny Free <noreply@t3.chat>
Fumadocs centers a fixed-width grid, so past 1080p the sidebar floats mid-screen inside growing gutters. Above 120rem the layout goes full-bleed instead: sidebar and TOC at the edges, article centered. 1080p, mobile, and the build are untouched. Verified by screenshot at 1536, 2560, and 390px widths. Co-Authored-By: Space Bunny Free <noreply@t3.chat>
Status and code move under the title, fix guidance uses a shorter label, and the per-entry type URI line is gone. Anchors and the catalog data are untouched, so published problem URIs still resolve. Co-Authored-By: Space Bunny Free <noreply@t3.chat>
Co-Authored-By: Space Bunny Free <noreply@t3.chat>
Key order feeds Object.values straight into the generated OpenAPI and client schemas. Alphabetizing the moved entries reordered two unions and broke contracts:check. Order is load-bearing here, not cosmetic. Co-Authored-By: Space Bunny Free <noreply@t3.chat>
The main merge added an mcp-connections page with a /docs-prefixed link in another file; it now points at /mcp-connections like the sidebar does. Over-limit slugs threw Error and rendered a 500 leaking the message. Validation rejections are missing addresses, so they 404 like any other unknown slug. Hashed assets now carry an immutable cache policy instead of bare ETag/Last-Modified. The static mount is gone; paths are resolved explicitly with the same traversal guard as the API's static responder. The problems intro states the type URI pattern once, since per-entry URIs are gone but the page is still their dereference target. Also: dropped the stale preview script, made the healthcheck failure path explicit, logged the bound hostname, and strict-parsed the port (digits only, then range). Co-Authored-By: Space Bunny Free <noreply@t3.chat>
Fidasek009
force-pushed
the
feat/docs-deployment
branch
from
October 11, 2026 09:37
29e6d67 to
e9d4960
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes
apps/docsdeployable: every page incontent/docsrenders, and the site ships as a runnable image.Three of four pages 404'd. The only route imported one MDX file directly, and the compile-ahead collection imported MDX eagerly and server-only, so client-side navigation had no way to resolve a page body. Both are replaced by a splat route over the Fumadocs macro collection, which compiles one lazy chunk per page. The sidebar advertises the page tree, so it was shipping three dead links.
The site also drops its
/docsprefix./problemswas the only published URL on that origin and it was never prefixed, so the origin carried two conventions; nothing else lives there, so pages are now served at the root (docs.talqo.chat/deployment). No redirects or aliases:/docsand/docs/*404.docs.talqo.chatgets its own image rather than joining the root one: ADR-0016 scopes that image to the self-hosted product, no operator needs the docs site, and the published problem type URIs (https://docs.talqo.chat/problems#{code}, compiled into generated API schemas) pin the site to that origin. The CD pipeline that publishes it is a separate PR.Prerendering is deliberately left off. It writes HTML that nothing serves without additional server logic, and static versus server-rendered is a deployment decision rather than a routing fix.
The
/problemspage now reads what happened, why, and how to fix it, with code, status, and type URI on one muted line; anchors and URIs are unchanged. It links back to the documentation index but otherwise stays chrome-free, since API errors anchor straight into it. Sidebar narrowed 268px to 220px on desktop (mobile drawer untouched), verified by screenshot. Above 120rem the layout goes full-bleed so the sidebar sticks to the viewport edge instead of floating mid-screen; smaller viewports are untouched. It writes HTML that nothing serves without additional server logic, and static versus server-rendered is a deployment decision rather than a routing fix.How to test
bun run dev:docs, then open each of/,/deployment,/ai-provider-configuration,/embed-chat-integration, and/problems. Sidebar links must resolve rather than 404.docker build -f apps/docs/Dockerfile -t talqo-docs .from the repository root, thendocker run --rm -p 3002:3002 talqo-docs.http://127.0.0.1:3002, confirm/, the four doc pages, and/problemsreturn 200, a hashed/assets/...file returns 200, an unknown path returns the 404 page, anddocker psreports the container healthy.TALQO_DOCS_PORT="" bun src/serve.ts(fromapps/docs) must fail fast naming the variable, not bind a random port.Notes for reviewers:
src/server.tsas its SSR build entry and overwritesdist/server/server.jswith it, which is why the process entry issrc/serve.ts./problemsis a static route sibling to the root splat and relies on static paths outranking the splat. The catalog carries all 45 codes;PROBLEM_CODESlives in@talqo/shared/problem-codesand both apps consume it, so API-side additions cannot drift from the docs page. Generated contracts are byte-identical.apps/docs/Dockerfile.Review round (Fildass1, verified empirically): fixed a
/docs/mcp-connectionslink that arrived via the main merge after the prefix purge; over-limit slugs now 404 instead of 500ing with the message; hashed assets carryCache-Control: immutablewith a traversal guard; the problems intro states the type URI pattern once; dropped the stalepreviewscript; healthcheck failure path is explicit; port parsing is digits-only and the log shows the bound hostname.Gap:
bun run actions:checkcould not run locally becauseactionlintis not installed here. No workflow files changed.Checklist
Prepared with opencode (space-bunny-free).