Skip to content

ADFA-5171: Repair chunked Content rows misnumbered from -2 - #27

Merged
davidschachterADFA merged 1 commit into
ADFA-5153-content-brotli-dictionaryfrom
fix/ADFA-5171-fragment-renumbering
Aug 18, 2026
Merged

ADFA-5171: Repair chunked Content rows misnumbered from -2#27
davidschachterADFA merged 1 commit into
ADFA-5153-content-brotli-dictionaryfrom
fix/ADFA-5171-fragment-renumbering

Conversation

@davidschachterADFA

Copy link
Copy Markdown
Collaborator

Summary

  • WebServer.kt's fragment-reassembly loop always probes <path>-1 first, but 14 of 19 chunked Content rows in the shipped documentation.db number continuations starting at -2 with no -1 row — the lookup misses, the loop stops after the base 1 MB chunk, and the row is served short (silent truncation for compression='none', a decode failure/500 for 'brotli'). Confirmed against a local copy of the real database (md5 34c879595bd6fb87e5b68989369680a8).
  • No writer in this repo ever produced that numbering — populate_db.py, insert_optimized_media.py, and migrate_content_to_dictionary_brotli.py all go through insert_chunked_content, which has always started fragments at -1. This is inherited data older than the pipeline, not something re-running existing tools would fix.
  • Adds renumber_misnumbered_fragments.py: finds base rows whose fragment chain (via LIKE, sorted on the parsed numeric suffix rather than assumed paths) doesn't start at 1, and renumbers it to a contiguous -1, -2, ... run. A chain with a real gap (an actually missing chunk — a different failure) is reported and left untouched rather than guessed at. Only paths move, never content bytes, so it's safe regardless of a row's compression.
  • Based on ADFA-5153-content-brotli-dictionary since the script imports CHUNK_SIZE/backup_database from populate_db.py, which only exists on that branch.

Test plan

  • python3 -m unittest test_renumber_misnumbered_fragments -v — 6 new tests pass (misnumbered chain, single orphaned continuation, already-correct chain untouched, idempotent re-run, exact-1MB file with no continuation left alone, real gap reported and left alone).
  • Ran the script against a scratch copy of the real documentation.db: renumbered exactly 14 chains / 36 fragment rows, 0 gapped — matching the ticket's findings.
  • Simulated WebServer.kt's exact reassembly logic against the repaired copy: the devsite gif reassembles to the full 4,948,254 bytes, and j/html/api/index-all.html reassembles and brotli-decodes cleanly (previously 500/truncated).

🤖 Generated with Claude Code

WebServer.kt's reassembly loop always probes "<path>-1" first, but 14 of
19 chunked Content rows in the real documentation.db number their
continuations starting at "-2" instead, with no "-1" row at all. The
first lookup misses, the loop stops after the base 1 MB chunk, and the
row is served short: a corrupt image (compression='none', silent 200) or
a decode failure (compression='brotli', 500) - confirmed against a local
copy of the shipped database (md5 34c879595bd6fb87e5b68989369680a8).
No writer in this tool ever produced that numbering - populate_db.py,
insert_optimized_media.py, and migrate_content_to_dictionary_brotli.py
all go through insert_chunked_content, which has always started
fragments at -1. This is inherited data older than this pipeline, not
something it can regenerate correctly by re-running existing tools.
renumber_misnumbered_fragments.py finds base rows whose fragment chain
(via LIKE, sorted on the parsed numeric suffix rather than assumed
paths) doesn't start at 1, and renumbers it to a contiguous run starting
at -1, lowest-suffix first so each rename's target is the path just
vacated by the previous one. A chain with an actual gap (a genuinely
missing chunk, a different failure) is reported and left alone rather
than guessed at. Content bytes are never touched, only paths, so it's
safe regardless of a row's compression. Verified against a scratch copy
of the real database: renumbers exactly the 14 chains the ticket found,
and the two example rows (the devsite gif, the Javadoc index) reassemble
and decode correctly afterward.
@davidschachterADFA
davidschachterADFA merged commit 0599a37 into ADFA-5153-content-brotli-dictionaryAug 18, 2026
@davidschachterADFA
davidschachterADFA deleted the fix/ADFA-5171-fragment-renumbering branch August 18, 2026 15:35
alexmmiller pushed a commit that referenced this pull request Aug 25, 2026
Validated against ~/documentation.db (schema 2.0.0), now the source of truth.
populate_db.py
- A failed .md conversion left its stem in topic_index_db, so nav rendered
an ordinary, normally-styled link to a page the run had just deleted and
not replaced. Drop the stem (matching the blacklist path, so references
render as styled-broken) and refuse to modify the database at all unless
--allow-conversion-failures is passed - CI uploads this database straight
to production.
- Two same-stem .md files in different topics/ subdirectories both mapped to
k/html/<stem>, colliding on Content.path's UNIQUE constraint and aborting
the transaction mid-run. Defer to the keep-first choice build_topic_index
already makes and warns about.
- image_index_db keyed on the full zip entry name while Converter looks
images up by bare filename, so any nested zip entry would silently resolve
as a missing image. Key on the basename, matching Converter and
insert_optimized_media.py's own flattening, and warn on collisions.
insert_optimized_media.py
- delete_unreferenced_media deleted every image no page referenced, with no
floor check: run against a database whose k/html pages don't exist yet and
it wiped the entire image corpus, including rows inserted seconds earlier
in the same transaction. Raise instead when images are stored but nothing
references any of them, and document that CSS/template references are not
scanned.
- Added --dry-run (the most destructive of the three scripts was the only
one without one): does the whole run, then rolls back.
- Moved the renamed-away delete loop above the insert loop. With inserts
first, a rename whose new name equals another rename's old name deleted
the row just written - the chain-rename hazard rewrite_pages already
guards against for text substitution.
- delete_content built a LIKE pattern from a path without escaping, so "_"
and "%" acted as wildcards; NAV_CONTENT_PATH ("k/html/_nav.html") already
contains one. Escape via a new like_escape() and ESCAPE '\'.
sync_kdoc_json_to_db.py
- Wrote plain Brotli into a database whose every brotli row is compressed
against the shared CompressionDictionary (schema 2.0.0, ADFA-5153),
producing content the server cannot decode. Read the dictionary and
compress against it, falling back to plain Brotli only for older
databases; never create or retrain one. Needs the brotli CLI, now
installed in both workflows.
- Ignored the CHUNK_SIZE fragmentation contract: UPDATEd the full blob into
one row and deleted existing fragments individually. Split oversized
results into "<path>-N" continuations the way populate_db.py does, and
treat existing fragments as part of their base row.
- An unresolvable contentTypeID fell back to "uncompressed" and committed,
writing bytes that contradict the row's declared type. Now fatal.
- Backup used shutil.copy2; switched to VACUUM INTO, matching the other two
scripts and safe against a live database.
Also: corrected the now-stale claims that documentation.db ships without an
image/webp ContentTypes row (it has one, id 26) and that scour/cairosvg are
absent from requirements.txt; gitignored the timestamped *.db.backup-*/
*.db.bak.* files the three scripts write.
CLAUDE.md records the one review finding NOT fixed here: populate_db.py and
insert_optimized_media.py are still plain-Brotli and so broken against a
2.0.0 database. That fix already exists on fix/ADFA-4737 via merged PRs #26
and #27; reconciling with that branch is the right way to pick it up rather
than hand-porting it into a conflict.
Adds 30 regression tests covering each fix, including a dictionary
round-trip. Verified end-to-end on a copy of ~/documentation.db: 3,238 rows
rewritten, 12/12 sampled rows decode against the dictionary, untouched rows
unaffected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
alexmmiller pushed a commit that referenced this pull request Aug 25, 2026
…line)
Brings in the ADFA-5153/ADFA-5171 work merged to fix/ADFA-4737 via PRs #26
and #27, which this branch forked from #21 too early to receive. Without it
the pipeline cannot run against the current production database at all:
~/documentation.db is schema 2.0.0, every "brotli" Content row is compressed
against the shared 256 KiB raw LZ77 dictionary in CompressionDictionary, and
plain Brotli cannot decode any of it (measured: 0 of 24 sampled rows).
Conflict resolution - all twelve were add/add, so each was decided per file
rather than 3-way merged:
Took theirs (the dictionary lineage is strictly ahead on these three), then
re-applied this branch's review fixes on top:
- populate_db.py: DictionaryCompressor, train/load_or_create_dictionary,
fragment_chain, page_size pinning. Re-applied the conversion-failure
abort, the same-stem dedupe, and the basename-keyed image index.
- insert_optimized_media.py: dictionary-aware reads/writes. Re-applied the
delete_unreferenced_media floor check, the delete-before-insert ordering,
and --dry-run.
- sync_kdoc_json_to_db.py: DictionaryBrotli, load_compression_dictionary,
MAX_DELETE_FRACTION. Re-applied CHUNK_SIZE fragmentation, the fatal
unknown-contentTypeID, and the VACUUM INTO backup.
Took ours (PR #23/#24 refined these after the split): md_to_json.py,
find_missing_assets.py, optimize_media.py, assets/docs.css, README.md,
run_e2e_pipeline_test.sh, .gitignore.
Hand-merged: build-kotlin-docs.yaml (our corrected requirements/webp comments
plus their brotli-CLI rationale); CLAUDE.md (ours, with the 2.0.0 blocker note
rewritten as a description of how the three writers now handle the dictionary,
since the merge resolves it).
Two of this branch's own fixes were dropped as superseded:
- like_escape/ESCAPE '\' is replaced by fragment_chain, which does the
over-matching LIKE once and re-checks each candidate's digit suffix. That
also handles ADFA-5171 chains numbered from -2, which escaping does not.
sync_kdoc_json_to_db.fragment_paths was rewritten to match rather than
probing "-1" and stopping at the first gap.
- The hand-rolled DictionaryCompressor added to the sync script last commit
is replaced by theirs.
Tests updated for the merged APIs (collect_referenced_media and
delete_unreferenced_media now take a compressor; DictionaryBrotli is
compress-only, so its tests decode through the brotli CLI). 105 pass: 78 in
ProcessKotlinWebsiteJSON, 27 in scripts/sync_kotlin_stdlib_docs.
Verified against a copy of ~/documentation.db: 3,238 stdlib rows rewritten,
12/12 sampled decode against the dictionary, untouched trees unaffected, row
count unchanged at 30,649.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@davidschachterADFA