Enable markdown negotiation for agents#451
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds markdown negotiation utilities, a proxy middleware that rewrites eligible requests to an agent-markdown route, a dynamic Node route handler for GET/HEAD returning markdown, server-side site-markdown generation for several routes, and tests covering negotiation and generation. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Proxy as Proxy\nMiddleware
participant Route as Agent-Markdown\nRoute Handler
participant Site as Site-Markdown\nGenerator
Client->>Proxy: GET /docs/readme\nAccept: text/markdown
Proxy->>Proxy: acceptsMarkdown(Accept)
Proxy->>Proxy: check exclusions (/_next, /api, assets, file.ext, prefix)
Proxy->>Route: rewrite -> /agent-markdown/docs/readme
Route->>Route: extract segments & searchParams
Route->>Site: createSiteMarkdown(["docs","readme"], searchParams)
Site->>Site: resolveMarkdownRoute -> docs\nload doc source / fetch release / pick example
Site-->>Route: return markdown string (or null)
Route->>Route: headers = markdownResponseHeaders(markdown)
Route->>Client: respond 200/404 with headers (body only for GET)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Review rate limit: 0/5 reviews remaining, refill in 59 minutes and 14 seconds. Comment |
Contributor
Benchmark Results407 benchmarks Interpreted: 🟢 33 improved · 🔴 228 regressed · 146 unchanged · avg -3.5% arraybuffer.js — Interp: 🟢 1, 🔴 8, 5 unch. · avg -5.1% · Bytecode: 🟢 8, 6 unch. · avg +4.9%
arrays.js — Interp: 🔴 10, 9 unch. · avg -3.4% · Bytecode: 🟢 12, 7 unch. · avg +4.3%
async-await.js — Interp: 🔴 5, 1 unch. · avg -8.2% · Bytecode: 🟢 3, 3 unch. · avg +10.1%
async-generators.js — Interp: 🔴 2 · avg -9.1% · Bytecode: 2 unch. · avg +4.9%
base64.js — Interp: 🔴 9, 1 unch. · avg -6.9% · Bytecode: 🟢 6, 4 unch. · avg +4.8%
classes.js — Interp: 🔴 24, 7 unch. · avg -4.1% · Bytecode: 🟢 5, 🔴 2, 24 unch. · avg +1.5%
closures.js — Interp: 🔴 7, 4 unch. · avg -3.5% · Bytecode: 🟢 3, 8 unch. · avg +2.1%
collections.js — Interp: 🟢 1, 🔴 10, 1 unch. · avg -6.1% · Bytecode: 🟢 10, 2 unch. · avg +5.8%
csv.js — Interp: 🟢 2, 🔴 4, 7 unch. · avg -1.3% · Bytecode: 🟢 7, 🔴 1, 5 unch. · avg +2.0%
destructuring.js — Interp: 🔴 7, 15 unch. · avg -2.2% · Bytecode: 🟢 14, 8 unch. · avg +5.6%
fibonacci.js — Interp: 🔴 7, 1 unch. · avg -4.8% · Bytecode: 🟢 6, 2 unch. · avg +5.7%
float16array.js — Interp: 🟢 3, 🔴 17, 12 unch. · avg -2.7% · Bytecode: 🟢 18, 🔴 4, 10 unch. · avg +4.8%
for-of.js — Interp: 7 unch. · avg -1.3% · Bytecode: 🟢 6, 1 unch. · avg +8.4%
generators.js — Interp: 🔴 3, 1 unch. · avg -5.6% · Bytecode: 🟢 3, 1 unch. · avg +5.3%
iterators.js — Interp: 🟢 2, 🔴 25, 15 unch. · avg -3.3% · Bytecode: 🟢 28, 🔴 1, 13 unch. · avg +6.2%
json.js — Interp: 🔴 5, 15 unch. · avg -1.8% · Bytecode: 🟢 3, 🔴 5, 12 unch. · avg -0.6%
jsx.jsx — Interp: 🔴 19, 2 unch. · avg -5.2% · Bytecode: 🟢 13, 8 unch. · avg +4.9%
modules.js — Interp: 🟢 1, 🔴 2, 6 unch. · avg -0.9% · Bytecode: 🟢 5, 4 unch. · avg +3.6%
numbers.js — Interp: 🟢 1, 🔴 1, 9 unch. · avg -1.0% · Bytecode: 🟢 2, 🔴 5, 4 unch. · avg +1.2%
objects.js — Interp: 🔴 4, 3 unch. · avg -3.2% · Bytecode: 🟢 1, 🔴 3, 3 unch. · avg -0.3%
promises.js — Interp: 🔴 10, 2 unch. · avg -7.4% · Bytecode: 🟢 6, 6 unch. · avg +3.9%
regexp.js — Interp: 🟢 3, 🔴 1, 7 unch. · avg +1.0% · Bytecode: 🟢 1, 🔴 6, 4 unch. · avg -4.9%
strings.js — Interp: 🔴 14, 5 unch. · avg -4.3% · Bytecode: 🟢 6, 🔴 1, 12 unch. · avg +2.4%
tsv.js — Interp: 🟢 5, 4 unch. · avg +5.5% · Bytecode: 🟢 2, 🔴 2, 5 unch. · avg -0.5%
typed-arrays.js — Interp: 🔴 20, 2 unch. · avg -23.2% · Bytecode: 🟢 4, 🔴 10, 8 unch. · avg -6.9%
uint8array-encoding.js — Interp: 🟢 14, 🔴 2, 2 unch. · avg +40.7% · Bytecode: 🟢 9, 🔴 2, 7 unch. · avg +1.7%
weak-collections.js — Interp: 🔴 12, 3 unch. · avg -32.0% · Bytecode: 🟢 12, 3 unch. · avg +51.9%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
Contributor
Suite Timing
Measured on ubuntu-latest x64. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
website/src/__tests__/markdown-negotiation.test.ts (1)
22-31: Add parser edge-case assertions for invalid/clampedqvalues.Given
parseQualityclamps and normalizes values, lock those behaviors in tests (q=1.5,q=-1,q=abc) to prevent regressions in negotiation semantics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/__tests__/markdown-negotiation.test.ts` around lines 22 - 31, Add tests around acceptsMarkdown that lock parseQuality's clamping/normalization: assert that "text/markdown;q=1.5" is treated as q=1 and returns true, while invalid or out-of-range values are treated as 0 and return false (e.g., "text/markdown;q=-1" -> false and "text/markdown;q=abc" -> false). Use the existing acceptsMarkdown test harness and reference parseQuality behavior to ensure these three edge cases are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/src/lib/site-markdown.ts`:
- Around line 99-101: The code dereferences EXAMPLES items into heroExample (and
similar usage at the later block around lines 214-217), which will throw if
EXAMPLES is empty; update the selection logic to guard against an empty array by
first checking EXAMPLES.length > 0 and if empty using a safe default object
(e.g., { id: "", label: "", desc: "", code: "" }) so downstream reads of
desc/label/code won't crash; apply this same defensive pattern to both the
heroExample assignment and the other EXAMPLES lookups referenced in the file.
In `@website/src/proxy.ts`:
- Around line 16-17: The current pathname checks only handle subpaths but miss
the exact roots; update the conditional logic that uses the pathname variable in
proxy.ts (the block with "if (pathname.startsWith(...))") to also return true
when pathname is exactly "/api" or "/_next" (i.e., treat pathname === "/api" and
pathname === "/_next" as skips in addition to startsWith("/api/") and
startsWith("/_next/")). Ensure both exact-match and prefix-match cases are
covered so exact root paths are not rewritten.
---
Nitpick comments:
In `@website/src/__tests__/markdown-negotiation.test.ts`:
- Around line 22-31: Add tests around acceptsMarkdown that lock parseQuality's
clamping/normalization: assert that "text/markdown;q=1.5" is treated as q=1 and
returns true, while invalid or out-of-range values are treated as 0 and return
false (e.g., "text/markdown;q=-1" -> false and "text/markdown;q=abc" -> false).
Use the existing acceptsMarkdown test harness and reference parseQuality
behavior to ensure these three edge cases are covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: caad7118-71bb-42ae-9b89-b7425f34073b
📒 Files selected for processing (5)
website/src/__tests__/markdown-negotiation.test.tswebsite/src/app/agent-markdown/[[...path]]/route.tswebsite/src/lib/markdown-negotiation.tswebsite/src/lib/site-markdown.tswebsite/src/proxy.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/src/lib/site-markdown.ts (1)
28-32: Escape YAML frontmatter values before interpolation.Raw scalar interpolation can break frontmatter parsing if
titleordescriptionever contains YAML-significant characters (for example:or newlines).Proposed patch
+function yamlQuote(value: string): string { + return JSON.stringify(value); +} + function frontmatter(title: string, description: string): string { - return ["---", `title: ${title}`, `description: ${description}`, "---"].join( + return [ + "---", + `title: ${yamlQuote(title)}`, + `description: ${yamlQuote(description)}`, + "---", + ].join( "\n", ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/lib/site-markdown.ts` around lines 28 - 32, The frontmatter function currently interpolates title and description raw, which can break YAML if they contain colons, newlines, or quotes; update the frontmatter(title, description) implementation to escape or properly quote both values before joining (e.g., use a safe serializer like js-yaml's dump for scalars or JSON.stringify to produce a quoted/escaped string) so the returned frontmatter always yields valid YAML; ensure both title and description are passed through the chosen escaping/serialization function before being inserted into the `title:` and `description:` lines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@website/src/lib/site-markdown.ts`:
- Around line 28-32: The frontmatter function currently interpolates title and
description raw, which can break YAML if they contain colons, newlines, or
quotes; update the frontmatter(title, description) implementation to escape or
properly quote both values before joining (e.g., use a safe serializer like
js-yaml's dump for scalars or JSON.stringify to produce a quoted/escaped string)
so the returned frontmatter always yields valid YAML; ensure both title and
description are passed through the chosen escaping/serialization function before
being inserted into the `title:` and `description:` lines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3e3dde6-fbb6-4a45-a26c-b42a22d3d4c3
📒 Files selected for processing (3)
website/src/__tests__/markdown-negotiation.test.tswebsite/src/lib/site-markdown.tswebsite/src/proxy.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- website/src/proxy.ts
- website/src/tests/markdown-negotiation.test.ts
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
1 participant
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
Verification