Skip to content

Add capture/, and make the directory READMEs the authority - #66

Merged
ptr727 merged 13 commits into
developfrom
add-capture-directory
Aug 8, 2026
Merged

Add capture/, and make the directory READMEs the authority#66
ptr727 merged 13 commits into
developfrom
add-capture-directory

Conversation

@ptr727

@ptr727 ptr727 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

The migration's process lived in three places at once: a 2.9 GB directory outside the repo, a blog post, and five committed docs restating each other. Concretely, before this change:

Duplicated Times
"provenance script, not a CI step, selects the export by content" 6
The Blogger slug-truncation fact 4
The capture's four-directory layout 2
"What the post covers" 2

And OPERATIONS.md asserted the migration was "documented once, as a post", which was already false when it was written.

capture/ is to the migration what ops/ is to the backup pull: the durable scripts plus a README that is the authoritative procedure.

Which scripts, and why those

The fourteen split by whether they can ever run again, not by whether they look reusable. That distinction is the whole call:

  • A re-export re-runs the conversion chain, so run-wp2hugo.sh, clean-content.py, restructure-content.py, localize-external.py and enumerate-media.py are carried. The last is carried by dependency: build-redirects.py reads an inventory only it produces.
  • build-golden.py and classify.py fail that test and are carried anyway, as the record behind an append-only contract, and the README says so out loud. An append to golden-urls.txt is only reviewable against the derivation that produced the original, and that derivation dies with the old hosting.
  • The crawl pair and the three fetchers are named, not carried. Their outputs are the durable artifact, and the scripts hit a site that will be gone. mirror.sh's span-host list is recorded in the README anyway, because checks/README.md adjudicates orphaned media against that tree.

The move, and the one thing that proves it

build-redirects.py moves from checks/ to capture/. GOVERNANCE.md defines checks/ as "the URL contract and the gates that enforce it" — this generates and gates nothing, which is exactly why six documents had to say so in prose. Moving it makes the disclaimer structural, and closes the three-copies problem: both stale copies are now named by path and marked stale so finding one is not mistaken for finding a source.

The proof is that the maps are byte-identical. git diff --stat deploy/maps/ is empty after re-running the moved generator, which is what shows the move and the CHECKSREPO re-anchor changed no behavior.

Nothing here names a machine

Four CAPTURE_* values carry the address, the paths and the account slug. CAPTURE_AUTHOR_SLUG exists because classify.py hardcoded an account username, which cannot go in a public repo; unset, the script skips the backfill and says so rather than emitting a list silently short by the author URLs.

check-env-docs.py was watched failing on each of the three new values before the rows were added, and watched failing in the reverse direction too. A required value that is unset stops the script and names what is missing.

ENVIRONMENT.md records that CAPTURE_SOURCE_URL is not HUGO_BASEURL — they hold the same string after cutover and mean different things, so merging them would point a verification run at the new site while every check still passed.

The hierarchy inverts

READMEs are procedure and fact; the post is the casual account; a post may cite a README and a README never cites the post. Applied as a class rather than an instance: OPERATIONS.md and TODO.md both carried the old direction.

The post keeps its prose in full and gains one line pointing at the maintained version. Its capture/ layout code block deliberately stays — it teaches a reader migrating their own blog, which is different from recording ours.

Verification beyond the linters

  • Maps byte-identical after re-running the moved generator.
  • Every script run from /tmp against a scratch capture resolves inputs and outputs there, not into the repo. Three previously located both from __file__, so this was the real risk of the move. git status clean and the two committed URL lists untouched afterwards.
  • All eight refuse when their variables are unset, naming what is missing.
  • run-wp2hugo.sh asks build-redirects.py --print-export rather than reimplementing the export choice. A shell version cannot match it: the test is that one item carries both post_type=post and status=publish, where two greps over a whole file would accept a media-only export. My first attempt did exactly that and found zero where the Python finds one — so the two now share one implementation and provably agree.

Gates: shellcheck and shfmt -d on 5 scripts (the new .sh added to CI's array first, since one not in it is silently ungated), markdownlint 19 files, editorconfig, actionlint, cspell, check-env-docs.py 31/31. The hub's prose_lint.py is at baseline on every file touched and clean on all three I authored.

Also fixed in passing: checks/check-env-docs.py had no .gitattributes LF pin, and .editorconfig named the generator's old path.

One near-miss worth recording

While testing, I ran run-wp2hugo.sh against the real capture. It is read-only and a conversion writes into it. A head -2 in my pipeline sent SIGPIPE before anything landed, and I verified nothing was modified — but that was luck rather than design, and the conversion script should not be exercised against the provenance store at all.

The migration's process lived in a 2.9 GB directory outside the repo, a
blog post, and five committed docs that restated each other. The
"provenance script, not a CI step, selects the export by content"
paragraph existed six times, the Blogger truncation fact four times, the
capture layout twice. OPERATIONS.md asserted the migration was
"documented once, as a post", which was already false.

capture/ is to the migration what ops/ is to the backup pull: the
durable scripts plus a README that is the authoritative procedure.

The fourteen capture scripts split by whether they can ever run again,
not by whether they look reusable. A re-export re-runs the conversion
chain, so those five are carried. build-golden.py and classify.py are
carried as the record behind an append-only contract and say so, since
they cannot run once the old hosting ends. The crawl pair and the three
fetchers are named rather than carried, because their outputs are the
durable artifact.

build-redirects.py moves from checks/ to capture/. GOVERNANCE.md defines
checks/ as the contract and the gates that enforce it, and this
generates rather than gates, which is why six documents had to say so in
prose. Moving it makes that structural and closes the three-copies
problem: the two stale copies are now named by path and marked stale.

Nothing in capture/ names a machine. Four CAPTURE_* values carry the
address, the paths and the account slug, documented in ENVIRONMENT.md
and gated by check-env-docs.py, which was watched failing on each before
the rows were added. A required value that is unset stops the script and
names what is missing.

The doc hierarchy inverts, as a class rather than an instance: the
READMEs are procedure and fact, the post is the casual account, and a
post may cite a README where a README never cites the post. TODO.md
carried the same old claim and is fixed too.

Verified beyond the linters:

- capture/build-redirects.py reproduces deploy/maps/ byte for byte,
  which is what proves the move and the REPO re-anchor changed nothing.
- Every script run from /tmp against a scratch capture resolves its
  inputs and outputs there, not into the repo. Three of them previously
  located both from __file__, so this was the real risk of the move.
- run-wp2hugo.sh asks build-redirects.py --print-export rather than
  reimplementing the choice. A shell version could not match it: the
  test is that one item carries both post_type=post and status=publish,
  where two greps over a whole file accept a media-only export. My first
  attempt did exactly that and found zero where the Python finds one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 8, 2026 22:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces capture/ as the authoritative home for the migration provenance tooling and procedure, consolidating previously duplicated migration documentation into directory READMEs and re-homing the redirect-map generator accordingly.

Changes:

  • Add capture/ with migration/provenance scripts and an authoritative capture/README.md procedure.
  • Move/update references to the redirect-map generator so it lives under capture/ (and update docs to make READMEs the source of truth).
  • Extend environment documentation/examples for capture-specific variables and tighten repo hygiene (CI shell lint list, .gitattributes, doc link updates).

Reviewed changes

Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
TODO.md Updates migration documentation pointer to capture/README.md and adjusts build-redirects.py path references.
README.md Adds capture/ to the repo overview and re-points migration documentation to capture/README.md.
ops/README.md Renames/clarifies the backup pull README header and description.
OPERATIONS.md Reframes migration record authority to directory READMEs and updates build-redirects.py references.
example.env Adds capture-related environment variables (CAPTURE_SOURCE_URL, CAPTURE_SOURCE_API, CAPTURE_AUTHOR_SLUG) and clarifies capture-root behavior.
ENVIRONMENT.md Documents capture-related variables and their intended semantics/usage.
deploy/README.md Updates references from checks/build-redirects.py to capture/build-redirects.py and rehomes explanatory text to capture/README.md.
content/posts/2026/08/01/moving-this-blog-from-wordpress-to-hugo.md Adds a pointer from the narrative post to the maintained capture/README.md.
checks/README.md Moves generator-specific rationale out of checks/ and points to capture/README.md for those details.
capture/run-wp2hugo.sh Adds a conversion driver that aligns export selection with build-redirects.py --print-export.
capture/restructure-content.py Adds a reshaping step to align converted content tree layout with published URLs.
capture/README.md Adds the authoritative capture procedure, rationale, and variable documentation.
capture/localize-external.py Adds a step to localize externally hosted images and rewrite content accordingly.
capture/enumerate-media.py Adds a script to enumerate media references via the old platform API and write inventory artifacts.
capture/clean-content.py Adds a cleanup step to drop comments and reduce front matter to site-driving keys.
capture/classify.py Adds URL classification tooling with optional author-archive backfill based on env.
capture/build-redirects.py Updates generator anchoring/arg handling and adds --print-export to share export selection with shell tooling.
capture/build-golden.py Adds derivation+verification tooling to build/validate the golden URL set from multiple sources.
.gitignore Updates Python byproduct comment to include capture/ provenance tools.
.github/workflows/validate-task.yml Adds capture/run-wp2hugo.sh to the shell lint/format gated script list.
.gitattributes Pins LF handling for executable Python scripts under capture/ and adds LF pin for checks/check-env-docs.py.
.editorconfig Updates generator-path comment for deploy/maps/*.map to match the new capture/build-redirects.py location.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread capture/enumerate-media.py
Posts went through all_items() and paginated. Pages were a single
number=100 request, so a site with more than a hundred of them would
lose the rest with no error and no sign in the output.

It was correct for this site, which has two. It stops being merely
correct-for-now the moment the script is carried as a durable tool, and
silent truncation is the exact shape of loss the capture exists to
catch, so the special case is gone rather than commented.

Raised by Copilot on PR #66.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 8, 2026 22:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

capture/classify.py:118

  • The output lists are written with the platform default encoding. Write them as UTF-8 explicitly to avoid locale-dependent behavior (and to match the rest of the capture tooling which uses UTF-8).
(ROOT / "checks" / "golden-urls.txt").write_text("".join(f"{u}\n" for u in sorted(golden)))
(ROOT / "checks" / "redirect-urls.txt").write_text("".join(f"{u}\n" for u in sorted(redirect)))

capture/localize-external.py:159

  • external-media-map.tsv is written with the platform default encoding. Since the data can include non-ASCII URLs and this script is intended to be run on different machines, explicitly write UTF-8 to avoid locale-dependent output / decode issues later.
        (inv / "external-media-map.tsv").write_text(
            "source_url\tlocal_path\n" + "".join(f"{u}\t{v}\n" for u, v in sorted(mapping.items()))
        )

capture/build-golden.py:96

  • posts.json is read with the platform default encoding. Since it is written as UTF-8 by enumerate-media.py, read it as UTF-8 explicitly to avoid decode errors or silent mojibake on non-UTF-8 locales.
    # 3. Derivation - the part no crawl can reach.
    posts = json.loads((ROOT / "inventory" / "posts.json").read_text())
    derived = set()

capture/classify.py:49

  • url-verification.tsv is read with the platform default encoding. Since the capture files are UTF-8 (and URLs can contain non-ASCII), read as UTF-8 explicitly so the script behaves consistently across environments.

This issue also appears on line 117 of the same file.

rows = [
    line.rstrip("\n").split("\t")
    for line in (ROOT / "checks" / "url-verification.tsv").read_text().splitlines()[1:]
]

Comment thread capture/restructure-content.py
Python's default text encoding is locale-dependent, so a capture file
written as UTF-8 on one machine and read on another with a different
locale decodes wrongly or raises. The data invites it: URLs in the
verification table and the external-media map carry non-ASCII.

Swept as a class rather than patching the four sites the review named.
Eleven reads and writes across four scripts now state utf-8, and the
directory has none left. The one open() untouched is a urllib opener
rather than a file.

These scripts were written to run in one place and are now carried to be
run anywhere, which is what turns an implicit encoding from a non-issue
into a portability bug.

Re-verified after the change: the maps are still byte-identical, and
every script still refuses when its variables are unset.

Raised by Copilot on PR #66.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 8, 2026 22:22
shutil.move replaces an existing destination without a word, and this
applies moves in bulk, so a collision would destroy a post and print
APPLIED. Two ways it can happen: two sources resolving to the same
destination, which flattening pages to content/<slug>.md makes possible,
and a destination left behind by a half-finished earlier run.

Both are now detected before anything moves, and the script exits
non-zero naming each one, rather than discovering the second case half
way through a bulk move with no way back.

Measured before deciding how to treat it: the real capture has 110 files
carrying a url and 110 distinct destinations, so this is a latent hazard
rather than a live defect. It earns a fix anyway because the operation is
destructive and silent, which is a different risk class from a wrong
string.

Verified against fixtures for both branches, and the real capture still
plans its moves cleanly.

Raised by Copilot on PR #66.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727

ptr727 commented Aug 8, 2026

Copy link
Copy Markdown
Owner Author

Four suppressed findings, all the same class, all real, fixed in d8e3267.


capture/classify.py:49, capture/classify.py:118, capture/build-golden.py:96, capture/localize-external.py:159 — read/written with the platform default encoding.

Correct, and the timing is what makes it matter. These scripts were written to run in one place, against one locale, as one-shots in a scratch directory. This PR carries them to be run anywhere, and that is exactly what turns an implicit encoding from a non-issue into a portability bug. The data invites it too: the verification table and the external-media map both carry URLs with non-ASCII in them.

Swept as a class rather than patched at the four sites named, per this repo's rule that a review flags an instance and the fix is the class. Eleven reads and writes across classify.py, build-golden.py, build-redirects.py and localize-external.py now state utf-8, and a grep for an implicit one across the directory returns nothing.

Two the sweep touched that the review did not name are worth calling out, because both would have been worse than the four:

  • build-redirects.py writes the committed deploy/maps/. An encoding difference there lands in the repository rather than in a scratch file.
  • build-golden.py reads spider.log with errors="replace" and no encoding. That combination is the quiet one: it would not raise, it would substitute characters and produce a URL list subtly wrong on a machine with a different locale.

The one open() left alone is a urllib opener rather than a file.

Re-verified after the change, because build-redirects.py is in that set: the maps are byte-identical, and every script still refuses when its variables are unset.


Coverage

Still 21 of 22 files with no file list, so the unread one cannot be identified from the API. Reporting rather than re-requesting, as on the previous PRs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (2)

capture/localize-external.py:44

  • MAGIC includes (b"RIFF", ".webp"), which matches any RIFF container (e.g., WAV/AVI) and makes the more specific RIFF+WEBP check below unreachable. This can misclassify non-WebP content as an image and write a bogus .webp file.
    (b"GIF89a", ".gif"),
    (b"RIFF", ".webp"),
    (b"BM", ".bmp"),

capture/localize-external.py:104

  • The -h wrapper handling is gated on body.lstrip()[:6].lower() == b"<html>", but many HTML pages (including Google-hosted ones) start with a doctype (<!doctype html>), which would skip the wrapper logic and report a false "not an image".
    if depth == 0 and body.lstrip()[:6].lower() == b"<html>":

Copilot AI review requested due to automatic review settings August 8, 2026 22:31
@ptr727
ptr727 requested a lite review from Copilot and removed request for Copilot August 8, 2026 22:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (2)

capture/build-redirects.py:88

  • build-redirects.py now treats any single non---print-export argument as the capture directory, even if it’s an unknown option like --help. That produces a misleading “no WordPress export XML found under --help/…” error instead of a usage message. Reject any arg starting with - (other than --print-export) as an unknown option.
    capture/clean-content.py:41
  • clean-content.py depends on yaml (PyYAML), which is not part of the Python stdlib and the repo doesn’t appear to vendor or declare Python dependencies. As-is, a fresh environment running the capture tooling will fail with ModuleNotFoundError without a clear remediation path. Either avoid the dependency or fail fast with an explicit install instruction.
import os
import re
import shutil
import sys
import pathlib
from pathlib import Path

import yaml

Two suppressed findings, both real.

A single non-flag argument was taken as the capture directory, so
`--help` produced "no export XML found under --help/export/raw" and sent
the reader looking for a missing file rather than a mistyped flag. Any
argument starting with a dash other than --print-export is now refused
with a usage line.

clean-content.py imports PyYAML, which is the only third-party import in
capture/ or checks/, verified by resolving every import in both against
the standard library. Nothing declared it, so a fresh environment failed
with a bare ModuleNotFoundError. It now names the package and the install
command, and PyYAML has a row in the tools table beside the others.

Both verified by running them: the unknown option exits 2 with a usage
line, the dependency guard fires with yaml shadowed on PYTHONPATH, and
the maps are still byte-identical.

Raised by Copilot on PR #66.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 8, 2026 22:37
@ptr727

ptr727 commented Aug 8, 2026

Copy link
Copy Markdown
Owner Author

Two more suppressed findings, both real, fixed in 3fda126.


capture/build-redirects.py:88 — treats any single non---print-export argument as the capture directory, even if it's an unknown option like --help. That produces a misleading "no WordPress export XML found under --help/…" error instead of a usage message.

Correct, and it is my regression — the argument used to be mandatory and positional, and making CAPTURE_ROOT the default is what opened the gap. Reproduced exactly as described:

$ capture/build-redirects.py --help
no WordPress export XML found under --help/export/raw

That sends a reader looking for a missing file rather than a mistyped flag, which is the expensive kind of wrong error. Any argument starting with a dash other than --print-export is now refused with a usage line, exit 2.


capture/clean-content.py:41 — depends on yaml (PyYAML), which is not part of the stdlib and the repo doesn't appear to declare Python dependencies.

Correct. I checked whether it was the only one rather than assuming: resolving every import across capture/ and checks/ against the standard library, yaml is the single third-party dependency in either directory. Nothing declared it, so a fresh environment got a bare ModuleNotFoundError with no remediation.

Both halves of your suggested fix, since they serve different readers:

  • The script names the package and the install command instead of raising. Verified by shadowing yaml on PYTHONPATH: PyYAML is required by this script: pip install PyYAML (Debian: apt install python3-yaml), exit 1.
  • PyYAML has a row in the tools table in deploy/README.md beside python3, rsync and the rest, and capture/README.md notes that everything else there is standard library.

The maps are byte-identical after both changes.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (3)

capture/localize-external.py:44

  • MAGIC treats any RIFF file as a WebP because it includes (b"RIFF", ".webp"). That can misclassify non-WebP RIFF containers (e.g., WAV) as images and cause the script to write garbage with a .webp extension. Since you already have a RIFF+WEBP check below, drop the broad RIFF entry and let the data[8:12] == b"WEBP" guard decide.
MAGIC = [
    (b"\x89PNG\r\n\x1a\n", ".png"),
    (b"\xff\xd8\xff", ".jpg"),
    (b"GIF87a", ".gif"),
    (b"GIF89a", ".gif"),

capture/localize-external.py:112

  • Wrapper-page detection is too strict: it only follows the -h HTML wrapper when the body starts with <html>. Real pages often start with <!doctype html> or whitespace/comments, so the script can incorrectly report not an image for a valid wrapper page. Consider attempting the <img src> extraction (and the -h rewrite fallback) whenever sniff() fails at depth 0, without requiring an exact <html> prefix.
    # Not an image. If it is Google's `-h` wrapper page, it names the real image.
    if depth == 0 and body.lstrip()[:6].lower() == b"<html>":
        m = IMG_IN_WRAPPER.search(body)
        if m:
            return fetch(m.group(1).decode("utf-8", "replace"), depth + 1)

.editorconfig:67

  • The comment says the Caddy map files are tab-separated, but the generated maps in deploy/maps/*.map are space-separated (e.g., deploy/maps/blogger.map lines 1-3). This comment can mislead editors/tools and future maintainers; update it to match the actual format.
# Caddy map files are tab-separated redirect tables generated by `capture/build-redirects.py`.
# Trailing whitespace is significant to the parse, and the generator owns the formatting.

Three suppressed findings, all real, and the first two are in the very
function that exists to catch a body that is not an image.

MAGIC carried a bare RIFF entry mapping to .webp. RIFF is a container,
not a format, so WAV and AVI share the header and both sniffed as WebP.
That also made the correct check below it, which confirms WEBP at bytes
8 to 12, unreachable. The bare entry is gone and the real check does the
work: a WAV now returns None and is refused, where it previously would
have been written out with an image extension.

Wrapper detection required the body to start with literal <html>, so a
page opening with <!doctype html>, a comment, or whitespace was reported
as "not an image" instead of being followed to the image it names. That
is the trap this script was written for, defeated by the most ordinary
way to write the page it was looking for.

Third: .editorconfig described the Caddy maps as tab-separated. All five
are space-separated, confirmed with cat -A. That comment is pre-existing
and I moved it in this branch without reading it.

Verified by exercising sniff() and the wrapper predicate directly rather
than by reading them.

Raised by Copilot on PR #66.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 8, 2026 22:42
@ptr727

ptr727 commented Aug 8, 2026

Copy link
Copy Markdown
Owner Author

Three suppressed findings, all real, fixed in 78ea788. The first two are in the very function that exists to catch a body that is not an image, which makes them the best finds of this PR.


capture/localize-external.py:44MAGIC treats any RIFF file as a WebP. That can misclassify non-WebP RIFF containers (e.g., WAV) as images.

Correct, and worse than misclassification: it made the correct check unreachable. RIFF is a container rather than a format, so WAV and AVI share the header, and the bare (b"RIFF", ".webp") entry matched first — leaving the proper confirmation at bytes 8 to 12 in sniff() as dead code.

Reproduced before and after:

before   WAV  -> .webp        WEBP -> .webp
after    WAV  -> None         WEBP -> .webp        PNG -> .png

A sound file would have been written out with an image extension by the guard whose entire job is refusing non-images.


capture/localize-external.py:112 — Wrapper-page detection is too strict: it only follows the -h HTML wrapper when the body starts with <html>.

Correct, and this one defeats the script's headline feature. The -h wrapper is the trap this file was written for, and requiring a literal <html> meant the most ordinary way to write that page — a doctype first — was reported as "not an image" and the image was never fetched.

Now matched on <html or <!doctype html after stripping leading whitespace, verified across all three shapes:

<html><img src="x">                       -> detected
<!DOCTYPE html>\n<html><img src="x">      -> detected
\n  <!doctype html><img src="x">          -> detected

.editorconfig:67 — The comment says the Caddy map files are tab-separated, but the generated maps are space-separated.

Correct, and it is pre-existing. All five maps are space-separated, confirmed with cat -A. I moved that line in this branch without reading it, which is the more useful lesson: a mechanical path update is still an edit, and it carries whatever the line already got wrong.


Both code fixes were exercised directly rather than read.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (1)

capture/localize-external.py:176

  • localize-external.py prints an UNRESOLVED section when some external images fail to download, but it still exits with status 0. That makes a partially localized run look successful in scripts/automation and is easy to miss in long output. Return a non-zero exit code when there are unresolved failures (both in dry-run and apply mode), and sys.exit() with that code in __main__.
    if failures:
        print(f"\nUNRESOLVED ({len(failures)}):")
        for u, why in failures[:20]:
            print(f"  {why:14} {u[:110]}")
    print("\nAPPLIED" if apply else "\nDRY RUN - pass --apply")

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (2)

example.env:65

  • The comment says every script under capture/ resolves its inputs and outputs beneath CAPTURE_ROOT, but capture/build-redirects.py writes redirect maps under deploy/maps/ in the repository (not under the capture). This line is misleading for someone setting up the capture variables.
# The provenance capture, holding the exports and the crawl of the old platform.
# Every script under capture/ resolves its inputs and outputs beneath it.
CAPTURE_ROOT=/path/to/blog-capture

ENVIRONMENT.md:30

  • This CAPTURE_ROOT row claims every script under capture/ resolves inputs and outputs beneath the capture, but capture/build-redirects.py writes committed maps to deploy/maps/ in the repo. The note should reflect that exception so readers don’t assume all outputs stay under CAPTURE_ROOT.
| `CAPTURE_ROOT` | the provenance capture, holding the WordPress exports, the crawl of the old platform, and the inventories derived from it | Every script under [`capture/`](./capture/) resolves its inputs and outputs beneath it, and [`capture/build-redirects.py`](./capture/build-redirects.py) also accepts it as a first argument, which wins. Environment-independent, so it belongs in the default file only. |

Both example.env and ENVIRONMENT.md said every script under capture/
resolves its inputs and outputs beneath CAPTURE_ROOT. Seven do.
build-redirects.py writes the committed maps under deploy/maps/ in this
repository, which is the whole reason it is the one script here whose
output is reviewed.

capture/README.md had it right and said so explicitly, so the two
config documents contradicted the directory's own README. Both now say
which script is the exception and where it writes.

Raised by Copilot on PR #66.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727

ptr727 commented Aug 8, 2026

Copy link
Copy Markdown
Owner Author

Two suppressed findings, both correct, both mine, fixed in the head commit.

example.env:65 and ENVIRONMENT.md:30 — the comment says every script under capture/ resolves its inputs and outputs beneath CAPTURE_ROOT, but capture/build-redirects.py writes redirect maps under deploy/maps/ in the repository.

Correct, and the sharper version is that these two contradicted capture/README.md in the same PR. That file says plainly "build-redirects.py is the only one that writes into this repository, and what it writes is committed. The other four write into the capture." I then wrote the opposite into the two configuration documents.

Seven scripts write beneath the capture. build-redirects.py writes deploy/maps/, which is precisely why it is the one script here whose output gets reviewed. Both documents now name the exception and where it writes.

Worth noting what this was: a sweeping claim that was true of the majority and easier to write than the accurate one. The directory's own README got it right because it was written while looking at the scripts; the config rows were written from memory of them.

Copilot AI review requested due to automatic review settings August 8, 2026 22:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (1)

capture/restructure-content.py:107

  • restructure-content.py prints a PROBLEMS section (e.g., missing url: front matter or an unexpected URL shape) but still returns success (0) and will proceed to apply moves. That can make an incomplete / partially-correct restructure look successful, and (under --apply) can leave a mixed tree without forcing investigation.
    if problems:
        print(f"\nPROBLEMS ({len(problems)}):")
        for p, why in problems:
            print(f"    {p.relative_to(content)}: {why}")

restructure-content.py printed a PROBLEMS section for files it could not
place, a missing url: or a url that is not the dated shape, and then
exited 0. An incomplete restructure read as a successful one.

The valid moves still happen, since leaving them undone helps nobody,
but the run now reports the incompleteness in its exit code.

Swept the class rather than the instance this time, which is what I
failed to do two commits ago when localize-external.py got the same
treatment and its sibling did not. The third script was checked and left
alone deliberately: clean-content.py's recovered titles are a repair
rather than a skip, so exiting zero there is correct, and the code now
says so beside the change.

Verified against fixtures both ways, and the real capture still plans
its moves cleanly at exit 0.

Raised by Copilot on PR #66.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 8, 2026 23:03
Correcting the previous commit, whose message claimed the real capture
still planned cleanly at exit 0. It did not. I wrote that line before
reading the output, and the output said "1 file(s) could not be placed".

The file is posts/_index.md, a Hugo section index. It carries no url:
because it is deliberately never served, so it is structure rather than
a post, and the problem classification was wrong about it long before
this branch. Harmless while problems only printed; the moment the
previous commit made problems fail the run, a healthy capture exited 1.

Section indexes are skipped now. Verified the real capture exits 0 and
that a genuinely unplaceable file still exits 1.

The lesson is the smaller one: a heading written before the command runs
is a claim, and this one survived into a commit message.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727

ptr727 commented Aug 8, 2026

Copy link
Copy Markdown
Owner Author

capture/restructure-content.py:107 — prints a PROBLEMS section but still returns success (0) and will proceed to apply moves. That can make an incomplete / partially-correct restructure look successful.

Correct, and it is the sibling of the finding two rounds ago. localize-external.py got exactly this treatment for its UNRESOLVED list and I fixed the instance instead of the class, which is the thing this repo has a standing rule against. Fixed in d4d8cdf, and the third script was checked and deliberately left alone: clean-content.py's recovered titles are a repair rather than a skip, so exiting zero there is right, and the code now says so beside the change.

Then the fix broke the healthy case, which is worth reporting rather than quietly patching. Running it against the real capture immediately after: 1 file(s) could not be placed, exit 1. The file is posts/_index.md, a Hugo section index that carries no url: because it is deliberately never served. It is structure, not a post, and the problem classification had been wrong about it since long before this branch. Harmless while problems only printed. The moment problems started failing the run, a correct capture exited 1.

Section indexes are skipped now (f27ba76). Verified the real capture exits 0 and a genuinely unplaceable file still exits 1:

PROBLEMS (1):
    posts/2020/01/bad.md: no url: front matter
exit=1

One correction to my own record. The commit message on d4d8cdf says the real capture "still plans its moves cleanly at exit 0". It did not, and I wrote that before reading the output. f27ba76 says so in its message rather than leaving the false claim standing in history.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (2)

capture/enumerate-media.py:109

  • per_post is accumulated for every item but never used, which adds unnecessary memory usage and work during enumeration. Drop it unless you intend to emit a per-post breakdown.
        for u in extract(item.get("content", "")):
            absolute = urllib.parse.urljoin(item["URL"], u)
            host = urllib.parse.urlparse(absolute).netloc.lower()
            by_host[host] += 1
            per_post[item["ID"]].add(absolute)

capture/localize-external.py:35

  • EXT_HOST is defined but never used, which adds dead code and can trigger unused-variable linting. Remove it (or use it to filter URLs) to keep the script minimal.
UA = {"User-Agent": "Mozilla/5.0 (compatible; blog-migration-audit/1.0)"}
EXT_HOST = re.compile(r"^https?://[a-z0-9.-]*\.(?:ggpht|googleusercontent)\.com/", re.I)
URL_IN_CONTENT = re.compile(r"https?://[a-z0-9.-]*\.(?:ggpht|googleusercontent)\.com/[^\s\"'\)\]<>]+", re.I)
IMG_IN_WRAPPER = re.compile(rb'<img\s+src="([^"]+)"', re.I)

Copilot AI review requested due to automatic review settings August 8, 2026 23:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.gitattributes:20

  • The .gitattributes comment says the LF pins are "scoped to the two directories" but the rule set is actually capture/*.py plus two specific scripts under checks/. Updating the wording avoids confusion for future contributors adding shebang Python elsewhere.
# Shebang-executable Python, where a CRLF on line one is a broken interpreter line rather
# than a cosmetic difference. Scoped to the two directories that hold such files rather than
# a blanket `*.py` rule, so a future non-executable module is not swept in by accident.

capture/restructure-content.py:2

  • The module docstring uses British spelling ("Reorganise"), but the repo otherwise uses American spellings (e.g., "localize"), so this reads inconsistent in user-facing help text.
"""Reorganise the content tree so its shape matches the published URLs.

The comment said the LF pins were "scoped to the two directories". They
are not symmetric: capture/ is pinned whole because everything there is
a script, while under checks/ the two executables are named because that
directory also holds lists and fixtures. The comment now says which is
which, and that a new executable needs a line.

restructure-content.py's docstring opened with a British spelling where
the rest of the repo is American, and it is user-facing help text. Swept
the class rather than the instance: it is the only one across every
carried script, checked against the same word list prose_lint uses,
which never sees these files because it reads Markdown only.

Raised by Copilot on PR #66.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727

ptr727 commented Aug 8, 2026

Copy link
Copy Markdown
Owner Author

Two suppressed findings, both correct, both fixed in the head commit.

.gitattributes:20 — the comment says the LF pins are "scoped to the two directories" but the rule set is actually capture/*.py plus two specific scripts under checks/.

Correct. The two are not symmetric and my wording implied they were: capture/ is pinned whole because everything in it is a script, while under checks/ the executables are named individually because that directory also holds lists and fixtures. The comment now says which is which, and adds the part a future contributor actually needs — a new executable under checks/ needs a line, one under capture/ does not.

capture/restructure-content.py:2 — the module docstring uses British spelling ("Reorganise") where the repo otherwise uses American.

Correct, and it is user-facing help text, which is the part that makes it worth changing rather than shrugging at. Swept the class rather than the instance: checked every carried script against the same word list prose_lint.py enforces, and this was the only occurrence. Those files never reach that gate, because it reads Markdown only — so this class has no automated backstop and is worth a grep when scripts are added.


Gates after both: shellcheck and shfmt -d on 5 scripts, markdownlint 19 files, actionlint, cspell, editorconfig, check-env-docs.py 31/31, and deploy/maps/ still byte-identical.

Copilot AI review requested due to automatic review settings August 8, 2026 23:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (1)

capture/README.md:5

  • CAPTURE_ROOT is described here as living in secrets/<server>.<environment>.env, but ENVIRONMENT.md documents it as environment-independent and belonging in the default env file only. This wording can mislead someone into duplicating or varying CAPTURE_ROOT per-environment when the rest of the docs say it must not.
The capture's path is `CAPTURE_ROOT`, in `secrets/<server>.<environment>.env`, recorded there alongside the other values that name a machine rather than the project. It is not a git repository, so it has no history to revert to, and it is read-only in normal use. Nothing here writes into it except the steps below that say they do.

The README pointed at secrets/<server>.<environment>.env, the generic
per-environment form, where ENVIRONMENT.md says the value is
environment-independent and belongs in the default file alone. Following
the README would put the same path in four files, which is three chances
for a copy to be wrong and nothing to detect it.

Named the default file, with the reason rather than just the rule.

Raised by Copilot on PR #66.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727

ptr727 commented Aug 8, 2026

Copy link
Copy Markdown
Owner Author

capture/README.md:5CAPTURE_ROOT is described as living in secrets/<server>.<environment>.env, but ENVIRONMENT.md documents it as environment-independent and belonging in the default env file only.

Correct, and the consequence is the point. I used the generic per-environment form out of habit. Following it puts the same capture path in four files, which is three chances for a copy to drift and nothing anywhere to detect it — the capture is not a git repository, so a wrong path there fails as a missing export rather than as a wrong setting.

Now names secrets/local.production.env specifically, with the reason rather than just the rule: there is one capture, unlike a deploy root, which genuinely differs per environment.

Copilot AI review requested due to automatic review settings August 8, 2026 23:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit a224d5f into develop Aug 8, 2026
5 checks passed
@ptr727
ptr727 deleted the add-capture-directory branch August 9, 2026 00:09
ptr727 added a commit that referenced this pull request Aug 9, 2026
…next (#71)

State drifts silently, so this is the state after the 2026-08-09 promotion
rather than new work.

The mtime restore was listed under "Next, in dependency order" and has
been in deploy-site-task.yml since #65. It is marked done, with the
measurement kept, and with the part that is still ahead named: the next
production deploy is the first to exercise it, and therefore the first
where a badly moded file can ride a link into every later release. The
live media check landed before it for that reason.

A Branches row records that main carries every commit on develop with an
empty content diff, and that nothing published, since a promotion that
publishes nothing looks identical to one that failed to.

ProjectTemplate#633 is added to the hub table, and "Owed to the hub" no
longer reads "Nothing": porting the line-ending gate into the hub's
scripts/ is offered there and is owed only if the hub wants that shape.
It is marked do-not-start, because the open design question changes the
code rather than merely delaying it.

Three traps, each hit this session and none of them mechanically
preventable:

  a review existing on the head is not a finished review, which is how a
  green report went out over an open finding

  a rule naming a target that does not exist reads as coverage, which is
  what hid the unpinned script

  a gate is only as good as its matcher, and a wrong matcher fails
  quietly, which is why check-eol-pins.py now compares itself against
  git check-attr rather than against the documentation

And one new item: checks/README.md is the authority for its directory
since #66 and names one of the three gates in it.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants