ADFA-4739: Pipeline for producing template-based Kotlin documentation

[alexmmiller]

Summary

An end-to-end pipeline for producing templated Kotlin documentation.

Changes

Kotlin website docs

• Adds Python scripts used to convert kotlin-web-site/docs (Writerside Markdown) into JSON, build nav, optimize media, and insert everything into documentation.db, into ProcessDocs/ProcessKotlinDocs/ProcessKotlinWebsiteJSON/. Adds a README documenting each script's purpose, required inputs (kr.tree, webHelpImages.zip, config JSON), and end-to-end usage — including populate_db.py --blacklisted-element-titles, the mechanism this ticket needs for pruning docs sections (e.g. Kotlin/Wasm) we don't want to ship.

Kotlin-stdlib docs

• Adds a sync_kdoc_json_to_db.py script into a new directory scripts/sync_kotlin_stdlib_docs/, for syncing generated kotlin-stdlib docs against the database.

Templated docs pipeline

• Add ProcessDocs/ProcessKotlinDocs/run_e2e_pipeline_test.sh which builds the kotlin-stdlib and Kotlin website JSON documentation and inserts it into CoGo's documentation database.

Test plan for Kotlin website docs:

• Reviewer confirms the new scripts match their originals in /home/alex/ADFA/KotlinWebsiteProcess/ and Dokka-plugin-kdoc2json/scripts/ (no edits were made during the copy)
• README instructions are accurate for running populate_db.py with --blacklisted-element-titles against a scratch database

Note on KDoc-to-JSON Dokka plugin version

We currently include a stale copy of the KDoc-to-JSON Dokka plugin while that work is still being done this PR. Once those changes are merged into main we'll merge them into this branch.

[davidschachterADFA]

Claude says:

Issues found

1. README is out of sync with what's actually in the PR.
   The README says (lines 32–35):

per ADFA-4737, only the .py files from the original working directory were copied here. templates/ and assets/ ... must be placed alongside these scripts before running populate_db.py.

But the PR itself does include templates/{page.peb,nav.peb,nav.html}, assets/{docs.css,tabs.js,sidebar.js}, and config.json (added by the "Config/assets for pipeline test" commit). A reviewer or new user following the README literally would go hunting for files that are already sitting right there. Worth updating that note (or clarifying it only applied to the original copy-over before the pipeline-test commit added them).

2. sync_kdoc_json_to_db.py defaults --db to a personal path.

python
parser.add_argument("--db", default="/home/alex/documentation.db", ...)

This is clearly a leftover from the author's local machine. It's harmless in that a wrong path just fails the os.path.isfile check, but it shouldn't ship as the default in a shared repo — either make it required (no default) or default to documentation.db in cwd, matching populate_db.py's convention.

3. Possible chain-rename bug in insert_optimized_media.py's rewrite_pages.
   rewrite_pages iterates rename_map.items() and does sequential str.replace calls on the same new_text buffer:

python
for old_name, new_name in rename_map.items():
...
new_text = new_text.replace(old_ref, new_ref)

If one rename's new_name happens to equal another rename's old_name (e.g. foo.png → foo.webp from one optimization, and some unrelated foo.webp → foo-2.webp renamed elsewhere), the second substitution could re-match text that the first substitution just wrote, double-rewriting a reference that was never actually pointing at the second file. It's a narrow edge case (requires two renames whose old/new basenames chain), but since build_rename_map only guards against the same old name mapping to two different new names, not this chain scenario, it's worth at least a comment acknowledging the assumption (or building a single combined regex/dict-based single-pass substitution instead of N sequential replaces).

4. Minor: sync_kdoc_json_to_db.py doesn't explicitly BEGIN the way the other two DB-writing scripts do (conn.execute("BEGIN")). Python's sqlite3 module will still implicitly open a transaction before the first DML statement with the default isolation level, so this isn't a correctness bug, but it's an inconsistency with the explicit-transaction style used elsewhere in the same PR — could confuse a future reader into thinking it's unguarded.

[alexmmiller]

e2e_run_output.log
I attached the log from running the updated pipeline in the most recent commit.

Summary:

== Summary ==
Database: /Users/alex/test_db_assets/documentation.test.db
k/html/* rows: 502
k/html/images/* rows: 235
k/html/images/.webp rows: 161
k/kotlin-stdlib/ rows: 3132
k/kotlin-reflect/* rows: 65
k/kotlin-test/* rows: 41

== Blacklist pruning verification ==
Blacklisted toc-element path(s) checked: 3
Development > Web development: matched
Interoperability > JavaScript interop: matched
Interoperability > Swift/Objective-C and C interop: matched
Topic page(s) expected removed: 38
PASS: all 38 blacklisted topic page(s) confirmed absent from /Users/alex/test_db_assets/documentation.test.db.

Done. Backups (populate_db.py, insert_optimized_media.py, and sync_kdoc_json_to_db.py
each make their own) live alongside /Users/alex/test_db_assets/documentation.test.db as documentation.test.db.backup-* and
documentation.test.db.bak.*
The real database at /Users/alex/test_db_assets/documentation.db was never opened for writing.
EXIT_CODE=0

[hal-eisen-adfa]

Deep review of the new pipeline. 15 findings, left inline. Six were reproduced by actually running the converter/scripts locally; the rest are traced through the code but not triggered.

Blocking — please address before merge:

1. sync_kdoc_json_to_db.py — orphan cleanup deletes hand-authored tooltip text that exists nowhere else, and the new workflow now points it at the production DB with dry_run gating only the upload, not the mutation.
2. populate_db.py — page conversion failures are caught, printed, and the process still exits 0, so a partially-converted DB publishes as if clean.
3. md_to_json.py — TAG_RE matches <table> as a <tab> container and destroys every raw HTML table in the source. This one is already costing us: the workaround comment in templates/page.peb:128-132 is describing this bug's symptom.
4. build-kotlin-docs.yaml — the WIF auth step is missing the drive.readonly scope, so both Drive downloads will fail on the first real run.

The rest are ordinary bugs, papercuts and one efficiency note — worth fixing but not all of them need to block. Happy to pair on any of these.

Overall the pipeline structure and the e2e test script are a nice piece of work; the findings are concentrated in the Writerside-tag parsing and in the CI wiring, not in the design.

[hal-eisen-adfa]

Blocking — unrecoverable data loss against the production DB.

A single dead TooltipButtons.uri takes out the entire parent Tooltips record — the hand-authored summary/detail plus every other, still-valid button hanging off it.

Failure path: a kotlin-stdlib symbol gets renamed upstream, so k/kotlin-stdlib/kotlin.text/old-name/index.html has no matching JSON and its Content row is deleted. cleanup_orphaned_tooltips then sees one orphaned button URI and deletes the whole tooltip. That tooltip text is not in the plugin output and cannot be regenerated — it is only recoverable from the .bak. file.

Two things make this sharp rather than theoretical:

• build-kotlin-docs.yaml runs step 5/5 unconditionally (no skip_website_docs guard) against the real documentation.db.
• dry_run gates only the Drive upload, not this mutation — so a "dry run" still destroys tooltips in the downloaded DB, and a subsequent non-dry run publishes that.

Suggested fix: only delete a Tooltips row when all of its buttons are orphaned, and drop individual dead TooltipButtons rows otherwise. Even then, please log every tooltip id being deleted, and consider requiring an explicit --prune-tooltips flag so the destructive path is opt-in rather than a side effect of a docs rebuild.

[hal-eisen-adfa]

Blocking — silent failure: broken DB publishes with exit 0.

A page that fails to convert is logged to stderr and skipped, but the process still exits 0. (Same pattern at md_to_json.py:607.)

If 40 topics hit an unhandled markdown-it edge case, this prints Converted 960/1000 pages to stderr and exits clean. The workflow then sails through the blacklist verification — which only checks that blacklisted pages are absent, not that expected pages are present — uploads the artifact, and with dry_run: false overwrites the production database with 40 pages missing and 40 dangling nav/prev-next links pointing at them.

Nothing anywhere in the pipeline fails on a non-zero skip count.

Suggested fix: count the skips and exit non-zero at the end if any occurred, with an explicit --allow-conversion-failures N escape hatch if you want tolerance for a known-bad page:

failed = []
# ... in the except: failed.append((md_path, exc))
if failed:
    print(f"{len(failed)} page(s) failed to convert:", file=sys.stderr)
    for p, e in failed:
        print(f"  {p}: {e}", file=sys.stderr)
    if len(failed) > args.allow_conversion_failures:
        sys.exit(1)

[hal-eisen-adfa]

Blocking — this silently destroys every raw HTML table in the docs.

There's no word boundary after the tag alternation, so <table> / </table> match as tab container markers.

Reproduced by running the converter:

input:  <table>\n<tr><td>Kotlin 2.0</td><td>Supported</td></tr>\n</table>
output: [{"type":"tab","attrs":{},"blocks":[{"type":"html","html":"<tr><td>Kotlin 2.0</td>..."}]}]

The <table>/</table> lines are swallowed, and page.peb's b.type == "tab" branch then emits bare <tr>/<td> with no table wrapper — which browsers discard outright. <table class="x"> matches too, yielding garbage attrs parsed from le class="x".

This module's own docstring notes that pages like roadmap.md are "basically hand-written HTML tables", so this is hitting real content. And the comment at templates/page.peb:128-132 ("A lone <tab> not wrapped in <tabs> (e.g. seen in eap.json's HTML-table compatibility layout)") is this bug's symptom being papered over in the template rather than fixed here.

Suggested fix — add \b, and while you're here make the self-closing case explicit (see my comment on line 396):

TAG_RE = re.compile(r"^<(/?)(tabs|tab|note|tip|warning)\b([^>]*?)(/?)>$", re.I)

With \b, <table> no longer matches (tab can't be followed by l), while <tab>, <tab id="x"> and <tabs> all still do. Worth adding a regression test for <table> and <tabs> side by side.

[hal-eisen-adfa]

Blocking — both Drive downloads will 403/404 on the first real run.

drive.file only grants access to files the service account itself created. It cannot read a pre-existing Drive file, which is exactly what check-tools/download_database.py does here (service.files().get(fileId=...) / get_media).

Compare docdb-regression-test.yaml:46-48, which downloads the same GOOGLE_DRIVE_FILE_ID via the same script and requests both scopes. This workflow dropped drive.readonly, so "Download current documentation.db" and "Download Writerside image export" will both fail.

Suggested change

	access_token_scopes: |
	https://www.googleapis.com/auth/drive.file
	access_token_scopes: |
	https://www.googleapis.com/auth/drive.readonly
	https://www.googleapis.com/auth/drive.file

[hal-eisen-adfa]

A filename collision upstream takes down the whole run.

Page ids are flattened to the bare .md stem with no collision handling, and Content.path has an enforced UNIQUE constraint (verified: sqlite_autoindex_Content_1).

If kotlin-web-site ever adds e.g. topics/tour/overview.md alongside topics/native/overview.md, both produce db_id = "k/html/overview". The second INSERT at line 550 raises sqlite3.IntegrityError: UNIQUE constraint failed: Content.path, the transaction rolls back, and the CI job dies with a traceback.

The docstring asserts that "filenames are already unique across the whole topics/ tree" — but that's a third-party repo that changes weekly, so it's an assumption with an expiry date rather than an invariant. Note that the neighbouring code already takes this more seriously: md_to_json.build_topic_index tolerates collisions via setdefault, and build_image_index explicitly warns on the analogous image case. This path does neither.

At minimum, detect the collision and fail with a clear message naming both files, rather than letting it surface as an opaque IntegrityError 70 lines later. Better still, derive the id from the path relative to topics/ so it can't collide.

[hal-eisen-adfa]

Walking work_dir instead of this run's manifest lets stale files from a previous run into the DB.

work_dir is only auto-removed when it's the temp fallback (work_dir_is_temp). When passed as the third positional argument it's mkdir(exist_ok=True)'d and never cleaned — so a reused --work-dir accumulates.

Two concrete misbehaviours:

• Run once with --webp (produces mascot.webp), then re-run without --webp into the same dir: this rglob("*") now finds both mascot.png and the stale mascot.webp, and inserts both.
• A media file deleted from media_dir between runs still sits in work_dir and gets re-inserted — resurrecting content this run was supposed to drop.

The rename_map is built only from this run's manifest, so it knows nothing about either case.

Fix: iterate the manifest (which is already the authoritative record of what this run produced) rather than the directory listing. If you'd rather keep the rglob, clear work_dir at the start of every run.

Minor, but worth noting: sorted(...) over the whole tree also means the fix is cheap either way.

[hal-eisen-adfa]

Emitting a second style attribute instead of merging — the coloring is silently dropped.

Reproduced:

input:  <p><a href="https://example.com" style="font-weight:bold">ext</a></p>
output: <a href="https://example.com" style="font-weight:bold" style="color: #cc0000;">

HTML parsers keep the first style and discard duplicates, so the broken/external-link indicator that config.json's broken-ext-link-color exists to provide never renders — and it fails specifically on hand-authored HTML passthrough links, which are the ones most likely to already carry an inline style.

Merge into the existing attribute if present (append ;color: ... to its value), and only append a fresh style= when there isn't one.

[hal-eisen-adfa]

The inputs are documented as accepting a commit SHA, but git clone --branch doesn't take one.

Both input descriptions say "Branch/tag/commit of ...". Passing a SHA gives:

git clone --depth 1 --branch <sha> https://github.com/JetBrains/kotlin.git
fatal: Remote branch <sha> not found in upstream origin

Same issue on the kotlin_ref clone below — whose description explicitly frames pinning as the route to "a reproducible build", i.e. precisely the case that fails.

Either fix the docs to say "branch or tag", or support SHAs properly:

git init kotlin-web-site && cd kotlin-web-site
git remote add origin https://github.com/JetBrains/kotlin-web-site.git
git fetch --depth 1 origin "$KOTLIN_WEB_SITE_REF"
git checkout FETCH_HEAD

Given reproducibility is the stated goal, I'd lean toward supporting SHAs.

[hal-eisen-adfa]

Unpinned bare pip install, and requirements.txt was never updated with the new dependencies.

Two separate problems here:

1. Missing deps. markdown-it-py, scour and cairosvg are absent from requirements.txt — they're only installed by the ad-hoc pip install on line 155. Confirmed locally: after a plain pip install -r requirements.txt, import md_to_json fails with ModuleNotFoundError: No module named 'markdown_it'. So anyone following README.md outside CI gets a broken checkout. These belong in requirements.txt.

2. No pinning, and inconsistent with the rest of the repo. Nothing is version-pinned and there's no lockfile, so an upstream markdown-it-py major bump breaks the build with nothing to roll back to. The repo's other workflows already use uv — docdb-regression-test.yaml uses uv sync/uv run, publish-doc-db.yaml uses uv venv/uv run. This step should match them rather than introducing a third dependency-management style into the same repo.

[hal-eisen-adfa]

Non-blocking / efficiency: this entire pngquant pass is thrown away by the very next pipeline step.

Both run_e2e_pipeline_test.sh (steps 2-3) and build-kotlin-docs.yaml (steps 2-3) unzip the same webHelpImages.zip twice. Here, insert_file pngquants and brotli-compresses each PNG into k/html/images/<name>; then insert_optimized_media.insert_optimized_file calls delete_content and re-inserts the re-optimized bytes at the same path.

With --webp it's not even an overwrite — the step-2 .png rows are orphaned and then swept up by delete_unreferenced_media.

On a doc set of ~1000 images that's ~1000 discarded pngquant subprocess invocations plus a full brotli pass, every run. Skipping media entirely in step 2 when step 3 is going to run should be a straight win on CI wall-clock.