From 6320e2c0c90a5bb01cca917970d823c1e02ce7a9 Mon Sep 17 00:00:00 2001 From: jim Date: Wed, 5 Aug 2026 09:36:17 -0500 Subject: [PATCH] fix: stop --ingest's NULL sentinel from clobbering post content UpsertPostFromTextFile (the persistence layer under --ingest) uses NULL as its "this file's record had no line for that field" sentinel -- the direct analog of the "." convention just fixed in UpdatePost. IngestMode strips a trailing "_N" off the folder name before it ever reaches this function, so a duplicate export folder deliberately collapses onto the same BlogName -- reconciling multiple differently-formatted files for one post is the whole point of --ingest. Files are walked in raw filesystem enumeration order, never sorted, so which file's call lands last for a given (BlogName, PostID) is arbitrary. The UPDATE branch set every column unconditionally, so whichever file processed last for a PostID nulled out every field its own record didn't carry, silently erasing real content another file had. Worse than the "." case: that one only caused churn (two writes cancelling out); this one loses data, in an order that depends on filesystem enumeration. Every content column is now guarded the same way, NULL instead of "." as the sentinel: `col = CASE WHEN @col IS NULL THEN col ELSE @col END` in the SET list, `(@col IS NOT NULL AND IFNULL(col,'') <> @col) OR ...` in the change-detection. Narrow the same way: only a missing line (NULL) is the sentinel -- G() already distinguishes that from present-but-blank (""), so an explicit empty field still overwrites. HasImage is deliberately left unguarded and documented as a known gap: IngestMode always computes a concrete bool, defaulting false when a file has no "Has Image:" line, so this function can't currently tell "no image" from "not reported" without changing the parameter to bool? and threading that through IngestMode/LegacyPostsDbImporter too. Verified against a throwaway DB using the exact SQL text and parameter binding: a full-format record's real Title/Slug/Tags now survive a same-PostID partial record whose format doesn't carry those fields, in both file orders, while a genuine content change and an explicit empty value still write and still move DateModified. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 30 ++++++++++ URLNotesGrabberCORE/DataAccess.cs | 91 ++++++++++++++++++------------- 2 files changed, 83 insertions(+), 38 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index eb15fcc..b602bda 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -124,6 +124,36 @@ though the true content never changes. source can legitimately supply `"."` as "field absent" before deciding whether it needs the same `CASE` treatment — don't assume every column needs it +**`--ingest` (`UpsertPostFromTextFile`) uses `NULL`, not `"."`, for the same "field absent" +convention, and reconciling exactly this kind of duplicate IS the feature's job.** +`IngestMode` strips a trailing `_N` from the folder name before it ever reaches +`UpsertPostFromTextFile`, so a duplicate export folder collapses onto the same `BlogName` on +purpose — the whole point is to merge multiple differently-formatted files for the same post +into one row. `IngestMode.G(key)` returns `null` (not `"."`) when a field's line is absent +from a given file, `LegacyPostsDbImporter` passes `null` straight from a `NULL` source column, +and files are walked in raw filesystem enumeration order — never sorted — so which file's call +lands last for a given `(BlogName, PostID)` is arbitrary. + +- Before the fix, the `UPDATE` branch set every column unconditionally, so whichever file + processed last for a `PostID` would null out every field its own record didn't carry — + silently erasing real `Title`/`Slug`/`Tags`/… another file had, the opposite of what + `--ingest` exists to do. This is worse than the `"."` case above: that one only caused + churn (the two writes canceled out); this one loses data, and which posts lose which + fields depends on filesystem enumeration order +- Same shape of fix, `NULL` instead of `"."` as the sentinel: `col = CASE WHEN @col IS NULL + THEN col ELSE @col END` in the `SET` list, `(@col IS NOT NULL AND IFNULL(col, '') <> @col) + OR ...` in the change-detection +- Same narrow rule: only `NULL` (the field's line was never present in this file) is the + sentinel. `G()` already distinguishes this from "present but blank" — a dictionary miss is + `null`, an empty value after the prefix is `""` — so an explicitly blank field still + overwrites +- `HasImage` is **not** guarded and remains a known gap: `IngestMode` always computes a + concrete `bool` (defaulting `false` when a file has no `Has Image:` line), so there is no + way for this function to tell "this format says no image" from "this format doesn't report + it at all" without changing the parameter to `bool?` and threading that through + `IngestMode`/`LegacyPostsDbImporter`. Fix this the same way if `--ingest` is observed + downgrading a post's `HasImage` from `1` to `0` + ### Testing - No existing test suite; use xUnit if adding tests - Test critical logic: `ApiKeyPool` init, color parsing, config persistence diff --git a/URLNotesGrabberCORE/DataAccess.cs b/URLNotesGrabberCORE/DataAccess.cs index 1dc991a..1247d32 100644 --- a/URLNotesGrabberCORE/DataAccess.cs +++ b/URLNotesGrabberCORE/DataAccess.cs @@ -2026,48 +2026,63 @@ namespace URLNotesGrabberCORE if (rowsInserted == 0) { + // NULL is this function's sentinel for "this file's record had no line for + // that field" (IngestMode's G(key) misses return null; LegacyPostsDbImporter + // passes null straight from a NULL source column) -- it does not mean "clear + // this field". --ingest's entire reason to exist is reconciling multiple + // export files for the same (BlogName, PostID) -- IngestMode normalizes a + // "_2"-suffixed duplicate folder onto the same blog name specifically so a + // second, differently-formatted file for a post it already has gets merged in. + // Files are walked in filesystem enumeration order, not sorted, so which + // file's UpsertPostFromTextFile call runs last for a given PostID is + // effectively arbitrary. An unconditional SET here would let whichever file + // processed last silently null out every column its own record didn't carry, + // erasing real content the other file had -- the opposite of "clean up". Each + // column is CASE-guarded to keep the existing value when this call's parameter + // is NULL, and the change-detection ignores a NULL-vs-real mismatch the same + // way, so a partial record converges into the row instead of overwriting it. string updateSql = @"UPDATE Posts SET - reblogURL = @reblogURL, - PostDate = @PostDate, - PostURL = @PostURL, - Slug = @Slug, - ReblogKey = @ReblogKey, - ReblogName = @ReblogName, - Summary = @Summary, - Quote = @Quote, - Body = @Body, - Tags = @Tags, - Link = @Link, - PhotoURL = @PhotoURL, - PhotoCaption = @PhotoCaption, - DownloadedFiles = @DownloadedFiles, - AudioCaption = @AudioCaption, - Question = @Question, - Answer = @Answer, - Title = @Title, - PostType = @PostType, + reblogURL = CASE WHEN @reblogURL IS NULL THEN reblogURL ELSE @reblogURL END, + PostDate = CASE WHEN @PostDate IS NULL THEN PostDate ELSE @PostDate END, + PostURL = CASE WHEN @PostURL IS NULL THEN PostURL ELSE @PostURL END, + Slug = CASE WHEN @Slug IS NULL THEN Slug ELSE @Slug END, + ReblogKey = CASE WHEN @ReblogKey IS NULL THEN ReblogKey ELSE @ReblogKey END, + ReblogName = CASE WHEN @ReblogName IS NULL THEN ReblogName ELSE @ReblogName END, + Summary = CASE WHEN @Summary IS NULL THEN Summary ELSE @Summary END, + Quote = CASE WHEN @Quote IS NULL THEN Quote ELSE @Quote END, + Body = CASE WHEN @Body IS NULL THEN Body ELSE @Body END, + Tags = CASE WHEN @Tags IS NULL THEN Tags ELSE @Tags END, + Link = CASE WHEN @Link IS NULL THEN Link ELSE @Link END, + PhotoURL = CASE WHEN @PhotoURL IS NULL THEN PhotoURL ELSE @PhotoURL END, + PhotoCaption = CASE WHEN @PhotoCaption IS NULL THEN PhotoCaption ELSE @PhotoCaption END, + DownloadedFiles = CASE WHEN @DownloadedFiles IS NULL THEN DownloadedFiles ELSE @DownloadedFiles END, + AudioCaption = CASE WHEN @AudioCaption IS NULL THEN AudioCaption ELSE @AudioCaption END, + Question = CASE WHEN @Question IS NULL THEN Question ELSE @Question END, + Answer = CASE WHEN @Answer IS NULL THEN Answer ELSE @Answer END, + Title = CASE WHEN @Title IS NULL THEN Title ELSE @Title END, + PostType = CASE WHEN @PostType IS NULL THEN PostType ELSE @PostType END, HasImage = @HasImage, DateModified = @DateModified WHERE BlogName = @BlogName AND PostID = @PostID AND ( - IFNULL(reblogURL, '') <> IFNULL(@reblogURL, '') OR - IFNULL(PostDate, '') <> IFNULL(@PostDate, '') OR - IFNULL(PostURL, '') <> IFNULL(@PostURL, '') OR - IFNULL(Slug, '') <> IFNULL(@Slug, '') OR - IFNULL(ReblogKey, '') <> IFNULL(@ReblogKey, '') OR - IFNULL(ReblogName, '') <> IFNULL(@ReblogName, '') OR - IFNULL(Summary, '') <> IFNULL(@Summary, '') OR - IFNULL(Quote, '') <> IFNULL(@Quote, '') OR - IFNULL(Body, '') <> IFNULL(@Body, '') OR - IFNULL(Tags, '') <> IFNULL(@Tags, '') OR - IFNULL(Link, '') <> IFNULL(@Link, '') OR - IFNULL(PhotoURL, '') <> IFNULL(@PhotoURL, '') OR - IFNULL(PhotoCaption, '') <> IFNULL(@PhotoCaption, '') OR - IFNULL(DownloadedFiles, '') <> IFNULL(@DownloadedFiles, '') OR - IFNULL(AudioCaption, '') <> IFNULL(@AudioCaption, '') OR - IFNULL(Question, '') <> IFNULL(@Question, '') OR - IFNULL(Answer, '') <> IFNULL(@Answer, '') OR - IFNULL(Title, '') <> IFNULL(@Title, '') OR - IFNULL(PostType, '') <> IFNULL(@PostType, '') OR + (@reblogURL IS NOT NULL AND IFNULL(reblogURL, '') <> @reblogURL) OR + (@PostDate IS NOT NULL AND IFNULL(PostDate, '') <> @PostDate) OR + (@PostURL IS NOT NULL AND IFNULL(PostURL, '') <> @PostURL) OR + (@Slug IS NOT NULL AND IFNULL(Slug, '') <> @Slug) OR + (@ReblogKey IS NOT NULL AND IFNULL(ReblogKey, '') <> @ReblogKey) OR + (@ReblogName IS NOT NULL AND IFNULL(ReblogName, '') <> @ReblogName) OR + (@Summary IS NOT NULL AND IFNULL(Summary, '') <> @Summary) OR + (@Quote IS NOT NULL AND IFNULL(Quote, '') <> @Quote) OR + (@Body IS NOT NULL AND IFNULL(Body, '') <> @Body) OR + (@Tags IS NOT NULL AND IFNULL(Tags, '') <> @Tags) OR + (@Link IS NOT NULL AND IFNULL(Link, '') <> @Link) OR + (@PhotoURL IS NOT NULL AND IFNULL(PhotoURL, '') <> @PhotoURL) OR + (@PhotoCaption IS NOT NULL AND IFNULL(PhotoCaption, '') <> @PhotoCaption) OR + (@DownloadedFiles IS NOT NULL AND IFNULL(DownloadedFiles, '') <> @DownloadedFiles) OR + (@AudioCaption IS NOT NULL AND IFNULL(AudioCaption, '') <> @AudioCaption) OR + (@Question IS NOT NULL AND IFNULL(Question, '') <> @Question) OR + (@Answer IS NOT NULL AND IFNULL(Answer, '') <> @Answer) OR + (@Title IS NOT NULL AND IFNULL(Title, '') <> @Title) OR + (@PostType IS NOT NULL AND IFNULL(PostType, '') <> @PostType) OR IFNULL(HasImage, 0) <> @HasImage )";