Sitemap: operator-seeded shards, listed in the index while their blob exists by feruzm · Pull Request #1591 · ecency/vision-web

feruzm · 2026-08-20T19:36:56Z

What changed

sitemap-shards.ts: OPERATOR_SHARDS (recovery.xml), known to the public shard route but not part of the generated set.
sitemap-generate/route.ts: after the generated shards, each operator shard whose blob exists in Redis is added to the sitemap index with the lastmod stored beside it (or the previously recorded one); the generator never writes the blob and never stamps it with the run's time. Removing the key retires the shard on the next run.
Specs: allowlist test for operator shards; route test that the index lists the shard only while the blob exists and carries the operator's lastmod.

Operation (outside the repo): the recovery list is seeded per origin under seo:sitemap:recovery.xml with seo:sitemap:recovery.xml:lastmod set to the day the blacklist gate was removed; it is deleted once Search Console shows the affected pages recrawled.

… exists A shard an operator writes straight into Redis (blob under the shard key, optional lastmod beside it) is served by the public route and listed in the index by the generator for as long as the key exists; removing the key retires it on the next run. The generator never writes it and never stamps it with its own time. First use: a temporary recovery list of posts that still carry the noindex the removed blacklist gate left in the index, so they get recrawled without waiting for a chance revisit.

qodo-free-for-open-source-projects · 2026-08-20T19:37:03Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Falsy blob existence check~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The operator shard is considered “missing” when its Redis value is an empty string because the
generator checks if (!blob) instead of blob === null. This can incorrectly omit an operator
shard from the index even though the key exists, violating the PR’s “listed while blob exists”
behavior.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R484-486]

+      const blob = await redis.get(K(name));
+      if (!blob) continue;
+      const lastmod = (await redis.get(`${K(name)}:lastmod`)) || recorded[name] || nowDay;

Relevance

●●● Strong
This is a local deterministic null-versus-falsy correctness fix, matching the team’s recent sitemap
correctness fixes.
PR-#1585
PR-#802

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

In ioredis, redis.get() returns string | null; the current if (!blob) check treats "" as
absent. The new operator-shard path relies on this check to decide whether to include the shard in
the sitemap index.

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-488]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The operator-shard existence check uses a falsy check (`if (!blob)`) on the Redis GET result. Redis can store an empty string value, and `GET` returns `""` (falsy) for it, which the current code treats as “missing”.
### Issue Context
This code controls whether an operator shard is included in the generated sitemap index.
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-488]
### Suggested change
- Replace `if (!blob) continue;` with a strict null check: `if (blob === null) continue;`
- If you adopt `EXISTS` (see related finding), this issue is automatically avoided.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Falsy blob existence check~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The operator shard is considered “missing” when its Redis value is an empty string because the
generator checks if (!blob) instead of blob === null. This can incorrectly omit an operator
shard from the index even though the key exists, violating the PR’s “listed while blob exists”
behavior.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R484-486]

+      const blob = await redis.get(K(name));
+      if (!blob) continue;
+      const lastmod = (await redis.get(`${K(name)}:lastmod`)) || recorded[name] || nowDay;

Relevance

●●● Strong
This is a local deterministic null-versus-falsy correctness fix, matching the team’s recent sitemap
correctness fixes.
PR-#1585
PR-#802

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

In ioredis, redis.get() returns string | null; the current if (!blob) check treats "" as
absent. The new operator-shard path relies on this check to decide whether to include the shard in
the sitemap index.

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-488]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The operator-shard existence check uses a falsy check (`if (!blob)`) on the Redis GET result. Redis can store an empty string value, and `GET` returns `""` (falsy) for it, which the current code treats as “missing”.
### Issue Context
This code controls whether an operator shard is included in the generated sitemap index.
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-488]
### Suggested change
- Replace `if (!blob) continue;` with a strict null check: `if (blob === null) continue;`
- If you adopt `EXISTS` (see related finding), this issue is automatically avoided.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Falsy blob existence check~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The operator shard is considered “missing” when its Redis value is an empty string because the
generator checks if (!blob) instead of blob === null. This can incorrectly omit an operator
shard from the index even though the key exists, violating the PR’s “listed while blob exists”
behavior.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R484-486]

+      const blob = await redis.get(K(name));
+      if (!blob) continue;
+      const lastmod = (await redis.get(`${K(name)}:lastmod`)) || recorded[name] || nowDay;

Relevance

●●● Strong
This is a local deterministic null-versus-falsy correctness fix, matching the team’s recent sitemap
correctness fixes.
PR-#1585
PR-#802

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

In ioredis, redis.get() returns string | null; the current if (!blob) check treats "" as
absent. The new operator-shard path relies on this check to decide whether to include the shard in
the sitemap index.

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-488]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The operator-shard existence check uses a falsy check (`if (!blob)`) on the Redis GET result. Redis can store an empty string value, and `GET` returns `""` (falsy) for it, which the current code treats as “missing”.
### Issue Context
This code controls whether an operator shard is included in the generated sitemap index.
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-488]
### Suggested change
- Replace `if (!blob) continue;` with a strict null check: `if (blob === null) continue;`
- If you adopt `EXISTS` (see related finding), this issue is automatically avoided.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View high (5)

4. ~~Falsy blob existence check~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The operator shard is considered “missing” when its Redis value is an empty string because the
generator checks if (!blob) instead of blob === null. This can incorrectly omit an operator
shard from the index even though the key exists, violating the PR’s “listed while blob exists”
behavior.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R484-486]

+      const blob = await redis.get(K(name));
+      if (!blob) continue;
+      const lastmod = (await redis.get(`${K(name)}:lastmod`)) || recorded[name] || nowDay;

Relevance

●●● Strong
This is a local deterministic null-versus-falsy correctness fix, matching the team’s recent sitemap
correctness fixes.
PR-#1585
PR-#802

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

In ioredis, redis.get() returns string | null; the current if (!blob) check treats "" as
absent. The new operator-shard path relies on this check to decide whether to include the shard in
the sitemap index.

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-488]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The operator-shard existence check uses a falsy check (`if (!blob)`) on the Redis GET result. Redis can store an empty string value, and `GET` returns `""` (falsy) for it, which the current code treats as “missing”.
### Issue Context
This code controls whether an operator shard is included in the generated sitemap index.
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-488]
### Suggested change
- Replace `if (!blob) continue;` with a strict null check: `if (blob === null) continue;`
- If you adopt `EXISTS` (see related finding), this issue is automatically avoided.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Loads blob to test existence~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

The generator performs redis.get() of the full operator shard XML just to check whether the key
exists, which is unnecessary and can time out or increase memory/latency when the recovery shard is
large. A slow/failed read here fails the whole generation run (returns 500), preventing the normal
shards/index from being refreshed.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R483-485]
+    for (const name of OPERATOR_SHARDS) {
+      const blob = await redis.get(K(name));
+      if (!blob) continue;

Relevance

●● Moderate
Performance concern is plausible, but no close precedent establishes replacing Redis GET for this
operator-shard path.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The operator shard existence check currently does a full GET of the shard blob, even though it only
needs presence. The shared ioredis client is configured with commandTimeout: 1000, so large reads
are more likely to time out and abort the whole write block.
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-489]
apps/web/src/features/seo/seo-redis.ts[27-36]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The generator calls `redis.get(K(name))` for operator shards only to decide whether the blob exists. This fetches the entire XML payload, which can be large, slow, and unnecessary.
### Issue Context
`getSeoRedis()` configures ioredis with a 1000ms `commandTimeout`, so large GET payloads are more likely to exceed the timeout and throw, causing the generator to return 500.
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-489]
- apps/web/src/features/seo/seo-redis.ts[27-36]
### Suggested change
- Replace the GET-based existence check with `EXISTS`:
- `const exists = await redis.exists(K(name)); if (!exists) continue;`
- Then fetch only the small `:lastmod` key (and avoid fetching the blob altogether).
- Optional: pipeline `EXISTS` / `GET :lastmod` for all `OPERATOR_SHARDS` to reduce RTTs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. ~~Loads blob to test existence~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

The generator performs redis.get() of the full operator shard XML just to check whether the key
exists, which is unnecessary and can time out or increase memory/latency when the recovery shard is
large. A slow/failed read here fails the whole generation run (returns 500), preventing the normal
shards/index from being refreshed.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R483-485]
+    for (const name of OPERATOR_SHARDS) {
+      const blob = await redis.get(K(name));
+      if (!blob) continue;

Relevance

●● Moderate
Performance concern is plausible, but no close precedent establishes replacing Redis GET for this
operator-shard path.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The operator shard existence check currently does a full GET of the shard blob, even though it only
needs presence. The shared ioredis client is configured with commandTimeout: 1000, so large reads
are more likely to time out and abort the whole write block.
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-489]
apps/web/src/features/seo/seo-redis.ts[27-36]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The generator calls `redis.get(K(name))` for operator shards only to decide whether the blob exists. This fetches the entire XML payload, which can be large, slow, and unnecessary.
### Issue Context
`getSeoRedis()` configures ioredis with a 1000ms `commandTimeout`, so large GET payloads are more likely to exceed the timeout and throw, causing the generator to return 500.
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-489]
- apps/web/src/features/seo/seo-redis.ts[27-36]
### Suggested change
- Replace the GET-based existence check with `EXISTS`:
- `const exists = await redis.exists(K(name)); if (!exists) continue;`
- Then fetch only the small `:lastmod` key (and avoid fetching the blob altogether).
- Optional: pipeline `EXISTS` / `GET :lastmod` for all `OPERATOR_SHARDS` to reduce RTTs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

7. ~~Loads blob to test existence~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

The generator performs redis.get() of the full operator shard XML just to check whether the key
exists, which is unnecessary and can time out or increase memory/latency when the recovery shard is
large. A slow/failed read here fails the whole generation run (returns 500), preventing the normal
shards/index from being refreshed.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R483-485]
+    for (const name of OPERATOR_SHARDS) {
+      const blob = await redis.get(K(name));
+      if (!blob) continue;

Relevance

●● Moderate
Performance concern is plausible, but no close precedent establishes replacing Redis GET for this
operator-shard path.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The operator shard existence check currently does a full GET of the shard blob, even though it only
needs presence. The shared ioredis client is configured with commandTimeout: 1000, so large reads
are more likely to time out and abort the whole write block.
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-489]
apps/web/src/features/seo/seo-redis.ts[27-36]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The generator calls `redis.get(K(name))` for operator shards only to decide whether the blob exists. This fetches the entire XML payload, which can be large, slow, and unnecessary.
### Issue Context
`getSeoRedis()` configures ioredis with a 1000ms `commandTimeout`, so large GET payloads are more likely to exceed the timeout and throw, causing the generator to return 500.
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-489]
- apps/web/src/features/seo/seo-redis.ts[27-36]
### Suggested change
- Replace the GET-based existence check with `EXISTS`:
- `const exists = await redis.exists(K(name)); if (!exists) continue;`
- Then fetch only the small `:lastmod` key (and avoid fetching the blob altogether).
- Optional: pipeline `EXISTS` / `GET :lastmod` for all `OPERATOR_SHARDS` to reduce RTTs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

8. ~~Loads blob to test existence~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

The generator performs redis.get() of the full operator shard XML just to check whether the key
exists, which is unnecessary and can time out or increase memory/latency when the recovery shard is
large. A slow/failed read here fails the whole generation run (returns 500), preventing the normal
shards/index from being refreshed.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R483-485]
+    for (const name of OPERATOR_SHARDS) {
+      const blob = await redis.get(K(name));
+      if (!blob) continue;

Relevance

●● Moderate
Performance concern is plausible, but no close precedent establishes replacing Redis GET for this
operator-shard path.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The operator shard existence check currently does a full GET of the shard blob, even though it only
needs presence. The shared ioredis client is configured with commandTimeout: 1000, so large reads
are more likely to time out and abort the whole write block.
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-489]
apps/web/src/features/seo/seo-redis.ts[27-36]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The generator calls `redis.get(K(name))` for operator shards only to decide whether the blob exists. This fetches the entire XML payload, which can be large, slow, and unnecessary.
### Issue Context
`getSeoRedis()` configures ioredis with a 1000ms `commandTimeout`, so large GET payloads are more likely to exceed the timeout and throw, causing the generator to return 500.
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-489]
- apps/web/src/features/seo/seo-redis.ts[27-36]
### Suggested change
- Replace the GET-based existence check with `EXISTS`:
- `const exists = await redis.exists(K(name)); if (!exists) continue;`
- Then fetch only the small `:lastmod` key (and avoid fetching the blob altogether).
- Optional: pipeline `EXISTS` / `GET :lastmod` for all `OPERATOR_SHARDS` to reduce RTTs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

9. ~~Misleading lockstep comment~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The generator comment claims “every child the index points at comes from … SITEMAP_SHARDS”, but this
PR intentionally adds operator shards to the index that are not in SITEMAP_SHARDS. This mismatch
can mislead future edits into reintroducing 404/allowlist drift or incorrect typing assumptions.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R475-482]

+    // Operator-seeded shards ride along in the index while their blob
+    // exists; their lastmod is whatever the operator set beside the blob (or
+    // the previously recorded one), never this run's time: nothing here
+    // changed them.
+    const children: { name: string; lastmod: string }[] = SITEMAP_SHARDS.map((name) => ({
+      name,
+      lastmod: lastmods[name]
+    }));

Relevance

●●● Strong
Recent sitemap history accepts comments clarifying behavioral asymmetries and preventing future
drift.
PR-#803
PR-#1585

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The comment asserts the index only references SITEMAP_SHARDS, but the PR adds logic to append
OPERATOR_SHARDS to children before writing the index.
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[428-432]
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[475-489]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A nearby comment in the generator states that all index children come from `SITEMAP_SHARDS`, but the new operator-shard behavior explicitly appends shards outside that set.
### Issue Context
The code now builds `children` from `SITEMAP_SHARDS` and then appends `OPERATOR_SHARDS` (when present in Redis).
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[428-432]
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[475-489]
### Suggested change
- Amend the comment to reflect the new contract, e.g.:
- Generated shards come from `SITEMAP_SHARDS`.
- Index may additionally include `OPERATOR_SHARDS` when present.
- Public route allowlist covers both.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

10. ~~Misleading lockstep comment~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The generator comment claims “every child the index points at comes from … SITEMAP_SHARDS”, but this
PR intentionally adds operator shards to the index that are not in SITEMAP_SHARDS. This mismatch
can mislead future edits into reintroducing 404/allowlist drift or incorrect typing assumptions.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R475-482]

+    // Operator-seeded shards ride along in the index while their blob
+    // exists; their lastmod is whatever the operator set beside the blob (or
+    // the previously recorded one), never this run's time: nothing here
+    // changed them.
+    const children: { name: string; lastmod: string }[] = SITEMAP_SHARDS.map((name) => ({
+      name,
+      lastmod: lastmods[name]
+    }));

Relevance

●●● Strong
Recent sitemap history accepts comments clarifying behavioral asymmetries and preventing future
drift.
PR-#803
PR-#1585

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The comment asserts the index only references SITEMAP_SHARDS, but the PR adds logic to append
OPERATOR_SHARDS to children before writing the index.
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[428-432]
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[475-489]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A nearby comment in the generator states that all index children come from `SITEMAP_SHARDS`, but the new operator-shard behavior explicitly appends shards outside that set.
### Issue Context
The code now builds `children` from `SITEMAP_SHARDS` and then appends `OPERATOR_SHARDS` (when present in Redis).
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[428-432]
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[475-489]
### Suggested change
- Amend the comment to reflect the new contract, e.g.:
- Generated shards come from `SITEMAP_SHARDS`.
- Index may additionally include `OPERATOR_SHARDS` when present.
- Public route allowlist covers both.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

11. ~~Misleading lockstep comment~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The generator comment claims “every child the index points at comes from … SITEMAP_SHARDS”, but this
PR intentionally adds operator shards to the index that are not in SITEMAP_SHARDS. This mismatch
can mislead future edits into reintroducing 404/allowlist drift or incorrect typing assumptions.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R475-482]

+    // Operator-seeded shards ride along in the index while their blob
+    // exists; their lastmod is whatever the operator set beside the blob (or
+    // the previously recorded one), never this run's time: nothing here
+    // changed them.
+    const children: { name: string; lastmod: string }[] = SITEMAP_SHARDS.map((name) => ({
+      name,
+      lastmod: lastmods[name]
+    }));

Relevance

●●● Strong
Recent sitemap history accepts comments clarifying behavioral asymmetries and preventing future
drift.
PR-#803
PR-#1585

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The comment asserts the index only references SITEMAP_SHARDS, but the PR adds logic to append
OPERATOR_SHARDS to children before writing the index.
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[428-432]
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[475-489]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A nearby comment in the generator states that all index children come from `SITEMAP_SHARDS`, but the new operator-shard behavior explicitly appends shards outside that set.
### Issue Context
The code now builds `children` from `SITEMAP_SHARDS` and then appends `OPERATOR_SHARDS` (when present in Redis).
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[428-432]
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[475-489]
### Suggested change
- Amend the comment to reflect the new contract, e.g.:
- Generated shards come from `SITEMAP_SHARDS`.
- Index may additionally include `OPERATOR_SHARDS` when present.
- Public route allowlist covers both.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View low (1)

12. ~~Misleading lockstep comment~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The generator comment claims “every child the index points at comes from … SITEMAP_SHARDS”, but this
PR intentionally adds operator shards to the index that are not in SITEMAP_SHARDS. This mismatch
can mislead future edits into reintroducing 404/allowlist drift or incorrect typing assumptions.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R475-482]

+    // Operator-seeded shards ride along in the index while their blob
+    // exists; their lastmod is whatever the operator set beside the blob (or
+    // the previously recorded one), never this run's time: nothing here
+    // changed them.
+    const children: { name: string; lastmod: string }[] = SITEMAP_SHARDS.map((name) => ({
+      name,
+      lastmod: lastmods[name]
+    }));

Relevance

●●● Strong
Recent sitemap history accepts comments clarifying behavioral asymmetries and preventing future
drift.
PR-#803
PR-#1585

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The comment asserts the index only references SITEMAP_SHARDS, but the PR adds logic to append
OPERATOR_SHARDS to children before writing the index.
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[428-432]
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[475-489]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A nearby comment in the generator states that all index children come from `SITEMAP_SHARDS`, but the new operator-shard behavior explicitly appends shards outside that set.
### Issue Context
The code now builds `children` from `SITEMAP_SHARDS` and then appends `OPERATOR_SHARDS` (when present in Redis).
### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[428-432]
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[475-489]
### Suggested change
- Amend the comment to reflect the new contract, e.g.:
- Generated shards come from `SITEMAP_SHARDS`.
- Index may additionally include `OPERATOR_SHARDS` when present.
- Public route allowlist covers both.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

coderabbitai · 2026-08-20T19:37:06Z

Warning

Review limit reached

@feruzm, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 22 minutes

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 890f9360-9441-417d-b537-cb6ee4acb225

📥 Commits

Reviewing files that changed from the base of the PR and between 20f90b6 and 5783ffa.

📒 Files selected for processing (6)

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts
apps/web/src/app/sitemap/[shard]/route.ts
apps/web/src/features/seo/sitemap-shards.ts
apps/web/src/specs/api/sitemap-generate-route.spec.ts
apps/web/src/specs/api/sitemap-shard-route.spec.ts
apps/web/src/specs/features/seo/sitemap-shards.spec.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

qodo-free-for-open-source-projects · 2026-08-20T19:38:11Z

PR Summary by Qodo

Sitemap: include operator-seeded shards in index while Redis blob exists

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Add allowlisted operator shards that are served publicly but not generated.
• Extend sitemap generator to index operator shards only when their Redis blob exists.
• Add specs verifying shard allowlisting and index/lastmod behavior.

Diagram

graph TD
  G["Sitemap generator route"] --> M["sitemap-shards module"] --> P["Public sitemap route"]
  G --> R[("SEO Redis")]
  P --> R
  G --> I["Sitemap index XML"]
  O["Operator shard blob"] --> R
  T["Vitest specs"] --> G

  subgraph Legend
    direction LR
    _svc["Route/Service"] ~~~ _mod["Module"] ~~~ _db[("Database/Cache")]
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Redis-driven operator shard registry (set/list)

➕ No code change needed to add/remove operator shards
➕ Can support multiple temporary shards per incident
➖ Higher risk of accidentally exposing/serving unexpected shard names
➖ Requires additional validation and operational discipline

2. Unify shards into a typed config with generation mode

➕ Single source of truth for shard metadata (generated vs operator, lastmod semantics)
➕ Scales cleanly if more special-case shards appear
➖ More refactor than needed for a single recovery shard
➖ Adds abstraction overhead to a simple allowlist

Recommendation: Current approach (explicit OPERATOR_SHARDS allowlist + presence check in Redis) is a good safety-first design for a public endpoint: it keeps exposure tightly controlled and makes retirement as simple as deleting the blob key. If operator shards become a recurring tool, consider moving to a unified shard config or a validated Redis registry to avoid repeated code edits.

Files changed (4) +51 / -7

Enhancement (2) +28 / -7

route.tsIndex operator-seeded shards when their Redis blob exists +17/-5

Index operator-seeded shards when their Redis blob exists
• Adds OPERATOR_SHARDS to the sitemap index output when a corresponding shard blob key is present in Redis. Uses an operator-provided :lastmod (or previously recorded value) and explicitly avoids stamping operator shards with the generator run time.
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts

sitemap-shards.tsAdd OPERATOR_SHARDS allowlist and include in isKnownShard +11/-2

Add OPERATOR_SHARDS allowlist and include in isKnownShard

• Introduces an explicit OPERATOR_SHARDS constant (currently recovery.xml) with documentation for operational use. Expands the SitemapShard union and the known-shard set so public routes recognize operator shards without treating them as generated shards.

apps/web/src/features/seo/sitemap-shards.ts

Tests (2) +23 / -0

sitemap-generate-route.spec.tsTest operator shard indexing is conditional and preserves operator lastmod +13/-0

Test operator shard indexing is conditional and preserves operator lastmod
• Adds a route spec asserting that recovery.xml is only listed in the index while its Redis blob exists. Verifies the index lastmod comes from the operator key and the generator does not overwrite the shard blob.
apps/web/src/specs/api/sitemap-generate-route.spec.ts

sitemap-shards.spec.tsTest operator shards are known but not part of generated shard list +10/-0

Test operator shards are known but not part of generated shard list
• Adds a focused spec to ensure OPERATOR_SHARDS entries pass isKnownShard while remaining excluded from SITEMAP_SHARDS. Guards the intended separation between generated and operator-seeded shards.
apps/web/src/specs/features/seo/sitemap-shards.spec.ts

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5fc86c1c77

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-20T19:38:52Z

+export type SitemapShard = (typeof SITEMAP_SHARDS)[number] | (typeof OPERATOR_SHARDS)[number];

-const SHARD_SET: ReadonlySet<string> = new Set(SITEMAP_SHARDS);
+const SHARD_SET: ReadonlySet<string> = new Set<string>([...SITEMAP_SHARDS, ...OPERATOR_SHARDS]);


Stop treating deleted operator shards as permanently live

When the operator deletes seo:sitemap:recovery.xml, the next generation removes it from the index, but this permanent inclusion in SHARD_SET means the public shard route still considers it known; its missing Redis blob therefore takes the unavailable() path in app/sitemap/[shard]/route.ts and returns 503 forever rather than 404. Crawlers that retained the old URL will consequently keep retrying a shard that was meant to be retired, so operator shards need a way to stop being treated as live after removal.

Useful? React with 👍 / 👎.

Good catch. Fixed in 253d252: isOperatorShard() and the shard route now answers 404 (no Retry-After) for an operator shard whose blob is gone, while a generated shard that is merely not in Redis yet keeps the transient 503. The generator drops the entry from the index on its next run; a crawler holding the old index gets a clean "gone" in the meantime. New shard-route spec covers the three states.

greptile-apps · 2026-08-20T19:39:26Z

Greptile Summary

This PR adds support for temporary, operator-seeded sitemap shards whose Redis blob controls whether they appear in the sitemap index and remain publicly routable.

Adds recovery.xml as an allowlisted operator shard outside the generated shard set.
Includes operator shards in the generated index only while their Redis blobs are non-empty, preserving operator-supplied lastmod values.
Returns 404 for absent or retired operator shards while retaining retryable behavior for missing generated shards.
Adds generator, public-route, and shard-allowlist coverage for the new lifecycle.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts	Extends index generation to include non-empty operator-managed shards while leaving their blobs and timestamps under operator control.
apps/web/src/app/sitemap/[shard]/route.ts	Distinguishes retired operator shards from temporarily unavailable generated shards by returning 404 instead of 503.
apps/web/src/features/seo/sitemap-shards.ts	Introduces the operator-shard allowlist and incorporates it into public shard validation.
apps/web/src/specs/api/sitemap-generate-route.spec.ts	Covers operator-shard inclusion, supplied lastmod preservation, and removal for empty or deleted blobs.
apps/web/src/specs/api/sitemap-shard-route.spec.ts	Covers generated, unknown, seeded operator, and retired operator shard response semantics.
apps/web/src/specs/features/seo/sitemap-shards.spec.ts	Verifies operator shards are publicly known without entering the generated shard set.

Sequence Diagram

sequenceDiagram
  participant O as Operator
  participant R as Redis
  participant G as Sitemap generator
  participant I as Sitemap index
  participant C as Crawler
  O->>R: Seed recovery.xml and optional lastmod
  G->>R: STRLEN recovery.xml
  alt Blob is non-empty
    R-->>G: Positive length
    G->>R: Read operator lastmod
    G->>I: Include recovery.xml
    C->>R: GET /sitemap/recovery.xml
    R-->>C: 200 XML
  else Blob is absent or empty
    R-->>G: Zero length
    G->>I: Omit recovery.xml
    C->>R: GET /sitemap/recovery.xml
    R-->>C: 404 Not Found
  end

_{Reviews (4): Last reviewed commit: "Merge develop and make index and route a..." | Re-trigger Greptile}

An operator shard exists only through its blob, so a missing blob means the operator retired it, not that it is pending. Returning the transient 503 there would keep crawlers retrying a shard that was meant to be gone.

qodo-code-review · 2026-08-20T19:41:58Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. ~~Falsy blob existence check~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The operator shard is considered “missing” when its Redis value is an empty string because the
generator checks if (!blob) instead of blob === null. This can incorrectly omit an operator
shard from the index even though the key exists, violating the PR’s “listed while blob exists”
behavior.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R484-486]

+      const blob = await redis.get(K(name));
+      if (!blob) continue;
+      const lastmod = (await redis.get(`${K(name)}:lastmod`)) || recorded[name] || nowDay;

Relevance

●●● Strong
This is a local deterministic null-versus-falsy correctness fix, matching the team’s recent sitemap
correctness fixes.
PR-#1585
PR-#802

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

In ioredis, redis.get() returns string | null; the current if (!blob) check treats "" as
absent. The new operator-shard path relies on this check to decide whether to include the shard in
the sitemap index.

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-488]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The operator-shard existence check uses a falsy check (`if (!blob)`) on the Redis GET result. Redis can store an empty string value, and `GET` returns `""` (falsy) for it, which the current code treats as “missing”.

### Issue Context
This code controls whether an operator shard is included in the generated sitemap index.

### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-488]

### Suggested change
- Replace `if (!blob) continue;` with a strict null check: `if (blob === null) continue;`
- If you adopt `EXISTS` (see related finding), this issue is automatically avoided.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Loads blob to test existence~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

The generator performs redis.get() of the full operator shard XML just to check whether the key
exists, which is unnecessary and can time out or increase memory/latency when the recovery shard is
large. A slow/failed read here fails the whole generation run (returns 500), preventing the normal
shards/index from being refreshed.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R483-485]
+    for (const name of OPERATOR_SHARDS) {
+      const blob = await redis.get(K(name));
+      if (!blob) continue;

Relevance

●● Moderate
Performance concern is plausible, but no close precedent establishes replacing Redis GET for this
operator-shard path.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The operator shard existence check currently does a full GET of the shard blob, even though it only
needs presence. The shared ioredis client is configured with commandTimeout: 1000, so large reads
are more likely to time out and abort the whole write block.
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-489]
apps/web/src/features/seo/seo-redis.ts[27-36]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The generator calls `redis.get(K(name))` for operator shards only to decide whether the blob exists. This fetches the entire XML payload, which can be large, slow, and unnecessary.

### Issue Context
`getSeoRedis()` configures ioredis with a 1000ms `commandTimeout`, so large GET payloads are more likely to exceed the timeout and throw, causing the generator to return 500.

### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[483-489]
- apps/web/src/features/seo/seo-redis.ts[27-36]

### Suggested change
- Replace the GET-based existence check with `EXISTS`:
 - `const exists = await redis.exists(K(name)); if (!exists) continue;`
- Then fetch only the small `:lastmod` key (and avoid fetching the blob altogether).
- Optional: pipeline `EXISTS` / `GET :lastmod` for all `OPERATOR_SHARDS` to reduce RTTs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Misleading lockstep comment~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The generator comment claims “every child the index points at comes from … SITEMAP_SHARDS”, but this
PR intentionally adds operator shards to the index that are not in SITEMAP_SHARDS. This mismatch
can mislead future edits into reintroducing 404/allowlist drift or incorrect typing assumptions.

Code

apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[R475-482]

+    // Operator-seeded shards ride along in the index while their blob
+    // exists; their lastmod is whatever the operator set beside the blob (or
+    // the previously recorded one), never this run's time: nothing here
+    // changed them.
+    const children: { name: string; lastmod: string }[] = SITEMAP_SHARDS.map((name) => ({
+      name,
+      lastmod: lastmods[name]
+    }));

Relevance

●●● Strong
Recent sitemap history accepts comments clarifying behavioral asymmetries and preventing future
drift.
PR-#803
PR-#1585

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The comment asserts the index only references SITEMAP_SHARDS, but the PR adds logic to append
OPERATOR_SHARDS to children before writing the index.
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[428-432]
apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[475-489]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
A nearby comment in the generator states that all index children come from `SITEMAP_SHARDS`, but the new operator-shard behavior explicitly appends shards outside that set.

### Issue Context
The code now builds `children` from `SITEMAP_SHARDS` and then appends `OPERATOR_SHARDS` (when present in Redis).

### Fix Focus Areas
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[428-432]
- apps/web/src/app/api/internal/seo/sitemap-generate/route.ts[475-489]

### Suggested change
- Amend the comment to reflect the new contract, e.g.:
 - Generated shards come from `SITEMAP_SHARDS`.
 - Index may additionally include `OPERATOR_SHARDS` when present.
 - Public route allowlist covers both.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

✅ Compliance rules (platform): 82 rules

✅ Skills: 6 invoked

add-feature

add-query

add-sdk-mutation

add-test

code-review

debug

Review mode: ⚖️ Balanced: This changes runtime sitemap generation and Redis-backed public indexing behavior; it is localized but has meaningful state and route-contract implications best handled by a careful single-pass review.

Tip of the day

💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

…s present only with a non-empty blob Resolves the accessor conflict with the merged Redis warm-up change: the shard route keeps getSeoRedisReady() and gains isOperatorShard(). The generator now decides presence with STRLEN, the same rule the public route applies, so the index never advertises a shard URL that answers 404.

chatgpt-codex-connector Bot reviewed Aug 20, 2026

View reviewed changes

Answer 404 for an operator shard whose blob was removed

253d252

An operator shard exists only through its blob, so a missing blob means the operator retired it, not that it is pending. Returning the transient 503 there would keep crawlers retrying a shard that was meant to be gone.

qodo-code-review Bot reviewed Aug 20, 2026

View reviewed changes

Comment thread apps/web/src/app/api/internal/seo/sitemap-generate/route.ts Outdated

Comment thread apps/web/src/app/api/internal/seo/sitemap-generate/route.ts Outdated

Comment thread apps/web/src/app/api/internal/seo/sitemap-generate/route.ts

feruzm added 3 commits August 20, 2026 19:44

Test an operator shard's presence with EXISTS, not by loading the blob

10105ad

Mock EXISTS in the sitemap route specs and pin key-based presence

3fae596

feruzm merged commit a03fdb9 into develop Aug 20, 2026
9 checks passed

feruzm deleted the feat/sitemap-recovery-shard branch August 20, 2026 19:58

qodo-code-review Bot mentioned this pull request Aug 21, 2026

SSR RPC proxy: report rpcProxyStats from the web tier #1623

Merged

Navigation Menu

Uh oh!

Sitemap: operator-seeded shards, listed in the index while their blob exists - #1591

Sitemap: operator-seeded shards, listed in the index while their blob exists#1591
feruzm merged 5 commits into
developfrom
feat/sitemap-recovery-shard

feruzm commented Aug 20, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 20, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 20, 2026 •

edited

Loading

Review limit reached

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 20, 2026 •

edited by qodo-code-review Bot

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Aug 20, 2026

Uh oh!

feruzm Aug 20, 2026

Uh oh!

greptile-apps Bot commented Aug 20, 2026 •

edited

Loading

Important Files Changed

Sequence Diagram

Uh oh!

qodo-code-review Bot commented Aug 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

feruzm commented Aug 20, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

coderabbitai Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 20, 2026 • edited by qodo-code-review Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary by Qodo

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

feruzm Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

qodo-code-review Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qodo-free-for-open-source-projects Bot commented Aug 20, 2026 •

edited

Loading

coderabbitai Bot commented Aug 20, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Aug 20, 2026 •

edited by qodo-code-review Bot

Loading

greptile-apps Bot commented Aug 20, 2026 •

edited

Loading

qodo-code-review Bot commented Aug 20, 2026 •

edited

Loading