Repository navigation
feat(sitemap): add accurate lastmod from each doc's last content change - #928
amaan-bhati wants to merge 8 commits into
Conversation
Claude Review Skill: Iteration 1Verdict: 🔄 REQUEST CHANGES 1 blocking, 1 important, 4 nits. Reviewed at 🔴 Blocking
🟡 Important
🟢 Nits (non blocking)
✅ Verified
📚 Notes
|
Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
|
Addressed in Blocking
Important
Nits
Notes
|
Claude Review Skill: Iteration 2Verdict: 🔄 REQUEST CHANGES (one blocker left, everything else resolved) Reviewed at 🔴 Blocking
✅ Resolved since iteration 1
✅ Verified
Next
|
|
Acknowledged. Only DCO is left: signing off |
dhananjay6561
left a comment
There was a problem hiding this comment.
Reviewed by checking out bab6cc0d with full history and running withContentLastUpdate (NODE_ENV=production) over every built doc: 397 files across docs/, version-1.0.0, version-2.0.0 and version-4.0.0. That takes about 6s, so build cost isn't a concern. Copy/rename following works: none of the 209 v4 pages land on the v4 cut date (8a7d8f06, 2026-01-21).
Verdict: not mergeable as-is. The mechanism is sound, but the dates it ships aren't accurate yet. That undercuts the PR's own reason for existing: "Dates that move without real changes get ignored for the whole sitemap."
1. #897 is a sitewide SEO sweep, not a content change, and it now dates 41% of the sitemap.
- Of the 209 v4 pages (the only indexable version), 86 resolve to 2026-09-01 (
80307a58, #897), and 192 of 209 (92%) are dated 2026-09-01 or later. - The "2023-10 to 2026-10" spread in the description comes mostly from 1.0.0/2.0.0. Those are
noIndexand excluded byignorePatterns, so they never reach the sitemap. - Details and numbers are on
.lastmod-ignore-revs.
2. Collapsing all whitespace hides rendering changes. Markdown treats whitespace as meaningful. See the inline comment on L106.
3. The failure mode is silent. A shallow checkout only produces a console.warn, and the build ships the exact single-date bug this PR fixes. The repo already gates build output with verify-schema and verify-linkedin-tag, and nothing gates this. See the inline comment on L184.
4. DCO is red. 0cf7bf11 (the main feature commit) has no Signed-off-by.
Recommendation: fix #1 first, since it decides whether the sitemap is trustworthy on day one. Ignoring #897 per commit is too coarse: this repo squash-merges (259 non-merge vs 8 merge commits since 2025-01), so #897 fuses SEO boilerplate with a few real edits. Use an ignore rule that can strip known boilerplate blocks (## Related / ## Related Terms footers, <HowTo visible={false}>) from the compared body, or accept ignoring #897 entirely and lose a handful of small real edits. Then make CI fail on a shallow clone and add a verify-lastmod check alongside verify-schema.
Repro for every number here: run withContentLastUpdate over the file list above and group the resulting last_update.date by commit.
…sweep #897 Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
…uilds Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
|
@dhananjay6561 thanks, all four addressed in
The PR description now shows numbers for v4 only. |
Claude Review Skill: Iteration 3Verdict: 🔄 REQUEST CHANGES (DCO only, the code is approve ready) Reviewed at 🔴 Blocking
✅ Verified in
|
…rewrites Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
|
Nits
DCO: the author will sign off and force push this separately. |
Claude Review Skill: Iteration 4Verdict: ✅ APPROVE Reviewed at ✅ Resolved since iteration 3
✅ Final state
Nothing blocking. Good to merge. |
dhananjay6561
left a comment
There was a problem hiding this comment.
Full re-review at 21abd927. I re-ran withContentLastUpdate over all 397 built docs on a full clone (5.0s, 0 nulls) and checked each fix against real files.
Fixed and verified:
- #897: 0 v4 pages land on 2026-09-01 now. v4 has 46 distinct dates. The largest groups are #918 (72) and #790 (21), and both are real rewrites.
- Whitespace: indentation and single blank lines now count as changes.
\uFEFFis an escape now, not a raw byte. - Shallow clone: throws when
GITHUB_ACTIONS=true. The only workflows that run a build (main.yml,build_and_check.yml) both usefetch-depth: 0. verify-lastmod: runs indeploy-previewand passes.
Not mergeable yet. The new boilerplate stripping hides real content. Two inline comments:
- L117: the
## Relatedregex removes everything up to the next h1/h2 or the end of the file, not only the link list. I edited real files and passed them through the PR's owncontentOf():- Rewriting a FAQ answer in
glossary/integration-testing.md: not detected. The whole### FAQssection (6 Q&As, 36 lines) sits under## Related Terms. - Editing the closing paragraph of
api-testing-adding-labels.md: not detected. The same pattern exists inapi-testing-running-selective.mdandapi-testing-suite-settings.md. \balso matches hand-written sections that #897 never added:## Related Features/guides/pages/workflowson 8 pages, from #790, #809, #841 and #886.
- Rewriting a FAQ answer in
- L115:
<HowTo>is visible by default (src/components/HowTo.js:38,visible = true), but the regex strips every HowTo block. Today all 44 blocks arevisible={false}, so dates are unaffected. The first visible HowTo someone adds, which is the component's default, will have its step edits ignored.
Remaining debt (not blocking, flagging it so it's a deliberate choice):
MAX_SHARE_OF_ONE_DATE = 0.5would not have caught #897 (41%). That's a reasonable trade-off, and the error text now explains it, but the check only catches a total failure, not a sweep.- DCO is green through a manual override on the app, and
0cf7bf11still has noSigned-off-by. Squash-merge this PR, or the unsigned commit lands onmain.
Recommendation: narrow both regexes as described inline, re-run the 397-doc pass, then this is ready to merge.
dhananjay6561
left a comment
There was a problem hiding this comment.
Full re-review, still at 21abd927 (no new commits since my last review).
Newly verified this pass: both shallow-clone safety nets work end to end. I did a real npm run build on a --depth 1 clone of this head:
| Scenario | Result |
|---|---|
GITHUB_ACTIONS=true npm run build |
exit 1: [contentLastUpdate] shallow clone on CI: check out with fetch-depth: 0, … is raised from isEnabled (L200) and stops loadSite |
Same clone, no GITHUB_ACTIONS |
build exits 0 with the warning, then npm run verify-lastmod gives exit 1: only 1 distinct <lastmod> dates; is git history shallow? |
So neither a dropped fetch-depth: 0 in main.yml nor one in build_and_check.yml can ship single-date sitemaps silently.
Walk logic re-checked: neither ignored commit (#896, #897) creates, renames or copies any doc, so the ignore-on-copy branch (L170-173) never fires today. Copy-following through the v4 cut still holds: 0 v4 pages land on 8a7d8f06.
Still open, no reply yet:
- L117 (blocking): the
## Relatedstrip removes everything after the link list down to the next h1/h2. FAQ edits inglossary/integration-testing.mdand closing-paragraph edits on 3running-keploypages are not detected. Repro and a tested fix are in the inline thread. - L115:
<HowTo>renders visibly by default, but the regex strips all of them. No effect today; it becomes a gap with the first visible HowTo.
Once L117 is narrowed and the 397-doc pass is re-run, I have nothing else blocking.
…rom content Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
|
@dhananjay6561 both inline points fixed in On the remaining debt:
|
dhananjay6561
left a comment
There was a problem hiding this comment.
Re-review at ca10cce6. I validated both fixes myself.
Verified:
| Check | Result |
|---|---|
FAQ answer edit, glossary/integration-testing.md |
detected ✅ |
Closing paragraph edit, api-testing-adding-labels.md |
detected ✅ |
Link edit in hand-written ## Related Features (#790) |
detected ✅ |
Link edit inside a #897 ## Related Terms list (glossary/mocks.md) |
ignored ✅ (intended) |
Step edit in a hidden HowTo (keploy-explained/windows-wsl.md) |
ignored ✅ |
Same file with the block flipped to visible={true}, step edited |
detected ✅ |
v4 pages that still have a bare ## Related / ## Related Terms heading after stripping |
0 ✅ |
397-doc pass vs 21abd927 |
0 dates moved, 0 nulls; v4: 46 distinct dates, 0 on 2026-09-01, largest 2026-09-30 x72 ✅ |
Moving the trailing-whitespace strip ahead of the heading match is right: it lets ## Related match, and lets the blank-line branch use a plain \n.
I accept the 50% cap and the DCO plan as the author explained them. Squash-merge so 0cf7bf11 doesn't land on main unsigned.
One non-blocking nit inline (L118): a single-line visible <HowTo … /> followed by a hidden block hides everything between them. It can't happen today, because there are 0 single-line HowTo blocks in v4.
Verdict: approve-ready from my side once deploy-preview (running at review time) goes green with verify-lastmod. Nothing blocking left.
Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
|
@dhananjay6561 thanks. The L118 nit is fixed in |
dhananjay6561
left a comment
There was a problem hiding this comment.
Manual review at a45f25e1
I tested this end to end on a full clone (gh pr checkout 928, npm install).
1. Build and sitemap
npm run build
npm run verify-lastmod
# verify-lastmod: ok (216/217 URLs, 47 distinct dates, largest 2026-09-30 x73)dateModified on built pages now comes from the last content change, not the newest commit:
| Page | dateModified |
Stock git date |
|---|---|---|
concepts/general-glossary |
2024-02-15 | 2026-09-01 (#897 sweep) |
running-keploy/cli-commands |
2026-10-07 | 2026-10-08 |
The only sitemap URLs still on 2026-09-01 are the 6 React pages in src/pages/ (about, leadership, security, privacy-policy, application-development, glossary index). #897 really rewrote those, and they're out of scope for the module by design.
2. Each rule, tested with throwaway commits
On a scratch branch, I made one change at a time to glossary/integration-testing.md (and keploy-explained/windows-wsl.md for HowTo), committed with a fixed GIT_COMMITTER_DATE, and read the date back through withContentLastUpdate with NODE_ENV=production:
| Change | Expected | Result |
|---|---|---|
| Front matter only | unchanged | ✅ |
| Trailing spaces / extra blank lines | unchanged | ✅ |
Link edit inside ## Related Terms |
unchanged | ✅ |
| FAQ answer edit (under Related Terms) | moves | ✅ |
| Body sentence edit | moves | ✅ |
| Re-indent a list item | moves | ✅ |
| Step edit in a hidden HowTo | unchanged | ✅ |
| Step edit in a visible HowTo | moves | ✅ |
Body edit whose commit is in .lastmod-ignore-revs |
back to old date | ✅ |
The one-line visible HowTo followed by a hidden one is detected now as well, so the nit is fixed.
3. Shallow clone safety nets
git clone --depth 1 ...
GITHUB_ACTIONS=true npm run build # exit 1: "shallow clone on CI: check out with fetch-depth: 0..."
npm run build # exit 0, warns
npm run verify-lastmod # exit 1: "only 1 distinct <lastmod> dates"Small note: this site has no visible "Last updated on" line. The custom DocItem only uses the date for dateModified in the JSON-LD, so the sitemap and dateModified are where to check this.
Everything passes and CI is green. LGTM from my side. Please squash-merge so the unsigned 0cf7bf11 doesn't land on main.
|
@dhananjay6561 thanks for the end-to-end pass. Good catch on the "Last updated" line: the description now says |
What has changed?
Adds an accurate
<lastmod>to the docs sitemap, and fixes the TechArticledateModified(JSON-LD) on every doc page.Before this PR:
<lastmod>(the sitemap plugin'slastmodoption was unset).fetch-depth: 1), so every page reported the date of the newest commit. Live check: every page I sampled had the samedateModified(2026-10-11).What this PR does:
src/lastUpdate/contentLastUpdate.js): for each doc, walks git history and uses the last commit that changed the Markdown body.## Related/## Related Terms, and<HowTo />blocks markedvisible={false}). Indentation still counts, since it changes Markdown structure..lastmod-ignore-revs(works like.git-blame-ignore-revs).last_update.datein front matter still wins.markdown.parseFrontMatter, so no doc files change.lastmod: "date"in the sitemap plugin config.fetch-depth: 0inmain.yml(deploy) andbuild_and_check.yml(PR preview)..lastmod-ignore-revs. feat(docs): site health + AI-citation fixes (A1, A2, A8 …) #897 was squash-merged, so two pages lose a small real edit and keep an older date.scripts/verify-lastmod.js(npm run verify-lastmod, run inbuild_and_check.yml) fails if under 90% of sitemap URLs have<lastmod>, under 10 distinct dates, or one date covers over 50%.Why: Google and Bing use
<lastmod>only when it is consistently accurate. Dates that move without real changes get ignored for the whole sitemap.Type of change
How Has This Been Tested?
yarn buildpasses;npm run verify-lastmodandnpm run verify-schemapass.verify-lastmodon a sitemap rewritten to one date: exits 1.GITHUB_ACTIONS=true: build throws.Checklist: