Derive excerpts on every save, and stop 404ing unchanged saves - #214
Merged
Conversation
The excerpt box is optional and the editor sends it on every save, filled in or not. POST treated a blank one as "derive it from the body"; PUT and PATCH stored the blank verbatim. So an article got an excerpt when it was created and lost it on the first edit afterward, and nothing ever put one back. That was survivable while listings selected the body: they didn't read it, but it was there. It isn't any more -- listings select `excerpt` and no longer fetch `text` at all, so a blank column is a story that renders on the section pages and the homepage with no summary under it. 390 of 10,049 articles were in that state, the whole CMS-era corpus; the Sports page was three stories with nothing beneath the headlines. ExcerptOrDerived is now what every write that sets the column goes through, so the three paths cannot disagree again. PATCH gets a real case in the field switch instead of falling through to the write-it-verbatim default: it derives from the patch's own body when the save carries one and from the stored body when it doesn't, so an excerpt-only autosave still gets a summary. A non-string is a 400, matching the other typed fields. cmd/backfill-excerpts repairs the rows already blanked, through the same ExcerptOrDerived, so a repaired row is byte-identical to what a save writes today. It leaves an article alone when there is nothing to derive from -- image-only posts, political cartoons, the crossword embeds -- because an excerpt made of markup is worse than none. Run against production: 42 repaired, 358 left alone, and a second run finds nothing to do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six handlers read RowsAffected and treat zero as "no such row": author update, patch, archive and restore, and both article write paths. MySQL reports rows whose values it changed, not rows it matched, so an UPDATE that matched its row and changed nothing reports zero -- and a save that happened to change nothing was answered 404, telling an author the article they have open no longer exists. The editor sends the whole form on every save and autosaves on a timer, and mod_date is stamped to the second, so two saves inside the same second are enough: the second rewrites every column to what it already holds. Archive and restore are worse, because there is no timing element -- archiving an already-archived author reports that the author is not there. clientFoundRows asks the server for matched rows instead, which is what all six call sites already mean. No call site uses RowsAffected to ask whether anything changed, so nothing else moves. The integration harness adds the flag to CMS_TEST_DSN when it is missing. Without that the tests exercise semantics production never runs with, which is how this survived a suite that covers these handlers closely: forced back to clientFoundRows=false, the new test reproduces the 404 exactly. This is not what was behind the resting error rate on the public API -- that is inbound WordPress-era URLs, and every failing request there is a GET. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent write-path bugs, both found chasing the empty excerpts on the Sports section page.
Blank excerpts
The excerpt box is optional and the editor sends it on every save whether or not the author filled it in.
POSTtreated a blank one as "derive from the body";PUTandPATCHstored the blank verbatim. An article got an excerpt on create and lost it on the first edit afterward.This became visible when listings stopped selecting the article body: they render
excerptand nothing else, so a blank column is a story that appears on every section page and the homepage with no summary. 390 of 10,049 articles were in that state.ExcerptOrDerivedis now the single path for every write that sets the column.PATCHgets a real case in the field switch rather than falling through to the write-it-verbatim default, deriving from the patch's own body when it carries one and from the stored body when it doesn't.cmd/backfill-excerptsrepairs the already-blanked rows through the same function. It skips articles with nothing to derive from — image-only posts, political cartoons, crossword embeds — because an excerpt made of markup is worse than none.Already run against production: 42 repaired, 358 correctly left alone, second run is a no-op. It deliberately does not touch
mod_date: this is a repair of a value the CMS should have written, not an edit, and bumping it would move 42 articles' sitemaplastmodand re-queue them all for embedding.Unchanged saves answered as 404
Six handlers read
RowsAffectedand treat zero as "no such row". MySQL reports changed rows, not matched rows, so an UPDATE that matched its row and changed nothing reported zero — and the author was told the article they have open does not exist. Two saves inside the same second are enough, sincemod_dateis stamped to the second. Archive and restore have no timing element at all: archiving an already-archived author 404s.clientFoundRows=trueasks for matched rows, which is what all six call sites already mean. No call site usesRowsAffectedfor change detection.The integration harness now adds the flag when
CMS_TEST_DSNlacks it — without that, the tests exercise semantics production never runs with, which is how this survived a suite that covers these handlers closely.Testing
Full suite passes against a real MariaDB. The no-op test was verified to reproduce the bug: forced to
clientFoundRows=falseit fails with exactly the404 article not foundan editor would see.Note
The second fix is not what is behind the ~2% resting error rate on the public API. That is inbound WordPress-era URLs — stale tag archives and legacy author logins — and every failing request there is a
GET. The fix for that is Scalene'sfix/author-slug-resolutionbranch.🤖 Generated with Claude Code