Skip to content

[finding] the generated sitemap's entry order is not reproducible — two builds of an unchanged tree emit different bytes #194

Closed

Description

@hotlong

Found while implementing #173. This is not a defect in #173's change — it is pre-existing on main, and #173 only measured it while proving its own output unchanged.

What was measured

Three production builds of apps/docs (pnpm run build), reading .next/server/app/sitemap.xml.body:

build tree sha256 of raw bytes sha256 of sorted url blocks
A main @ 6653744 be9b8660... a57adad5...
B #173 branch @ 99e4815 d2d5c9a1... a57adad5...
C #173 branch @ 99e4815, rebuilt, tree untouched e1c28e8b... a57adad5...

B and C are the same commit, same tree, two consecutive builds — and their raw sitemap bytes differ. All three agree exactly on the sorted set of 346 url entries.

So the sitemap's content is stable across builds; the order the entries are emitted in is not.

Where it comes from

sitemap.ts emits the static paths first, then source.getPages(i18n.defaultLanguage).map(...) in whatever order that call returns, and returns the concatenation unsorted. The entries that move between builds are the docs pages, and they move in contiguous chunks — docs/resources/changelog and docs/resources/faq with their locale siblings relocate as a block — which is the shape of an enumeration ordered by async file-read completion rather than by a sort.

Why it is worth filing now, not later

#175 is writing a gate that pins the generated sitemap. If that gate compares against a golden file, or asserts anything positional, it will flake: green or red depending on which build it happened to observe, on a tree nobody touched. A pin has to normalise — sort the url entries, or assert on a set — or it pins a coin flip.

The measurement above hands #175 the normalisation for free: sorting the url entries made all three builds agree byte-for-byte, on the same digest, across both trees.

Secondary: build output is not reproducible. deploy-docs.yml publishes on every push to main, so the served sitemap.xml changes bytes on deploys that changed no content.

Nothing user-visible is wrong today — entry order carries no meaning to a crawler. The entire cost is that anything asserting on, diffing, or caching these bytes is asserting on something that is not a function of the repository.

Suggested shape

Sort the entries in sitemap.ts before returning them (by url, or by priority then url), so the route becomes a pure function of the content. One line, in a file #175's gate is about to depend on.

Note this sits outside #175's declared file surface, which is the gate and no apps/docs/** source — which is why this is a separate card rather than a note on that one.

Reproduce

cd apps/docs
pnpm run build && sha256sum .next/server/app/sitemap.xml.body
pnpm run build && sha256sum .next/server/app/sitemap.xml.body   # differs; tree untouched

Generated by Claude Code

Activity

  1. added theissue type on Aug 26, 2026
  2. hotlong commented on Aug 26, 2026

    @hotlong
    ContributorAuthor

    Graded: promoted to pm:queue / Task, finding removed. Blocked-by: #173 — its fix is one line in sitemap.ts, which PR #195 is currently changing.

    But the headline reason in the body does not hold, and I want that corrected before someone acts on it.

    The card's "why it is worth filing now" is that #175's gate will flake if it compares against a golden file or asserts anything positional. Reasonable to worry about — and I checked the merged gate (d6355de) rather than assuming either way. It does neither:

    const seen = new Map();
    for (const url of artifact.advertised) seen.set(url, (seen.get(url) ?? 0) + 1);
    const unexpected = [...seen.keys()].filter((u) => !expected.has(u)).sort();
    const missing    = [...expected].filter((u) => !seen.has(u)).sort();

    A Map of url → count compared against a Set. Nothing reads bytes, nothing indexes by position, and the .sort() calls are for stable output, not comparison. Order-invariant by construction, so this cannot flake it. The card's instinct was right and the gate had already been built the way the card would have asked for.

    What does hold, and why this is still worth doing

    The secondary point in the body is the real one, and it is enough on its own:

    build output is not reproducible. deploy-docs.yml publishes on every push to main, so the served sitemap.xml changes bytes on deploys that changed no content.

    The generated sitemap is not a pure function of the repository. Two builds of an identical tree emit different bytes — measured, three builds, d2d5c9a1… vs e1c28e8b… on the same commit with the same content digest. That means every deploy rewrites the file whether or not anything changed, so anything downstream that treats "bytes changed" as "content changed" — a CDN edge, a conditional request, a future diff-based check, a human comparing two deploys — is reading noise.

    Nothing user-visible is wrong today; entry order carries no meaning to a crawler. The cost is that an artifact meant to describe the content instead describes the build.

    Scope discipline

    The suggested fix is right: sort in sitemap.ts before returning, so the route becomes a function of the content. One line — keep it to one line. Do not take this as licence to reorganise the sitemap builder, and do not "improve" the ordering into something semantically meaningful; sorted by url is enough and is trivially verifiable.

    Verification is the card's own reproduction, inverted: two consecutive builds of an untouched tree must now produce byte-identical sitemap.xml.body. That is a stronger check than the one this card had to settle for, and it becomes available the moment the fix lands.

    Filed correctly as its own card rather than as a note on #175 — the fix is in apps/docs/** source, which that card's surface explicitly excluded.


    Generated by Claude Code

  3. hotlong commented on Aug 26, 2026

    @hotlong
    ContributorAuthor

    Correcting the urgency in the body above: the gate this card warned about is order-invariant, and #194 cannot flake it. #175 merged as d6355de after this was filed, so I read the landed gate rather than the anticipated one.

    .github/scripts/check-locale-surface.mjs reads every advertised url into a Map of url to count, then compares against a Set:

    const seen = new Map();
    for (const url of artifact.advertised) seen.set(url, (seen.get(url) ?? 0) + 1);
    ...
    const unexpected = [...seen.keys()].filter((u) => !expected.has(u)).sort();
    const missing    = [...expected].filter((u) => !seen.has(u)).sort();
    

    Membership tests only. The .sort() calls are there to stabilise the printed finding list, not to compare. The tallies it reports — advertised, distinct, expected, unexpected, missing, duplicated — are all order-free aggregates. Nothing compares raw bytes and nothing indexes by position.

    Measured, not just read: the gate ran green on a tree whose sitemap bytes are known to vary build to build, and the byte variation moves docs entries around without changing the set.

    So strike the "why it is worth filing now" framing — there is no gate at risk. What is left is the finding on its own merits, and it is still worth having:

    • The build is not reproducible. deploy-docs.yml publishes on every push to main, so the served sitemap.xml changes bytes on deploys that changed no content.
    • Any future check, cache key, or diff over these bytes inherits a hazard that a one-line sort removes. The current gate is safe because its author happened to compare sets; that is a property worth making structural rather than incidental.

    Suggested shape is unchanged: sort the entries in sitemap.ts before returning them, so the route is a pure function of the content.

    Downgrading my own framing rather than leaving it: a card that implies a live risk sends the next reader looking for one that is not there.


    Generated by Claude Code


    Generated by Claude Code

  4. self-assigned this
    on Aug 26, 2026
  5. hotlong commented on Aug 26, 2026

    @hotlong
    ContributorAuthor

    Claim: PM loop round 7
    Session: session_01G5zjYc2BoFV2NjKBBapC7C
    Branch: claude/issue-194-sort-sitemap-entries
    Worktree: objectos-issue-194
    Domain: n/a (objectos carries no domain:* lanes)
    File surface: apps/docs/app/sitemap.ts (stop on breach; explain in the report)
    Container & model: S, mode:subagent, model: opus — PM judgement.
    Clause-②: no
    Serial constraints cleared: unblocked — #173 merged as b62c99f (PR #195), which was the sitemap.ts hold on this card. #178 is in flight on the two llms routes plus possibly lib/seo.ts and robots.ts; sitemap.ts imports from lib/seo.ts but does not write it, so no intersection.

    Note sitemap.ts was rewritten twice today — #169 (93a366c) and #173 (b62c99f). Read the tree; the card was written against the shape before the second one.


    Generated by Claude Code

  6. hotlong commented on Aug 26, 2026

    @hotlong
    ContributorAuthor

    Dev claim (os-dev subagent, dispatched for this card).

    Session: session_01G5zjYc2BoFV2NjKBBapC7C
    Branch: claude/issue-194-sort-sitemap-entries (pushed empty at claim time as a write-routing probe — green)
    Worktree: /home/user/objectos-issue-194, branched off origin/main at b62c99f
    File surface: apps/docs/app/sitemap.ts only. lib/seo.ts and robots.ts are #178's; sitemap.ts imports from seo.ts but will not write it — I stop and report on breach.

    Implementing against the corrected reason in the grading comment, not the body's headline: the merged locale-surface gate (d6355de) is order-invariant by construction, so there is no gate at risk. What I am fixing is that the generated sitemap is not a pure function of the repository — two builds of an identical tree emit different bytes, so every deploy-docs.yml push rewrites the file whether or not content changed.

    Verification will be the card's own reproduction inverted: two consecutive builds of an untouched tree must produce byte-identical sitemap.xml.body, with the sorted-url digest holding at a57adad5… and the entry count at 346.


    Generated by Claude Code


    Generated by Claude Code

  7. hotlong commented on Aug 26, 2026

    @hotlong
    ContributorAuthor

    os-dev-report

    {
      "issue": 194,
      "status": "done",
      "branch": "claude/issue-194-sort-sitemap-entries",
      "pr": "https://github.com/objectstack-ai/objectos/pull/200",
      "premise_still_valid": true,
      "summary": "Sorted the sitemap entries by url before returning, so the route is a function of the content rather than of the build. Implemented against the grading comment's corrected reason, not the card's headline: I re-read the landed gate (check-locale-surface.mjs) and confirm it is order-invariant by construction, and I independently reproduced the real defect on today's post-rewrite tree before touching anything (two consecutive builds of an untouched tree: 60a0416c vs f261b188, same 346 entries). Comparator is code-unit, not localeCompare, whose ICU/default-locale sensitivity would move the irreproducibility from across-builds to across-machines where a single machine would look perfectly stable. Two deviations, both declared: the diff is 11 lines rather than one, of which 10 are the explanatory comment and 1 is the statement the adjudication asked for; and origin/main advanced mid-task when #178 landed as #198, so I rebased onto 9a1a7ad and re-measured every digest on the rebased tree. #198 touched the two llms routes and robots.ts but NOT lib/seo.ts, so there was no intersection with this file's dependency closure.",
      "tests": "All on final commit 41d145d (git rev-parse --short HEAD), builds serialised through the shared verify lock, each VERDICT command-exit 0. REPRODUCIBILITY, the card's own repro inverted: pre-fix two builds of an untouched tree gave 60a0416c and f261b188 (DIFFERENT, defect live on today's tree, already_sorted=false); post-fix two builds gave 84ec138c and 84ec138c (BYTE-IDENTICAL, cmp -s, already_sorted=true). ADVERTISED SET UNCHANGED: sorted-url-blocks digest a57adad516c2dc18914f7902339b42edca65e89dbdcd3dce847ed359b4e2d498 on all four builds, matching the card's a57adad5 exactly; the normalisation was identified rather than guessed (sorted url blocks joined by newline reproduces it), so this is the card's own invariant. Entry count 346, distinct 346, on every build. GATE: node .github/scripts/check-locale-surface.mjs exit 0, prints 'every advertised URL has a source file, and every source file is advertised', table 346/346/346 with 0 unexpected, 0 missing, 0 duplicated. Green for the same reason as before, measured not asserted: I ran the gate against the pre-fix artifact and the post-fix artifact and diffed its output, IDENTICAL on both. That swap mutated an untracked build artifact, so it was confirmed on disk by sha256 before the run (60a0416c, pre-fix) and restored under a trap EXIT INT TERM with the restore verified by sha256 (84ec138c) rather than by an exit code. OTHER GATES: pnpm run type-check exit 0, script name echoed in the output so it was a real run and not a zero-match no-op, tsc --noEmit silent; pnpm turbo run test --force green with 'Cached: 0 cached, 1 total' (the unforced run was a FULL TURBO cache replay, legitimate since the test task's declared inputs exclude app/sitemap.ts, but a replayed green is not a measurement); check-node-floor.mjs --self-test exit 0. translations.yml is path-filtered and this diff matches none of its paths, so it will not run. Exit codes captured by redirecting to a file before reading, never through a tail pipe.",
      "gates": {
        "type-check": "pass",
        "build": "pass",
        "check-locale-surface": "pass",
        "turbo-test-forced": "pass",
        "check-node-floor-self-test": "pass",
        "ci_on_pr": "in_progress"
      },
      "files_changed": ["apps/docs/app/sitemap.ts"],
      "deviations": [
        "Diff is 11 lines, not 1: one statement plus a 10-line comment explaining the irreproducibility and why localeCompare is not used. No restructuring; staticPaths/docPaths composition and the builder are untouched, as adjudicated. Flagged in the PR body for the reviewer to trim if it reads as too much.",
        "Rebased onto origin/main 9a1a7ad after #178 landed as #198 mid-task; all four digests re-measured on the rebased tree. No force-push (fast-forward from the probe commit)."
      ],
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Worktree objectos-issue-194 removed cleanly without --force; sibling worktrees untouched. PR left as draft for your review and merge.


    Generated by Claude Code


    Generated by Claude Code

  8. hotlong commented on Aug 26, 2026

    @hotlong
    ContributorAuthor

    ACCEPT — PM review of record. PR #200 at 41d145d.

    item verdict
    PR shape draft ✓, base main at 9a1a7ad, Fixes #194 ✓
    File surface 1 file, apps/docs/app/sitemap.ts ✓
    CI build (gate step included) and Node floor both completed/success ✓
    Path-surface fork apps/docs/**; no governed surface ✓
    Scope one statement; builder and staticPaths/docPaths composition untouched, as adjudicated ✓

    Rejecting localeCompare is the best call in this PR

    A comparator whose result depends on the machine would reintroduce exactly the irreproducibility this change removes, just across machines instead of across builds — and it would do so invisibly, since any single machine would look perfectly stable.

    The instinctive fix for a sorting problem is to reach for the "more correct" string comparison. Here that is the wrong direction, and wrong in a way that no amount of local testing would reveal: every machine would pass its own two-build check while disagreeing with every other machine. Fixing irreproducibility with a locale-sensitive comparator would have swapped a visible defect for an invisible one.

    Order-invariance was measured, not re-asserted

    The obvious way to show the gate still passes is to run it again and see green. That proves the gate passes on this artifact, not that it is indifferent to order.

    Instead: run the gate against the pre-fix artifact and the post-fix artifact — two files with the same 346 entries in different orders — and diff its stdout. Byte-identical. That measures the property. And since the swap mutated an untracked build artifact, it was confirmed by sha256 before the run and restored under a trap with the restore confirmed by sha256 rather than by an exit code.

    You did not take my correction on trust

    The grading comment struck the card's "the gate will flake" framing. You re-read the landed gate yourself and confirmed order-invariance independently before implementing against the corrected reason. That is the third independent reading of that file today reaching the same conclusion — mine, #173's dev's, and yours — and each one was done rather than inherited.

    You also re-reproduced the defect on today's post-rewrite tree before touching anything (60a0416c vs f261b188, same 346 entries) rather than trusting the card's measurement from before two rewrites of the file.

    The normalisation was identified, not approximated

    sorted url blocks joined by newline reproduces a57adad5… bit for bit, so this is the card's own invariant and not a similar-looking one of mine

    A digest that merely looks stable across your own builds proves nothing about the card's claim. Reproducing the exact recorded value is what makes the four-build table comparable to what came before it.

    The comment stays

    You flagged the 11-line diff as 1 line of code and 10 of comment, and offered to trim it. Keep it. It records why the comparator is code-unit — and the next person to read .sort() here is exactly the person who would otherwise "improve" it into localeCompare. A comment that prevents one plausible wrong edit has paid for itself.

    Notes

    The mid-task base move (a sibling agent's fetch advancing origin/main when #198 landed) was handled correctly: rebased onto 9a1a7ad, all four digests re-measured on the rebased tree, dependency closure checked, fast-forward rather than force-push.

    The channel note about MCP returning comment bodies HTML-entity-encoded is useful and distinct from the sanitizer behaviour recorded on #197 — that one is GitHub decoding on write, this is the read path encoding. Both worth knowing; neither is a defect in this repo.

    Proceeding to ready and merge.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions