Skip to content

docs: MDX content collection + public sync API, resynced with main (continues #4) - #13

Merged
barckcode merged 13 commits into
helmcode:mainfrom
santigamo:docs/content-collection-resync
Jul 14, 2026
Merged

barckcode merged 13 commits into
helmcode:mainfrom
santigamo:docs/content-collection-resync

Conversation

@santigamo

@santigamo santigamo commented Jul 9, 2026 •

Copy link
Copy Markdown
Contributor

Hola. Llegué a la comunidad hace poco, vi que el #4 llevaba cinco semanas parado y que bloqueaba los issues #8, #9 y #10, y me puse a mirarlo. Acabé arreglándolo, así que aquí lo traigo.

Antes de nada: el trabajo de fondo es de @Saul-Gomez-J y @Nxssie. La migración a content collection, el pipeline con remark-mdx, la whitelist de componentes con fail-fast, los fixtures, el ETag. Todo eso ya estaba y está bien hecho. Sus commits vienen intactos y con su autoría, y los he acreditado como co-autores. Yo he recogido el testigo donde se quedó.

No tengo permiso de push en el repo, y la rama del #4 no vive en un fork, así que no podía continuar allí. De ahí este PR nuevo. Si preferís que esto entre dentro del #4, o contra la rama de Saúl en vez de contra main, decídmelo y lo reoriento sin problema.

Antes de que te asuste el diff

GitHub dice 62 ficheros y eso da pereza solo de verlo. Pero el grueso es la conversión de Saúl, que ya revisaste dos veces en el #4 (56 ficheros, +3574 y -2211). Lo que ha cambiado desde aquella review son 20 ficheros, +914 y -72, repartidos así:

ficheros líneas
Código 8 +216 / -43
Tests 3 +248
Contenido de docs (portado de main) 5 +423 / -21
package-lock.json 1 +3 / -4

O sea que la superficie de código que hay que leer con cuidado son 216 líneas. El resto es contenido portado, que se revisa comparando contra los .astro de main, y tests.

Si te sirve, este es el orden que yo seguiría, commit a commit:

  1. 5fd334e el fail-fast. Dos ficheros. Es el que tiene más chicha conceptual.
  2. 5dbafec la fuente única de verdad de rate limits.
  3. 8ce89a8 el test de paridad con el bot.
  4. 26a71eb el contenido portado. Es largo pero mecánico: cada bloque sale de git show origin/main:src/pages/docs/<x>.astro.
  5. e0c2eec los arreglos que salieron al revisar lo anterior.

Del merge (3e3b88c) lo único que necesita ojos es la resolución del conflicto de examples.md. Los tres .astro borrados son las páginas que este PR migra, y su contenido no se pierde: se porta en el commit 4.

Y si aun así te parece demasiado, dilo y lo parto en PRs encadenados: uno con el merge y el resync, otro con los tres fixes. Prefiero que lo revises a gusto que colártelo entero.

Las cuatro correcciones de la review

El rateLimitsToMd() con los valores a mano. Ahora hay un src/lib/rateLimits.ts que es la fuente única de verdad, y lo leen tanto <RateLimits /> como la API de docs. Los dos límites por key salen de wrangler.jsonc bajo vars, con los mismos números como fallback. Si alguien pone un valor inválido, avisa por consola y usa el default, en vez de tumbar las docs.

Y resultó que hacía más falta de lo que parecía. Al mergear main, el componente pasó a renderizar 60 rpm mientras rateLimitsToMd() seguía sirviendo 100 rpm al bot. La página y la API decían cosas distintas del mismo límite, y nada lo detectaba. Por cierto que el número no vivía en dos sitios sino en tres: el tercero es prosa dentro de api.astro, en la sección de imágenes, con el 100 rpm ya viejo.

Sobre el valor en sí, @barckcode: decías 60 rpm y 3 concurrentes, pero main se corrigió solo en a37bb63, "the real per-key default is max_parallel_requests: 5". He puesto 60 y 5. Si el dato bueno es otro, es cambiar una línea.

Las expresiones MDX que se descartaban en silencio. Hecho, aunque con un matiz que cuento más abajo porque merece su propia conversación.

El test de paridad del hash. Está en src/lib/canonicalParity.test.ts. Transcribe canonicalize_doc_text del bot de forma independiente, como un escaneo de code points en lugar de una regex, para que si me equivoco de un lado no se copie el error en el otro. Comprueba el punto fijo canonicalize(mdxToText(x)) === mdxToText(x) sobre los fixtures y todo el corpus, con strip_frontmatter en los dos sentidos, y que ningún cuerpo canónico empieza por ---.

Los marcadores <!--ARTICLE-START/END-->. Esos ya no estaban. Saúl los quitó en c180c40, cuatro días antes de la review. grep da cero.

Poner la rama al día

Eran 34 commits de deriva. La rama se quedó congelada en mayo y main siguió documentando cosas, así que mergear sin más habría borrado de producción cuatro modelos y tres endpoints. He portado todo eso a la content collection: mimo-v2.5, glm5.2, rerank y flux-2-klein a models.mdx; POST /v1/rerank, los dos endpoints de imágenes y reasoning_effort a api.mdx; la tabla de tiers de Space a apps.md; y a examples.md las secciones de rerank y mimo-v2.5 más los configs de IDE con glm5.2. La cuota de deepseek-v4-flash pasa de 100M a 500M.

Tres sitios donde no he hecho lo que pedía la review

Y creo que en los tres tengo razón, pero son vuestros de decidir.

El fail-fast en mdxjsEsm. La review pedía lanzar en mdxjsEsm, mdxFlowExpression y mdxTextExpression. En los dos últimos, totalmente de acuerdo, y ahí está. Pero mdxjsEsm es el nodo de los import de cabecera, y api.mdx y models.mdx importan sus propios componentes; [...slug].astro renderiza <Content /> sin prop components, así que esos imports son la única vía por la que llegan. Si lanzamos ahí, revientan todos los tests del corpus y /api/docs/* devuelve 500. Además chocaría con un test que ya existía, el que afirma expect(codeFenceFree).not.toMatch(/^import\s/m): los imports se descartan a propósito.

Ahora bien, el instinto de la review era bueno. Solo que el hueco estaba una capa más abajo, y me costó verlo. transformTree no recorre .attributes, pero componentToBlockMd() sí las consume a través de getAttr(), y astToValue() devolvía undefined para todo lo que no supiera reducir. Resultado: una prop no literal hacía desaparecer el componente entero del texto que lee el bot, mientras la página seguía renderizándolo tan tranquila.

<EndpointGrid items={[{ method: 'GET', path: '/v1/models' }]} />   →  "- `GET /v1/models`"
<EndpointGrid items={items} />                                      →  ""

Ahora lanzan las expresiones sueltas, los identificadores, los spreads, los template literals con interpolación y las claves computadas de objeto. Esta última era la más traicionera: { [label]: 'model' } tiene una clave de tipo Identifier, así que se leía como el campo literal label, escribía la clave equivocada y dejaba en blanco la que el componente esperaba.

El nombre de las env vars. Proponías PUBLIC_RATE_LIMIT_RPM y PUBLIC_RATE_LIMIT_PARALLEL, y las he dejado como RATE_LIMIT_RPM y RATE_LIMIT_PARALLEL. El motivo es que en Astro el prefijo PUBLIC_ significa algo concreto: variable expuesta al cliente vía import.meta.env. Aquí se leen en servidor con import { env } from 'cloudflare:workers', que es el patrón del repo. Los valores son públicos, el mecanismo no, y me daba miedo que dentro de seis meses alguien diera por hecho que viajan al bundle. Si preferís el nombre original es un sed, sin discusión.

El pythonStrip(). Este no lo pedisteis y es un cambio de comportamiento, así que lo cuento entero. Escribiendo el test de paridad salió que .trim() de JS y .strip() de Python no recortan el mismo conjunto de caracteres: JS quita U+FEFF, que Python conserva, y Python quita de U+001C a U+001F y el U+0085, que JS conserva. Como el bot recanonicaliza lo que servimos antes de hashearlo, un carácter de control invisible al final de un doc bastaría para que los hashes no coincidieran nunca.

Hoy no cambia ni un hash, porque ningún documento lleva esos caracteres. Y siendo honesto, el test de idempotencia ya pondría el CI en rojo si mañana alguien pega uno. O sea que esto es cinturón además de tirantes. Si os parece ruido en este PR, lo saco a uno aparte y aquí nos quedamos solo con el test.

Dos cosas del bot, para cuando toque

No son de este PR, pero las vi de camino y prefiero decirlas.

docs_client.py:265 llama a canonicalize_doc_text con strip_frontmatter=True sobre un cuerpo que ya viene sin frontmatter. El comentario explica que es a propósito, por cachés viejas, y tiene sentido. Lo que pasa es que el stringifier usa rule: '-', así que un texto canónico que empezara por un thematic break --- haría que _FRONTMATTER_RE se comiera contenido hasta el siguiente. Hoy no lo dispara nadie y el test nuevo lo vigila. Se cerraría del todo poniendo rule: '*'.

Y una matización a la review: si el punto fijo se rompiera, no habría re-embed en cada poll. knowledge.py:351 corta por docs_manifest_version antes de comparar hashes documento a documento. Lo que pasaría es que esos docs se reindexarían en cada versión nueva de manifest aunque no hubieran cambiado, más un warning por fetch. Molesto y silencioso, no perpetuo.

Cómo lo he comprobado

Con los pasos exactos de .github/workflows/ci.yml y en Node 22: npm ci, npm test (275 tests, antes eran 180), npx astro check sin errores, y npm run build.

Y levantando el servidor, porque los tests no lo ven todo: las siete páginas de docs devuelven 200 y no queda ni una ancla interna rota, /api/docs/*.md no sirve HTML crudo ni entidades ni dobles backslash, ningún cuerpo empieza por ---, el If-None-Match sigue devolviendo 304 y el contentHash del manifest coincide con el X-Content-Hash de cada documento.

Por el camino salieron tres cosillas más que he arreglado. examples.md había perdido el id="qwen36-zed" en la conversión, y el enlace de "config completo arriba" apuntaba a un id que no existía; lo he restaurado en vez de repuntar el enlace, para no romper enlaces externos que ya anden por ahí. node.js y zed no son lenguajes de Shiki y los ejemplos caían a texto plano. Y unist-util-visit estaba en el package.json sin usarse. Con eso astro check pasa de 7 hints a 1, y el que queda está en CommunityBuilds.astro, que esta rama ni toca.


Nada de esto es innegociable. Si algo no encaja con cómo queréis llevar las docs, decidlo y lo cambio. Y si preferís que sea Saúl quien lo cierre, por mí perfecto: la rama está aquí y se puede coger entera.

Co-authored-by: Saúl Gómez Jiménez gomezjimenezsaul@gmail.com
Co-authored-by: Nxssie nxssiedev@gmail.com

Saul-Gomez-J and others added 12 commits May 25, 2026 18:35
Replace per-page .astro files with an Astro content collection
(src/content/docs/) so docs are a single source of truth that drives
both the rendered website and a public, machine-readable API consumed
by the Discord bot (and future clients).

Site changes:
- Add @astrojs/mdx integration.
- Define a docs collection with frontmatter schema (title, description,
  order, locale) loaded via glob.
- New dynamic route src/pages/docs/[...slug].astro renders entries; the
  layout reads the collection for nav/breadcrumb so adding a doc no
  longer requires editing route or nav lists.
- Extract reusable rich UI primitives (ModelCard, LimitationsCard,
  EndpointGrid, FieldList, Callout) so MDX stays readable.
- Polish .docs-content typography (h3/links/lists/blockquotes/tables/
  images), wrap markdown <pre> with a labeled toolbar + copy button,
  and constrain the inline-code background rule with :not(pre) > code
  so Shiki blocks no longer pick up the "selected" look.

Public API:
- GET /api/docs/manifest.json — versioned index with per-entry
  sha256 content hash and contentUrl.
- GET /api/docs/[slug].md — server-renders the live page, extracts the
  article between <!--ARTICLE-START--> / <!--ARTICLE-END--> markers,
  and converts the HTML to clean Markdown via lib/htmlToText.ts.
- Responses set Cache-Control, ETag and X-Content-Hash.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…lection

# Conflicts:
#	src/content/docs/examples.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit incorrectly renamed "### curl" section headings to
"### bash" along with code fence languages. The headings denote the
request method (curl vs SDK) rather than a syntax highlight language,
so they must stay as "curl". Code fences remain as "bash".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address PR helmcode#4 review blockers and shape the contract the bot will sync
against.

- Replace the SSR+htmlToText pipeline with a remark-based mdxToText that
  works on entry.body. Strips mdxjsEsm imports/exports, converts our MDX
  components (ModelCard, LimitationsCard, EndpointGrid, FieldList,
  Callout, RateLimits) and author-written HTML (h1-h4, a, code,
  strong/b, em/i, br, span) to markdown, then normalises to a stable
  canonical text. Unknown JSX components throw to fail closed.
- Rewrite /api/docs/manifest.json and /api/docs/[slug].md to hash the
  canonical text. Both endpoints honour If-None-Match (304 with Cache-
  Control + ETag + X-Content-Hash), validate slug with the shared
  SAFE_SLUG, and serve Cache-Control: public, max-age=900, s-maxage=900
  aligned with the bot's docs_refresh_interval.
- Manifest version is sha256 of [(slug, contentHash)] sorted, giving the
  bot a stable short-circuit.
- Drop the ARTICLE-START/END markers from Docs.astro and delete
  src/lib/htmlToText.ts; they are no longer reachable.
- Add nodejs_compat to wrangler.jsonc so the unified/remark stack runs
  on the Cloudflare Workers dev runtime.
- Tests: contentHash (empty/ASCII/UTF-8), docsApi helpers (SAFE_SLUG,
  Cache-Control, If-None-Match parsing), mdxToText against fixtures per
  component plus corpus invariants (no imports/exports, no residual
  HTML, every used component mapped, unknown component throws), and
  endpoint integration tests for both routes (shape, ordering, stable
  version/ETag, 304, per-entry hash matches body endpoint).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- mdxToText: replace TS2352 casts with index-signature access
- tsconfig: exclude tests so astro check no longer needs node:* types

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Bring 34 commits of main into the content-collection branch.

- Accept the deletion of src/pages/docs/{api,apps,models}.astro, which this
  branch migrated to src/content/docs/. Their new content from main is not
  lost: it is ported to MDX in follow-up commits.
- Port main's additions to examples into the markdown collection: the rerank
  and mimo-v2.5 sections, glm5.2 in the IDE config snippets, and the 500K
  contextWindow cap for mimo-v2.5 in the opencode snippet.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…luate

transformTree dropped mdxjsEsm, mdxFlowExpression and mdxTextExpression
alike. Only the first is safe to drop: import/export is how .mdx pulls in
its components, and src/pages/docs/[...slug].astro renders <Content />
without a `components` prop, so throwing there would break every doc.

The gap that actually mattered was elsewhere. transformTree never walks
.attributes, but componentToBlockMd() consumes them through getAttr(),
and astToValue() returned undefined for anything it could not reduce.
A non-literal prop therefore made the whole component vanish from the
canonical text while the page kept rendering it:

    <EndpointGrid items={[{ method: 'GET', path: '/v1/models' }]} />
    -> "- []() - `GET /v1/models`"
    <EndpointGrid items={items} />
    -> ""

Now astToValue throws on identifiers, spreads, interpolated template
literals and any other non-literal node, getAttr rejects spread
attributes, and bare {expr} at block or inline level throws. import and
export keep being dropped, which mdxToText.test.ts already pinned.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The numbers lived in three places and had already drifted. After merging
main, <RateLimits /> rendered 60 rpm while rateLimitsToMd() served 100 rpm
to the Discord bot, and the extractor knew nothing about the per-model tpm
and rerank tables the component shows.

src/lib/rateLimits.ts now owns the data. The component and the docs API
both read it, so the page and the bot cannot disagree. The two per-key
limits come from PUBLIC_RATE_LIMIT_RPM and PUBLIC_RATE_LIMIT_PARALLEL,
read through `cloudflare:workers` like every other env var here; an
invalid value warns and falls back rather than taking the docs down.

The per-model tables stay as typed data: they only change when a model is
added or removed, which is a code change anyway.

Defaults are 60 rpm and 5 concurrentes, per a37bb63, which corrected the
3 concurrentes the review had assumed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Discord bot re-canonicalises the body it fetches from /api/docs and
hashes it before comparing against our contentHash (nan-discord-bot,
bot/docs_client.py:265 and bot/knowledge.py:390). Our output therefore has
to be a fixed point of bot/knowledge.py::canonicalize_doc_text.

canonicalParity.test.ts transcribes that function independently, as a
code-point scan rather than a regex, so a typo on our side cannot be
mirrored in the reference. It asserts the fixed point over every fixture
and every doc, with strip_frontmatter both ways, and that no canonical body
opens with `---` (rule: '-' serialises thematic breaks that way, and the
bot's frontmatter regex would eat up to the next one).

Doing this surfaced a real divergence: .trim() and str.strip() do not agree.
JS strips U+FEFF, which Python keeps; Python strips U+001C..U+001F, which JS
keeps. normalizeCanonicalText now uses pythonStrip(), matching the 29 code
points Python's str.isspace() accepts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The branch had been frozen since May while main kept documenting new
models and endpoints. Everything below existed only in the .astro pages
this branch deletes, so merging without porting it would have removed it
from production.

models.mdx: mimo-v2.5, glm5.2, rerank and flux-2-klein as ModelCards;
deepseek-v4-flash's monthly quota corrected from 100M to 500M tokens.

api.mdx: POST /v1/rerank, POST /v1/images/generations and
POST /v1/images/edits; reasoning_effort for deepseek-v4-flash; the new
models in the endpoint index, in GET /v1/models and in the per-model
capability card; the 400 and 403 error rows.

apps.md: the Space tiers table (CPU, RAM, disk, pods, price).

intro.md: 100 rpm was stale, it is 60.

Two deliberate departures from main. The prose in the image rate limits
section repeated "100 rpm / 5 concurrentes"; it now links to the rate
limits section instead of restating numbers that already drifted once.
And ModelCard renders `description` as plain text, not set:html, so the
<code> tags main used inside it were dropped rather than shipped literally.

Verified against a running server: every docs page renders the new content,
/api/docs serves it, If-None-Match still yields 304, and the manifest
hashes match the per-document X-Content-Hash.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Computed object keys were still losing content silently. `{ [label]: 'x' }`
has an Identifier key, so astToValue() read it as the literal field `label`,
wrote the wrong key, and blanked out the one the component expected:

    export const label = 'name'
    <FieldList fields={[{ [label]: 'model', type: 'string', description: 'desc' }]} />
    -> "- `` - *string* - desc"

astToValue() now rejects computed keys, with a regression test.

examples.md lost `id="qwen36-zed"` in the conversion, so the "config completo
arriba" link pointed at an id the markdown heading never generated. Restored
the explicit id rather than repointing the link, which keeps existing deep
links working. The TOC is built from the DOM, so a raw <h3> still appears.

Rate limits are now deploy-reproducible: the values live in wrangler.jsonc
under `vars`. Renamed away from PUBLIC_*, which in Astro means "exposed to the
client through import.meta.env" and would mislead, since these are read
server-side from the Workers env.

Also: `node.js` and `zed` are not Shiki languages and fell back to plaintext;
dropped the unused `unist-util-visit`; and `z` from `astro:content` is
deprecated, so content.config.ts imports it from `astro/zod` as Astro asks.
That takes `astro check` from 7 hints to 1, and the last one is in a landing
component this branch does not touch.

Verified with the repo's CI steps on Node 22: npm ci, npm test (275),
npx astro check (0 errors), npm run build. Every docs page renders with no
broken internal anchors, and /api/docs serves no raw HTML or entities.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@barckcode barckcode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really strong PR, and thanks for picking this up and crediting Saúl and Nxssie properly. I went through it and re-ran the CI locally on Node 22: npm test (275 passing), npx astro check (0 errors, 0 warnings; the single hint is in CommunityBuilds.astro, which this branch does not touch), and npm run build. Approving the direction.

Security of the public sync API looks solid. SAFE_SLUG is anchored and disallows . and /, so there is no path traversal; getEntry('docs', slug) scopes lookups to the collection; there are no secrets in wrangler.jsonc or .env.example; ETag / 304 and content types are correct. The regex HTML stripping only ever runs over repo-authored doc content, never external input, so there is no injection or ReDoS surface there.

The mdxToText fail-fast is in the right layer. astToValue / getAttr throwing on non-literal props, spreads, bare expressions and computed object keys is exactly right, and the { [label]: 'x' } case is a sharp catch. The pythonStrip parity work is well reasoned.

Your three deviations: I agree with all three.

  1. Not throwing on mdxjsEsm: correct. Imports do not render and are the only path the components arrive through; the real gap was a layer down, which you fixed.
  2. RATE_LIMIT_* instead of PUBLIC_*: correct. PUBLIC_ in Astro means client-exposed via import.meta.env, and these are read server-side, so the original name would have been misleading.
  3. pythonStrip: fine to keep as a guard against a future control character breaking hash parity.

60 rpm and targeting main are both good. Leave them as they are.

Two small refinements to fold in before merge (non-blocking):

  1. Wrap the API routes in try/catch. Today an unsupported doc makes mdxToText throw and the route returns a raw 500 (and for manifest.json, one bad doc fails the whole manifest). CI validates every doc so it should not happen in practice, but a public endpoint should catch, log the slug plus the error for observability, and return a controlled 500. Keep it loud though: do not silently skip the failing doc, since dropping it would reintroduce the silent content loss this PR exists to prevent.

  2. Fail fast instead of silently filtering non-SAFE ids in the manifest. manifest.json.ts does .filter((e) => SAFE_SLUG.test(e.id)), so a doc in a subfolder (guides/foo) would disappear from the manifest with no trace, which is the same silent-loss pattern you are otherwise careful about. Since all docs are flat today, the cleanest fix is to throw when an id does not match SAFE_SLUG (CI catches it) rather than filter it out quietly. If nested docs ever become a real need, that is a separate change: a [...slug].md route plus a pattern that allows / but not ...

Bot follow-ups: your two notes on the bot repo (docs_client.py rule: '-' to '*', and the knowledge.py manifest-version short-circuit) are good catches. They belong in the bot repo, not here, so let's track them separately.

Nice work overall. Once the two refinements above are in, this is good to merge against main.

Two refinements from the sign-off review of PR helmcode#13.

Both public routes now wrap their handlers in try/catch: an unexpected
mdxToText failure logs the slug plus the error and returns a controlled 500
with no internal details in the body, instead of an unhandled 500. The
invalid-slug 400 stays outside the try. Nothing is skipped or degraded: one
bad doc still takes down manifest.json loudly, by design.

manifest.json no longer filters non-SAFE_SLUG ids silently; it throws, so a
doc whose effective id has a slash can never quietly vanish from the
manifest. The id a doc actually gets is decided by the glob loader, which
honours a frontmatter `slug:` verbatim before schema parsing and slugifies
path segments otherwise, so the CI corpus guard mirrors the loader exactly:
same frontmatter parser (@astrojs/markdown-remark), same discovery library,
pattern and options (tinyglobby), same per-segment github-slugger and
trailing /index collapse. Mechanism self-tests pin each branch: frontmatter
overrides in any YAML form, path normalization, dot-entry exclusion and
directory symlinks.

New route tests exercise the real GET handlers with the real mdxToText
throwing on a bare MDX expression: 400/404/304/200 behaviour is unchanged
and the 500s are controlled and logged.

Verified: npm test (294), npx astro check (0 errors), npm run build, and a
dev-server smoke run (manifest 200 with 7 entries, 304 on If-None-Match,
400 on traversal).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@santigamo

santigamo commented Jul 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Gracias por la review, Cristian. Los dos cambios están dentro, en 278db7a.

Las dos rutas van ahora en try/catch. Si mdxToText lanza, quedan el slug y el error en los logs y el endpoint responde un 500 controlado, sin detalles internos en el cuerpo. El 400 de slug inválido sigue fuera del try. Y mantuve lo que pediste de no saltarse el doc que falla: un doc malo sigue tirando el manifest entero, solo que ahora con rastro.

En el manifest, el filter silencioso ahora es un throw que dice qué id no cumple. Al montar el test de CI apareció una sutileza: el glob loader acepta slug: en el frontmatter como id de la entrada, tal cual y antes de pasar por el schema. O sea que un fichero plano con slug: guides/foo da el mismo problema que un guides/foo.md anidado. Para no perseguir eso a mano, el test usa las mismas piezas que el loader: tinyglobby para descubrir ficheros (mismo patrón y opciones), parseFrontmatter de @astrojs/markdown-remark para el frontmatter, y github-slugger por segmento con el colapso de /index para la ruta. Cada rama tiene su test: overrides en cualquier forma YAML, normalización de rutas, dotfiles y symlinks.

Los tests de las rutas ejercitan los GET reales con mdxToText lanzando de verdad: 400/404/304/200 quedan como estaban y los 500 salen controlados y logueados. En total 294 tests, astro check limpio, build limpio y un smoke contra el dev server (manifest 200 con los 7 docs, 304 con If-None-Match, 400 en el traversal).

Por mi parte no puedo lanzar el merge, así que cuando puedas es tuyo ;)

@santigamo
santigamo requested a review from barckcode July 14, 2026 13:11

@barckcode barckcode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both points from the last review are addressed cleanly, and thanks for the extra tests.

  • Controlled 500s: both routes now wrap the work in try/catch, log with context via console.error, and return a plain Internal error 500 instead of an unhandled throw. Good.
  • Manifest fail-fast: the silent .filter((e) => SAFE_SLUG.test(e.id)) is gone. A non-slug-safe id now throws and surfaces as the controlled 500, so a mis-shaped doc id fails loud instead of quietly disappearing from the index. Exactly what I was after.

Re-ran it locally: 294 tests passing (up from 275, good coverage on the two new route test files), and astro check is clean (0 errors, 0 warnings; the single hint is the pre-existing one in CommunityBuilds.astro, which this branch does not touch).

Looks good to merge against main. Nice work.

@barckcode
barckcode merged commit ec2d862 into helmcode:main Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants