Skip to content

Collapse dl's duplicate indent=2 JSON formatter onto core's #346

Closed

Description

@blooop

Question

A third copy of the ensure_ascii + surrogate-pair escaping survives #315 in the dl renderer's pretty formatter for --ls --json. That build collapsed the other two (core's json.rs and the metadata store's) into one shared write_ensure_ascii, but left this one deliberately: sharing it needs a new pub item on devlaunch-core, which moves a public-API snapshot — off-limits until Split the API snapshots lands so the snapshot files move once.

Scope it as the whole formatter, not just the escaper. The review of PR #345 measured the actual residue: render.rs:196-295 and metadata.rs:1015+ are two near-identical ~100-line indent=2 formatters that each delegate to serde_json::ser::PrettyFormatter, so the escaping is only the smaller half of what is duplicated. Exporting one pub formatter (or one pub document-writing fn) costs the same single pub line the escaper alone would have cost, and retires the duplicated layout delegation as well. PythonPretty has exactly one production caller — python_json_document at render.rs:180, reached only from commands.rs:225 — so the surface being replaced is small and fully known.

Red-first at the shared seam, and add the pin the other two got. Same principle as #315: a legitimate difference stays (this document is indented, the compact one is on one line); what must not stay is two copies of one fact.

Note the ordering trap: doing this before #338 merges would move a public-API snapshot twice, which is exactly what that ticket exists to prevent.

Activity

  1. changed the title [-]Collapse the third ensure_ascii copy in dl's --ls --json formatter[/-] [+]Collapse dl's duplicate indent=2 JSON formatter onto core's[/+] on Aug 22, 2026
  2. blooop commented on Aug 22, 2026

    @blooop
    OwnerAuthor

    breadcrumb: scope widened before any work started, on the reviewer's measurement in PR #345.

    agent-decided (simplicity): this ticket now collapses the whole duplicated indent=2 formatter, not just the escaping half. The escaper was the visible duplicate, but the measured residue is two near-identical ~100-line delegating formatters — so single-copying only the escaping would leave the larger duplicate standing while spending the same one pub line and the same snapshot move. Retiring both for that price is strictly better, and PythonPretty's single production caller makes the replaced surface small and fully known.

    Cheap to reverse (it narrows back to the escaper at any time), so not worth a human question. Title updated to match.

  3. blooop commented on Aug 29, 2026

    @blooop
    OwnerAuthor

    Claimed 2026-08-29: queued in the session working map #299's build tickets (waves behind #340/#349's PRs so the api surface and shared JSON writer move once). Build to follow from an opus agent in an isolated worktree. If you are another session seeing this, do not double-build.

  4. self-assigned this
    on Aug 29, 2026
  5. blooop commented on Aug 29, 2026

    @blooop
    OwnerAuthor

    breadcrumb: claimed and building. Stacked on the DEL-escaping PR (#504, branch wayfinder/devlaunch-349), not on main: that PR touches both copies of this escaping, so branching off main would rebuild the divergence it just closed. The PR will open with #504 as its base and be retargeted when #504 merges.

    What gets built. Core's JSON module already owns the compact json.dumps spelling and the escaping both documents share. It gains the indented spelling as well: the indent=2 formatter moves down out of the metadata store into that module, and one new pub document function over a JSON value sits beside the compact one already there. The renderer's --ls --json document then goes through it, and the renderer's own copy of the formatter, layout delegation and escaping loop alike, is deleted.

    Seams to test at, both named by the ticket:

    • Core's new indented document function, at its own boundary: the spelling of the whole ASCII range, the surrogate pair an astral character becomes, and DEL, held to the literals CPython printed. This is the seam the renderer's copy is being retired onto, so it has to carry the pins before it carries the caller.
    • The renderer's --ls --json document, unchanged as a boundary: the caller and the tests keep naming the same entry point, so every literal that pinned the deleted copy stays exactly where it is and now pins the collapsed path instead. That is the byte-identity proof, and it is why the pins go in before the deletion rather than after: a sweep literal that survives having its implementation swapped underneath it is a differential test with the two copies separated in time rather than in space.

    Not in scope, deliberately: the compact and the indented documents stay two spellings. The ticket keeps that split as the legitimate difference, exactly as #315 did.

    One thing about the snapshot. The new pub line lands in the freely regenerated tripwire file, not the frozen promise, because the split ticket landed first. Regenerating it needs a nightly toolchain this container does not carry, so the row is placed by hand in the order the generator emits and CI's snapshot job is what confirms the placement.

  6. blooop commented on Aug 29, 2026

    @blooop
    OwnerAuthor

    breadcrumb: built. PR #522 — #522

    Stacking: opened against the DEL-escaping PR (#504), not main, because that PR touches both copies of this escaping and branching off main would have rebuilt the divergence it just closed. Retarget to main when #504 merges. Its base moved mid-build (main merged in, a release cut), so this branch was rebased onto the new head and the changelog entry moved back under the now-empty [Unreleased] by hand: a rebase places it textually and put it inside the released section.

    The slice. The indented spelling lives in core's JSON module beside the compact one, exported as one pub document function; the renderer calls it and its own formatter is deleted. The metadata store keeps going through the formatter directly, because its document is a struct and routing it through a JSON value first would leave its field order to the map implementation.

    Byte-identity, proven twice. The pins on --ls --json never moved: the diff over the renderer contains no changed line carrying an assertion or a literal, so the sweep that went in to hold the two copies equal now measures the collapse instead. And the deleted formatter was reinstated beside the collapsed path and both run over every Unicode scalar value plus sixteen adversarial shapes — 1,112,080 documents, no mismatches.

    One thing reading the two copies would not have told you: they disagreed on how many methods the job takes. The renderer's forwarded one layout method core's does not, and it made no difference, because serde's pretty printer does not override that one either — the extra delegation forwarded the trait's own no-op. Identical bytes out of two spellings that were not the same spelling, which is what a second copy quietly accumulates.

    Ticket claims that did not survive contact: the ordering trap it warns about is spent. The snapshot split it says this waits on has merged, so the one new pub line lands in the freely regenerated tripwire file rather than the frozen promise, and there is nothing to sequence around. Its line references were stale in both files and were not used.

  7. blooop commented on Aug 29, 2026

    @blooop
    OwnerAuthor

    Two-axis review posted on #522: Comment — approve on substance, one mechanical blocker.

    Byte-identity independently reconstructed (2,224,151 documents old-vs-new, plus 129,099 against CPython json.dumps(indent=2), 0 mismatches both ways); end_object_key divergence confirmed immaterial against the pinned serde_json source. Blocker is a CHANGELOG.md conflict that appeared mid-review when main moved to 6df02bc — the #527 shape, resolution spelled out in the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions