Clean up

[zmofei]

Summary

Cleanup

Test plan

• cargo test (all suites)

[mattpodwysocki]

The mechanics here are right, and I checked the part that a diff can't show: the specs really are derived, not hand-edited.

I ran strip.py from #168's branch against internal/openapi-raw/ and compared to this PR, file by file:

tokens-api.yaml                   identical
fonts.production.v1.yaml          identical
static-images.production.v1.yaml  identical
rastertiles.production.v1.yaml    identical
styles.production.v1.yaml         identical

api-rasterarrays/…v1.yaml   strip.py: "has no enabled operations — nothing to write"

That last line is exactly why the file is deleted and the SpecEntry goes with it — the sync script's own header predicts it ("a file whose every operation is disabled gets no oss/openapi/ file at all"). So the removal is correct output, not a hand edit that the next regenerate would revert.

Also confirmed on a build of this branch: fonts list no longer offers --fresh, tilesets get-mrt is gone (unrecognized subcommand, with get-tile/get-mvt suggested), and --schema reports neither. 603 tests across 12 suites, 0 failures, fmt and clippy clean. The fresh hits still in docs/commands.md are all refresh/"fresh" as an English word — I checked all 17.

Three things I'd change before this merges.

1. It can't merge before #168, and #168 is still a draft

This PR's spec changes only stay true while command-config.yaml says so. #168 is what makes getMrtTile disabled. If this lands first, the next --regenerate restores the rasterarrays spec and the command with it — the sync script warns about exactly that, and AGENTS.md says an edit to openapi/ "survives until the next regeneration and no longer."

Not a change to the diff, just an ordering constraint worth stating so nobody merges it out of sequence.

2. A command is being removed, and the changelog doesn't say so

+- Clean up.

mapbox tilesets get-mrt stops existing. Anyone scripting it gets unrecognized subcommand on upgrade, and "Clean up." gives them nothing to search for. The file's own preamble asks for "what it means for scripts that already use this tool", and 0.2.0 set the format:

• Breaking: nine more commands renamed, continuing #116's cleanup, and two dropped outright: (was/is table)

This wants the same: a ### Changed entry marked Breaking, naming mapbox tilesets get-mrt, saying it's gone rather than renamed, and giving #168's reason ("no real use; team agreed to drop it") — which is exactly what a reader hitting the error needs and can't otherwise find, since it's in a private repo.

The fresh parameter deserves a line too. It keeps working, so it's not breaking, but a caller passing --fresh today will find it missing from --help and --schema and will want to know that's deliberate.

Small structural thing: the bullet sits above the first ### Added, so it's outside the Added/Changed/Fixed convention the rest of the file uses.

3. The version bump belongs to prepare-release.sh

-version = "0.2.2"
+version = "0.3.0"

0.3.0 is the right number — a breaking change raises the minor pre-1.0, so that reasoning is sound. But that script's job is exactly this:

Renames ## Unreleased to ## <version> - <date> in oss/CHANGELOG.md, opens a fresh empty ## Unreleased above it, sets the version in oss/Cargo.toml…

Doing it by hand leaves Cargo.toml at 0.3.0 with no ## 0.3.0 section, so mapbox --version reports a release that hasn't been cut and the changelog's newest heading is still 0.2.2. The Breaking entry from point 2 is what makes the next release 0.3.0; the script then sets the number. I'd drop the bump from this PR.

And a note on the comment rewrite, which is most of the diff

Worth naming, because the title doesn't: of the ~960 changed lines in src/, this is ~930 comment lines and 8 lines of code.

src/executor.rs        comments: +435 -465    code: +4 -4
src/spec.rs            comments: +390 -400    code: +0 -4
tests/docs_contract.rs comments: +108 -112    code: +0 -0

Most of it is a fair style edit — shorter sentences, fewer clauses, and I'm not going to argue prose preference. But some passes drop specifics that were the point of the comment:

-/// Not a workaround for a hard case — a correction. `starFile` declares
-/// `application/json` with a `boolean` schema, and the service answers
-/// `400 Must be plaintext true or false` to exactly that. Sending the same
-/// `true` as `text/plain` returns 204. Verified with curl against
-/// production: `application/json` and no content type both fail, `text/plain`
-/// succeeds.
+/// `starFile`'s spec says `application/json` with a `boolean` schema, but
+/// the service rejects that with `400 Must be plaintext true or false`.
+/// Sending the same `true` as `text/plain` works (204). Verified with curl
+/// against production.

Two losses there. "Not a workaround — a correction" told you how to treat the table. And the old version said which three things were tried — application/json fails, no content type fails, text/plain succeeds — so the next person can re-verify or challenge it. "Verified with curl against production" asks them to take it on faith.

Same shape elsewhere: "right for the twenty JSON operations and wrong for the three that are not" became "right for most operations but wrong for three" — the count was checkable, "most" isn't. And "the media type the API actually requires" became "wants", which is weaker than what's true.

Not blocking, and not a request to revert the lot. My suggestion is to keep the measured specifics wherever the old comment had them: the counts, the tried-and-failed lists, the words that record a constraint rather than a preference. Those are the sentences AGENTS.md means by "a comment that records the failure a line prevents is what stops the next person removing it".

Separately, it would be much easier to review as its own PR. The spec regeneration is mechanically verifiable in a minute; the comment rewrite needs reading 930 lines of prose, and together they hide each other.

[mattpodwysocki]

Approving, so you're not blocked on me. Taking your three in order, and I'm dropping most of what I asked for.

1 — agreed, resolved. #168 is approved and clean.

3 — your call, and I'll stop asking. The version bump is process preference, not correctness. One consequence worth knowing rather than arguing about: with Cargo.toml at 0.3.0 and no ## 0.3.0 heading, dev-channel builds report a version that hasn't been released, and a later prepare-release.sh 0.2.3 would fail check-release-ready.sh against the tag. Both are recoverable and neither is worth a round trip.

And on fresh, I withdraw the ask entirely. It keeps working, so no script breaks — the only person affected is someone who reads --help and finds it gone, which isn't a break. You're right that it needs no changelog line.

2 — I'd still like one line, and I think it costs you nothing

The premise is the part I'd push back on. "Only used by the SDK and not something users would call directly" doesn't match what we published. From CHANGELOG.md on main, in 0.2.0's Breaking table:

| `mapbox rasterarrays get-mrt-tile` | `mapbox tilesets get-mrt` |

And docs/commands.md gave it a section, an index entry, and a worked example:

105:  [tilesets.get-mrt](#mapbox-tilesets-get-mrt) ·
2358: ### `mapbox tilesets get-mrt`
2370: mapbox tilesets get-mrt --jobid <jobid> <tileset-id> 12 2048 1361 > tile.mrt

That page is on docs.mapbox.com. So we announced the command by name in a release note and then documented how to run it. Whoever followed that is precisely the person who upgrades and gets unrecognized subcommand with nothing to search for.

The disclosure concern and the changelog line aren't actually in tension, which is the bit I'd like to land. Your ask is not to highlight the endpoint or its parameters — and a removal line needs to do neither:

### Changed

- **Breaking**: `mapbox tilesets get-mrt` has been removed.

No endpoint, no parameters, no rationale, nothing about raster arrays or MRT. Just a command name that has been in our published changelog since 0.2.0 and in our public docs ever since — so it reveals nothing that isn't already indexed. What it buys is that the one person who was using it can find out why in five seconds instead of filing an issue.

Compare the alternative: they hit an error, search the changelog for "get-mrt", find the 0.2.0 entry that added it and nothing since, and conclude it's a bug.

If you still disagree after that, go ahead — it's your and the Maps team's call, the reasoning is on the record here, and I'm not holding the PR for it. I'd just rather we didn't pay for the omission in a support thread later.

For the record, verified on this branch

• docs/commands.md is complete on its own terms: zero get-mrt mentions and the --fresh prose at old lines 824 and 1918 is gone too. I checked because a doc that still explains a hidden flag would defeat the point, and it doesn't.
• The specs are genuinely derived — strip.py with #168's config produces byte-identical output for all five changed files, and api-rasterarrays answers "has no enabled operations — nothing to write".
• 603 tests across 12 suites, fmt and clippy clean, --schema reports neither the command nor the parameter.

One last thing, in #168 rather than here: after this lands, api-rasterarrays/rasterarrays.production.v1.yaml becomes unwired-and-untriaged, so spec-drift.sh will exit 1 on the next pull_request run. One line in scripts/spec-drift-ignore heads it off — details on that PR.