mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site): improve markdown rendering and top bar styling in agent chat (#22939)
## Changes ### Markdown rendering (response.tsx) - **Headings (h1-h6)**: Apple-like font scale from 13px base, with proper weight and spacing. - **Task-list checkboxes**: Replace disabled `<input>` with styled `<span>` (checked = filled blue + checkmark SVG, unchecked = bordered empty box). - **Table cells (th/td)**: Inherit 13px base font instead of streamdown's hardcoded `text-sm` (14px). - **Horizontal rules**: Explicit border styling to fix browser default inset/ridge when Tailwind preflight is off. - **List items**: Detect `task-list-item` class and remove default list marker. ### Top bar (TopBar.tsx) - Increased vertical padding (`py-0.5` -> `py-1.5`). - Parent chat button text size: `text-xs` -> `text-sm` to match active chat title. - ChevronRight icon: added `-ml-0.5` for even spacing around separator. - Removed redundant "Archived" badge (archived banner already shows below the top bar). ### Stories - Rewrote `WithMessageHistory` story with rich markdown covering headings, task lists, tables, code blocks, and horizontal rules.
This commit is contained in:
Kyle Carberry
@@ -35,6 +35,10 @@ type MarkdownComponentProps = {
|
||||
href?: string;
|
||||
children?: ReactNode;
|
||||
node?: HastNode;
|
||||
type?: string;
|
||||
checked?: boolean;
|
||||
disabled?: boolean;
|
||||
className?: string;
|
||||
};
|
||||
|
||||
type FileViewerThemeType = "light" | "dark";
|
||||
@@ -79,10 +83,99 @@ const createComponents = (
|
||||
{children}
|
||||
</a>
|
||||
),
|
||||
// Headings scaled for a 13px base using a tight,
|
||||
// Apple-like progression.
|
||||
h1: ({ children }: MarkdownComponentProps) => (
|
||||
<h1 className="mb-3 mt-5 text-xl font-semibold leading-snug first:mt-0">
|
||||
{children}
|
||||
</h1>
|
||||
),
|
||||
h2: ({ children }: MarkdownComponentProps) => (
|
||||
<h2 className="mb-2 mt-4 text-base font-semibold leading-snug first:mt-0">
|
||||
{children}
|
||||
</h2>
|
||||
),
|
||||
h3: ({ children }: MarkdownComponentProps) => (
|
||||
<h3 className="mb-1.5 mt-3 text-[15px] font-semibold leading-snug first:mt-0">
|
||||
{children}
|
||||
</h3>
|
||||
),
|
||||
h4: ({ children }: MarkdownComponentProps) => (
|
||||
<h4 className="mb-1 mt-3 text-sm font-semibold leading-snug first:mt-0">
|
||||
{children}
|
||||
</h4>
|
||||
),
|
||||
h5: ({ children }: MarkdownComponentProps) => (
|
||||
<h5 className="mb-1 mt-2 text-[13px] font-semibold leading-snug first:mt-0">
|
||||
{children}
|
||||
</h5>
|
||||
),
|
||||
h6: ({ children }: MarkdownComponentProps) => (
|
||||
<h6 className="mb-1 mt-2 text-xs font-semibold leading-snug text-content-secondary first:mt-0">
|
||||
{children}
|
||||
</h6>
|
||||
),
|
||||
// GFM task-list checkboxes: render a styled replacement
|
||||
// for the native <input type="checkbox" disabled> element.
|
||||
input: ({ type, checked, disabled }: MarkdownComponentProps) => {
|
||||
if (type !== "checkbox") {
|
||||
return <input type={type} disabled={disabled} />;
|
||||
}
|
||||
return (
|
||||
<span
|
||||
aria-hidden="true"
|
||||
className={cn(
|
||||
"mr-2 inline-flex size-4 shrink-0 items-center justify-center",
|
||||
"rounded-sm border border-solid",
|
||||
"align-middle relative -top-px",
|
||||
checked
|
||||
? "border-content-link bg-content-link text-white"
|
||||
: "border-border-default bg-surface-primary",
|
||||
)}
|
||||
>
|
||||
{checked && (
|
||||
<svg
|
||||
className="size-3"
|
||||
fill="none"
|
||||
viewBox="0 0 24 24"
|
||||
stroke="currentColor"
|
||||
strokeWidth={3}
|
||||
>
|
||||
<path
|
||||
strokeLinecap="round"
|
||||
strokeLinejoin="round"
|
||||
d="M5 13l4 4L19 7"
|
||||
/>
|
||||
</svg>
|
||||
)}
|
||||
</span>
|
||||
);
|
||||
},
|
||||
// Task-list items: remove the default bullet marker.
|
||||
li: ({ className, children }: MarkdownComponentProps) => {
|
||||
const isTask =
|
||||
typeof className === "string" && className.includes("task-list-item");
|
||||
return <li className={isTask ? "list-none" : undefined}>{children}</li>;
|
||||
},
|
||||
// Horizontal rule: reset browser default inset/ridge border
|
||||
// (preflight is disabled) to a clean 1px solid line.
|
||||
hr: () => (
|
||||
<hr className="my-6 border-0 border-t border-solid border-border-default" />
|
||||
),
|
||||
// Table cells: streamdown defaults to text-sm (14px).
|
||||
// Drop the explicit size so cells inherit the 13px base.
|
||||
th: ({ children }: MarkdownComponentProps) => (
|
||||
<th className="whitespace-nowrap px-4 py-2 text-left font-semibold">
|
||||
{children}
|
||||
</th>
|
||||
),
|
||||
td: ({ children }: MarkdownComponentProps) => (
|
||||
<td className="px-4 py-2">{children}</td>
|
||||
),
|
||||
// Inline code only — fenced blocks are handled by the pre override.
|
||||
code: ({ children }: MarkdownComponentProps) => (
|
||||
<code className="rounded bg-surface-quaternary/25 px-1 py-0.5 font-mono text-content-primary">
|
||||
{children}
|
||||
{children}{" "}
|
||||
</code>
|
||||
),
|
||||
// Fenced code blocks: extract language and content from the HAST
|
||||
|
||||
@@ -209,7 +209,9 @@ type Story = StoryObj<typeof AgentDetailLayout>;
|
||||
// Stories
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** Multi-turn conversation with message history and the chat input visible. */
|
||||
/** Multi-turn conversation with rich markdown rendering: headings, tables,
|
||||
* ordered/unordered lists, nested lists, code blocks, blockquotes,
|
||||
* horizontal rules, inline formatting, links, images, and task lists. */
|
||||
export const WithMessageHistory: Story = {
|
||||
parameters: {
|
||||
queries: buildQueries(
|
||||
@@ -217,10 +219,11 @@ export const WithMessageHistory: Story = {
|
||||
chat: {
|
||||
id: CHAT_ID,
|
||||
...baseChatFields,
|
||||
title: "Help me refactor this module",
|
||||
title: "Markdown rendering showcase",
|
||||
status: "completed",
|
||||
},
|
||||
messages: [
|
||||
// -- Turn 1: user asks for a summary --
|
||||
{
|
||||
id: 1,
|
||||
chat_id: CHAT_ID,
|
||||
@@ -229,10 +232,11 @@ export const WithMessageHistory: Story = {
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "Can you help me refactor the authentication module? It's gotten pretty messy.",
|
||||
text: "Give me a comprehensive overview of the auth module refactor. Include tables, lists, code examples, and anything else that would help me understand the plan.",
|
||||
},
|
||||
],
|
||||
},
|
||||
// -- Turn 2: assistant with headings, lists, table, blockquote --
|
||||
{
|
||||
id: 2,
|
||||
chat_id: CHAT_ID,
|
||||
@@ -241,10 +245,74 @@ export const WithMessageHistory: Story = {
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "Sure! I'll start by looking at the current structure. The main issues I can see are:\n\n1. **Mixed concerns** — token validation and session management are interleaved\n2. **No error hierarchy** — all auth errors are treated the same\n3. **Duplicated middleware** — the same checks appear in three places\n\nLet me propose a cleaner separation.",
|
||||
text: [
|
||||
"# Auth Module Refactor Plan",
|
||||
"",
|
||||
"## Current Problems",
|
||||
"",
|
||||
"The existing authentication module has several issues that need addressing:",
|
||||
"",
|
||||
"1. **Mixed concerns** - token validation and session management are interleaved",
|
||||
"2. **No error hierarchy** - all auth errors are treated the same",
|
||||
"3. **Duplicated middleware** - the same checks appear in three places",
|
||||
"4. **Missing observability** - no structured logging or tracing",
|
||||
"5. **Poor test coverage** - only 34% of branches tested",
|
||||
"",
|
||||
"### Impact Matrix",
|
||||
"",
|
||||
"| Component | Severity | Effort | Priority |",
|
||||
"|---|---|---|---|",
|
||||
"| Token Validation | High | Low | P0 |",
|
||||
"| Session Management | High | Medium | P0 |",
|
||||
"| Middleware Dedup | Medium | Low | P1 |",
|
||||
"| Error Hierarchy | Medium | Low | P1 |",
|
||||
"| Observability | Low | Medium | P2 |",
|
||||
"| Test Coverage | Low | High | P2 |",
|
||||
"",
|
||||
"> **Note:** The P0 items are blocking the v2 API release. We should tackle those first before moving to P1 and P2.",
|
||||
"",
|
||||
"---",
|
||||
"",
|
||||
"## Proposed Architecture",
|
||||
"",
|
||||
"The new architecture splits auth into three layers:",
|
||||
"",
|
||||
"- **Transport layer** - extracts credentials from HTTP requests",
|
||||
" - Bearer tokens from `Authorization` header",
|
||||
" - Session cookies from `Cookie` header",
|
||||
" - API keys from `X-API-Key` header",
|
||||
"- **Validation layer** - verifies credentials",
|
||||
" - JWT signature and expiration",
|
||||
" - Session lookup in database",
|
||||
" - API key hash comparison",
|
||||
"- **Authorization layer** - checks permissions",
|
||||
" - Role-based access control (RBAC)",
|
||||
" - Resource-level permissions",
|
||||
"",
|
||||
"### Error Types",
|
||||
"",
|
||||
"The new error hierarchy uses typed sentinel errors:",
|
||||
"",
|
||||
"```go",
|
||||
"var (",
|
||||
" // ErrInvalidToken indicates a malformed or unsigned token.",
|
||||
' ErrInvalidToken = errors.New("invalid token")',
|
||||
"",
|
||||
" // ErrTokenExpired indicates the token's exp claim is in the past.",
|
||||
' ErrTokenExpired = errors.New("token expired")',
|
||||
"",
|
||||
" // ErrSessionNotFound means the session ID doesn't exist.",
|
||||
' ErrSessionNotFound = errors.New("session not found")',
|
||||
"",
|
||||
" // ErrInsufficientPermissions means the user lacks required roles.",
|
||||
' ErrInsufficientPermissions = errors.New("insufficient permissions")',
|
||||
")",
|
||||
"```",
|
||||
].join("\n"),
|
||||
},
|
||||
],
|
||||
},
|
||||
// -- Turn 3: user follow-up --
|
||||
{
|
||||
id: 3,
|
||||
chat_id: CHAT_ID,
|
||||
@@ -253,10 +321,11 @@ export const WithMessageHistory: Story = {
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "That sounds right. Can you start with the token validation? I want to make sure we handle JWT expiration properly.",
|
||||
text: "Can you show me the token validation code and a comparison of the old vs new approach?",
|
||||
},
|
||||
],
|
||||
},
|
||||
// -- Turn 4: assistant with code, table, nested list, task list --
|
||||
{
|
||||
id: 4,
|
||||
chat_id: CHAT_ID,
|
||||
@@ -265,10 +334,72 @@ export const WithMessageHistory: Story = {
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "Here's the refactored token validation:\n\n```go\nfunc ValidateToken(ctx context.Context, token string) (*Claims, error) {\n claims, err := parseToken(token)\n if err != nil {\n return nil, ErrInvalidToken\n }\n if claims.ExpiresAt.Before(time.Now()) {\n return nil, ErrTokenExpired\n }\n return claims, nil\n}\n```\n\nKey changes:\n- Separated parsing from expiration checking\n- Added typed errors (`ErrInvalidToken`, `ErrTokenExpired`) so callers can distinguish between a malformed token and an expired one\n- The context parameter allows us to add tracing later",
|
||||
text: [
|
||||
"## Token Validation (Refactored)",
|
||||
"",
|
||||
"Here is the new `ValidateToken` function with proper error handling and context propagation:",
|
||||
"",
|
||||
"```go",
|
||||
"func ValidateToken(ctx context.Context, token string) (*Claims, error) {",
|
||||
" claims, err := parseToken(token)",
|
||||
" if err != nil {",
|
||||
' return nil, fmt.Errorf("%w: %v", ErrInvalidToken, err)',
|
||||
" }",
|
||||
" if claims.ExpiresAt.Before(time.Now()) {",
|
||||
" return nil, ErrTokenExpired",
|
||||
" }",
|
||||
" return claims, nil",
|
||||
"}",
|
||||
"```",
|
||||
"",
|
||||
"### Old vs New Comparison",
|
||||
"",
|
||||
"| Aspect | Old Implementation | New Implementation |",
|
||||
"|---|---|---|",
|
||||
"| Error types | Generic `error` | Typed sentinel errors |",
|
||||
"| Token parsing | Inline in handler | Extracted to `parseToken()` |",
|
||||
"| Expiry check | Mixed with validation | Separate step with `ErrTokenExpired` |",
|
||||
"| Context | Not used | Passed through for tracing |",
|
||||
"| Testability | Requires HTTP server | Pure function, unit-testable |",
|
||||
"",
|
||||
"### Key Changes",
|
||||
"",
|
||||
"The refactored version improves several areas:",
|
||||
"",
|
||||
"1. **Separation of concerns**",
|
||||
" - Parsing is isolated from expiration checking",
|
||||
" - Each failure mode has its own error type",
|
||||
" - Callers can match on `errors.Is(err, ErrTokenExpired)` to refresh tokens",
|
||||
"2. **Observability**",
|
||||
" - Context carries trace spans",
|
||||
" - Errors wrap the original cause with `%w`",
|
||||
"3. **Testing**",
|
||||
" - Pure function with no HTTP dependency",
|
||||
" - Table-driven tests for each error path",
|
||||
"",
|
||||
"> **Warning:** The old `ValidateAndRefresh` function is now deprecated. Callers should migrate to the new two-step pattern:",
|
||||
"> ```go",
|
||||
"> claims, err := ValidateToken(ctx, token)",
|
||||
"> if errors.Is(err, ErrTokenExpired) {",
|
||||
"> newToken, err := RefreshToken(ctx, token)",
|
||||
"> }",
|
||||
"> ```",
|
||||
"",
|
||||
"---",
|
||||
"",
|
||||
"#### Migration Checklist",
|
||||
"",
|
||||
"- [x] Extract `parseToken()` helper",
|
||||
"- [x] Define sentinel error types",
|
||||
"- [x] Add context parameter",
|
||||
"- [ ] Update all callers in `coderd/`",
|
||||
"- [ ] Add table-driven unit tests",
|
||||
"- [ ] Remove deprecated `ValidateAndRefresh`",
|
||||
].join("\n"),
|
||||
},
|
||||
],
|
||||
},
|
||||
// -- Turn 5: user asks about middleware --
|
||||
{
|
||||
id: 5,
|
||||
chat_id: CHAT_ID,
|
||||
@@ -277,10 +408,11 @@ export const WithMessageHistory: Story = {
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "Looks good. Now what about the middleware deduplication?",
|
||||
text: "Now show me the middleware consolidation with the full config and some inline code references.",
|
||||
},
|
||||
],
|
||||
},
|
||||
// -- Turn 6: assistant with code, inline code, links, images, nested blockquote --
|
||||
{
|
||||
id: 6,
|
||||
chat_id: CHAT_ID,
|
||||
@@ -289,7 +421,106 @@ export const WithMessageHistory: Story = {
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "I've consolidated the three middleware instances into a single composable chain:\n\n```go\nfunc AuthMiddleware(opts ...AuthOption) func(http.Handler) http.Handler {\n cfg := defaultAuthConfig()\n for _, opt := range opts {\n opt(&cfg)\n }\n return func(next http.Handler) http.Handler {\n return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {\n claims, err := ValidateToken(r.Context(), extractToken(r))\n if err != nil {\n cfg.ErrorHandler(w, r, err)\n return\n }\n ctx := context.WithValue(r.Context(), claimsKey, claims)\n next.ServeHTTP(w, r.WithContext(ctx))\n })\n }\n}\n```\n\nThe functional options pattern lets each route customize behavior (e.g. optional auth, different error responses) without duplicating the core logic.",
|
||||
text: [
|
||||
"## Middleware Consolidation",
|
||||
"",
|
||||
"I consolidated the three middleware instances into a single composable chain using the *functional options* pattern. The `AuthMiddleware` function accepts variadic `AuthOption` values to customize behavior per-route.",
|
||||
"",
|
||||
"### Configuration Struct",
|
||||
"",
|
||||
"```go",
|
||||
"type authConfig struct {",
|
||||
" // RequireAuth rejects unauthenticated requests when true.",
|
||||
" RequireAuth bool",
|
||||
" // ErrorHandler is called when authentication fails.",
|
||||
" ErrorHandler func(http.ResponseWriter, *http.Request, error)",
|
||||
" // AllowedRoles restricts access to specific roles.",
|
||||
" AllowedRoles []string",
|
||||
" // TokenSources defines where to look for credentials.",
|
||||
" TokenSources []TokenSource",
|
||||
"}",
|
||||
"",
|
||||
"func defaultAuthConfig() authConfig {",
|
||||
" return authConfig{",
|
||||
" RequireAuth: true,",
|
||||
" ErrorHandler: defaultErrorHandler,",
|
||||
" AllowedRoles: nil, // all roles",
|
||||
" TokenSources: []TokenSource{BearerToken, SessionCookie},",
|
||||
" }",
|
||||
"}",
|
||||
"```",
|
||||
"",
|
||||
"### Middleware Function",
|
||||
"",
|
||||
"```go",
|
||||
"func AuthMiddleware(opts ...AuthOption) func(http.Handler) http.Handler {",
|
||||
" cfg := defaultAuthConfig()",
|
||||
" for _, opt := range opts {",
|
||||
" opt(&cfg)",
|
||||
" }",
|
||||
" return func(next http.Handler) http.Handler {",
|
||||
" return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {",
|
||||
" claims, err := ValidateToken(r.Context(), extractToken(r))",
|
||||
" if err != nil {",
|
||||
" cfg.ErrorHandler(w, r, err)",
|
||||
" return",
|
||||
" }",
|
||||
" ctx := context.WithValue(r.Context(), claimsKey, claims)",
|
||||
" next.ServeHTTP(w, r.WithContext(ctx))",
|
||||
" })",
|
||||
" }",
|
||||
"}",
|
||||
"```",
|
||||
"",
|
||||
"### Usage Examples",
|
||||
"",
|
||||
"Here is how different routes use the middleware:",
|
||||
"",
|
||||
"| Route | Options | Behavior |",
|
||||
"|---|---|---|",
|
||||
'| `GET /api/users` | `WithRoles("admin")` | Requires admin role |',
|
||||
"| `GET /api/me` | *(defaults)* | Any authenticated user |",
|
||||
"| `GET /api/health` | `WithOptionalAuth()` | Auth is optional |",
|
||||
"| `POST /api/webhooks` | `WithAPIKeyOnly()` | Only API key auth |",
|
||||
"",
|
||||
"### Inline References",
|
||||
"",
|
||||
"The key types involved are:",
|
||||
"",
|
||||
"- `AuthOption` is a `func(*authConfig)` that mutates the config",
|
||||
"- `TokenSource` is an enum: `BearerToken`, `SessionCookie`, or `APIKey`",
|
||||
"- `Claims` holds the decoded JWT payload including `sub`, `exp`, and `roles`",
|
||||
"",
|
||||
"For more details, see the [Go middleware patterns](https://pkg.go.dev/net/http) documentation and the ~~old middleware README~~ (now removed).",
|
||||
"",
|
||||
"> **Tip:** You can compose multiple options together:",
|
||||
">",
|
||||
"> ```go",
|
||||
"> r.Use(AuthMiddleware(",
|
||||
'> WithRoles("admin", "editor"),',
|
||||
"> WithCustomErrorHandler(jsonErrorHandler),",
|
||||
"> ))",
|
||||
"> ```",
|
||||
">",
|
||||
"> This replaces the three separate middlewares we had before.",
|
||||
"",
|
||||
"---",
|
||||
"",
|
||||
"#### Performance Benchmarks",
|
||||
"",
|
||||
"| Benchmark | Old (ns/op) | New (ns/op) | Delta |",
|
||||
"|---|---:|---:|---:|",
|
||||
"| `BenchmarkValidateToken` | 4,521 | 1,203 | -73.4% |",
|
||||
"| `BenchmarkMiddlewareChain` | 12,887 | 3,456 | -73.2% |",
|
||||
"| `BenchmarkSessionLookup` | 89,102 | 45,330 | -49.1% |",
|
||||
"| `BenchmarkFullAuthFlow` | 102,340 | 48,912 | -52.2% |",
|
||||
"",
|
||||
"The performance improvement comes mainly from:",
|
||||
"",
|
||||
"1. Removing redundant token parsing (was done 3x per request)",
|
||||
"2. Caching parsed claims in context",
|
||||
"3. Using `sync.Pool` for the JWT parser",
|
||||
].join("\n"),
|
||||
},
|
||||
],
|
||||
},
|
||||
|
||||
@@ -72,7 +72,7 @@ export const AgentDetailTopBar: FC<AgentDetailTopBarProps> = ({
|
||||
const navigate = useNavigate();
|
||||
|
||||
return (
|
||||
<div className="flex shrink-0 items-center gap-2 px-4 py-0.5">
|
||||
<div className="flex shrink-0 items-center gap-2 px-4 py-1.5">
|
||||
{/* Mobile back button */}
|
||||
<Button
|
||||
variant="subtle"
|
||||
@@ -104,22 +104,17 @@ export const AgentDetailTopBar: FC<AgentDetailTopBarProps> = ({
|
||||
<Button
|
||||
size="sm"
|
||||
variant="subtle"
|
||||
className="h-auto max-w-[16rem] rounded-sm px-1 py-0.5 text-xs text-content-secondary shadow-none hover:bg-transparent hover:text-content-primary"
|
||||
className="h-auto max-w-[16rem] rounded-sm px-1 py-0.5 text-sm text-content-secondary shadow-none hover:bg-transparent hover:text-content-primary"
|
||||
onClick={() => onOpenParentChat(parentChat.id)}
|
||||
>
|
||||
<span className="truncate">{parentChat.title}</span>
|
||||
</Button>
|
||||
<ChevronRightIcon className="h-3.5 w-3.5 shrink-0 text-content-secondary/70" />
|
||||
<ChevronRightIcon className="h-3.5 w-3.5 shrink-0 text-content-secondary/70 -ml-0.5" />
|
||||
</>
|
||||
)}
|
||||
<span className="truncate text-sm text-content-primary">
|
||||
{chatTitle}
|
||||
</span>
|
||||
{isArchived && (
|
||||
<span className="shrink-0 rounded bg-surface-tertiary px-1.5 py-0.5 text-xs text-content-secondary">
|
||||
Archived
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user