Skip to content

docs(crate): ENG-4683 README, axum quickstart, and architecture diagram - #20

Draft
jimbofreedman wants to merge 16 commits into
mainfrom
worktree-ENG-4683
Draft

docs(crate): ENG-4683 README, axum quickstart, and architecture diagram#20
jimbofreedman wants to merge 16 commits into
mainfrom
worktree-ENG-4683

Conversation

@jimbofreedman

@jimbofreedman jimbofreedman commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Turns README.md from a repository-skeleton note into an OSS-facing
document, and — more importantly — makes it executable so it cannot
rot against the API. Closes ClickUp ENG-4683.

The README now carries the eleven sections the ticket asked for: badges,
the pitch, a pre-1.0 status callout, a library quickstart, a ~50-line
axum HTTP server, the build-pipeline diagram, the feature/size table,
the 13 edge groups, performance, licence/attribution, and contributing.
The existing CLI section is preserved (see Deliberate deviations).

What makes it more than a docs change: nothing in the README was
compiled by anything before this PR
. A #[cfg(doctest)] anchor in
src/lib.rs now feeds the README's fences to cargo test --doc, so the
library quickstart is compiled and executed on every CI run, and
examples/axum_server.rs is the single source of truth for the HTTP
block — the README embeds the same bytes and a test asserts they are
byte-identical.

Every factual claim is machine-checked or measured. The 13-group table
is asserted element-for-element against the generated
rustyroute::EDGE_GROUPS (names and their upstream pass tags, in
order); the menaiStrait bbox is asserted against build/groups.rs;
the section inventory is asserted as an ordered vector. The archive
sizes, node/edge counts and latency figures were measured on this
worktree, not estimated, and budgets are kept separate from observations.

Deliberate deviations from the ticket

  1. The CLI section is kept. The ticket says the README "replaces the
    current placeholder" and lists eleven sections, none of them CLI. But
    that section documents shipped ENG-4682 behaviour and I re-verified it
    is still accurate (flags, defaults, exit codes 0–4). Deleting correct
    user-facing docs to satisfy a literal reading would be a regression.
    Easy to drop if you disagree.
  2. tokio gets a net feature the ticket didn't list. The ticket
    specifies features = ["macros", "rt-multi-thread"], which cannot
    compile the example: axum::serve takes a tokio::net::TcpListener,
    gated behind net. axum's own tokio feature would supply it
    transitively, but the example names the path directly.
  3. The example reads HOST and PORT (default 127.0.0.1:3000).
    PORT=0 lets the E2E tests boot it on an OS-assigned port instead of
    racing for one; loopback-by-default came out of Copilot review — this
    is code people paste, and it should not expose a routing service on
    every interface because someone tried the quickstart. HOST=0.0.0.0
    opts in for containers.
  4. The README depends on rustyroute by git, not cargo add rustyroute.
    The crate is not on crates.io yet, so the plain command would fail —
    also from Copilot review. The "0.1" form is noted for after the
    first release.
  5. .gitignore now covers tests/downstream_consumer/target/. The
    root /target/ entry is anchored, so reproducing AC3 locally leaves
    ~300 MB untracked. I hit exactly that.

Acceptance criteria

AC Status How it is verified
Example boots; curl returns GeoJSON in [lng, lat] tests/axum_example_e2e.rs spawns the real binary over a real socket
README snippet compiles under cargo test --doc it compiles and runs; bypass via rust,ignore is now blocked
Clean external project gets a valid Route downstream_consumer sub-package, plus a fresh out-of-tree crate in QA
13 README groups match EDGE_GROUPS, asserted by a test ordered whole-vector assert_eq! against the generated constant

The README's stated quickstart output (16354.1 km over 106 points) is
now asserted too — readme_quickstart_output_matches_reality parses the
numbers out of the README and runs the same route to check them.

Verification

All gates CI runs, run locally, all green:

cargo fmt --all --check                                        ✅
cargo fmt --manifest-path tests/downstream_consumer/… --check  ✅
cargo clippy --all-targets --all-features -- -D warnings       ✅
cargo test --all-features                    ✅ 32 groups, 0 failed
cargo test --doc                             ✅ 3 doctests (2 run, 1 ignored)
RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features  ✅
cargo build --target wasm32-unknown-unknown --no-default-features  ✅
cargo deny check        ✅ advisories, bans, licenses, sources all ok
pre-commit run --all-files                   ✅ no tree drift
cargo package --allow-dirty --list           ✅ ships README + example

Live evidence from a running server (release build):

first position: [5.1927490234375, 43.230220794677734]   ← longitude first
positions: 106   distance_km: 16354.07348421216   resolution: 50
suezCanal blocked: 25047.47884941101 km             ← round the Cape
self-route:        2 positions                      ← RFC 7946 §3.1.4

A fresh crate outside the repo, default features only, running the
README snippet pasted verbatim, prints 16354.1 km over 106 points
exactly what the README claims.

Notes for the reviewer

Two defects were found by review/QA and fixed here, both worth a look:

  • A self-route (from == to) returns one coordinate, but a
    LineString needs ≥2 — the example would have emitted invalid
    GeoJSON
    . It now pads, as the CLI already does.
  • Retagging the quickstart ```rust,ignore silently disabled the
    doctest gate while every test stayed green. readme_rust_fences_are_exactly_the_two_expected
    now blocks it.

Copilot's review then found four more, all fixed in a14db49:

  • cargo add rustyroute contradicted the status callout two paragraphs
    above it — the crate is not published, so the command would fail.
  • The example bound 0.0.0.0, exposing a pasted quickstart on every
    interface. Now loopback by default.
  • RouteError::NoRoute returned bare text while every other failure
    returned {"error": …} JSON, forcing clients to branch on status
    before parsing. All errors now leave by one door.
  • 16354.1 km over 106 points was asserted nowhere. Rather than soften
    the claim, it is now checked against a real route.

That last fix exposed a weakness in one of my own tests: adding a
#-prefixed TOML comment made the heading-inventory test fail, because
it scanned every line for # and read the comment as an h1. Heading
extraction is now fence-aware.

Every new assertion was proven to fail against the specific defect it
guards before being accepted — including both directions of the new
output check (wrong distance, wrong point count).

Follow-ups (out of scope, not fixed here)

  • CONTRIBUTING.md still opens with "rustyroute is currently a
    repository skeleton… routing algorithms, maritime graph data,
    build scripts, and CI workflows are intentionally not yet present" —
    all five now exist. The new README links straight to it, so the
    contradiction is newly reader-visible.
  • src/loader.rs documents the OnceLock + Box::leak handle pattern,
    which the example mirrors — but Box::leak is redundant there, since
    static OnceLock<Graph> already yields &'static Graph.
  • src/loader.rs:379 says the k-d tree upgrade "is deferred to
    ENG-4690", but ENG-4690 shipped as benchmarks (ENG-4690: criterion benchmarks + CodSpeed regression tracking #13).

Checklist

  • PR title follows the Conventional Commits format above.
  • All commits are signed off (DCO — git commit -s).
  • No routing code, vendored data, build script, or CI workflow is
    added unless this PR explicitly owns that scope.
  • LICENSE and NOTICE attribution preserved when those files
    were touched.
  • Community / config files (*.json, *.yml, *.toml, *.md)
    parse cleanly with the validators listed in CONTRIBUTING.md.

🤖 Generated with Claude Code

…example

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
…am, and edge-group table

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
…e menai bbox, and its section order

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
… external consumer

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
…hen README drift tests

- examples/axum_server.rs: emit graph.resolution_km() instead of a
  hardcoded 50, so changing the load() call cannot silently report a
  wrong resolution.
- tests/readme_contract.rs: assert the menaiStrait bbox as a whole
  phrase so an axis swap (lat/lng transposed) fails; add
  readme_edge_group_table_pass_tags_match_pass_groups so the table's
  second column is verified against PASS_GROUPS too.
- README.md: drop the inaccurate "no build script" claim (rustyroute's
  own build.rs compiles bundled SQLite and parses ~17 MiB of
  GeoPackages), note the crate is not yet on crates.io at the point the
  reader runs cargo add, and say which features the Performance rows
  need.

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
…ckout

The peer-review pass edited README.md (build-cost accuracy, crates.io
availability note, Performance feature caveats) and re-spliced the axum
fence, but a `git checkout README.md` used to revert a deliberate
test-discrimination check also discarded those uncommitted edits, and
the previous commit captured the reverted file.

readme_axum_fence_matches_example_file caught it in the regression run —
which is the drift guard doing exactly its job.

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
AC1 ("boots; curl returns valid GeoJSON in [lng, lat] order") was the
only acceptance criterion with no automated coverage — it was verified
by hand during dev and would have rotted on the next handler edit.
readme_contract.rs proves the README and the example agree textually;
this proves the example actually works.

tests/axum_example_e2e.rs spawns the compiled example as a real process
and drives it over a real TCP socket with hand-written HTTP/1.1 — no
HTTP-client dependency added, since deny.toml gates every new crate.
Four tests: coordinate order, RFC 7946 self-route validity, blocked-edge
plumbing, and the 400 error contract.

The example gains PORT (default 3000) so parallel tests bind
OS-assigned ports instead of racing for one; the README fence was
re-spliced to match. example_binary() checks the binary's mtime against
the source and rebuilds when stale — without that, a filtered
`cargo test --test` run picks up a pre-PORT binary and three of four
tests lose a race for port 3000, which is exactly how this file failed
on its first run.

Also gitignore tests/downstream_consumer/target/: the root /target/
entry is anchored, so reproducing AC3 locally leaves ~300 MB untracked.

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
…n the E2E harness

P2. Retagging the README's library quickstart ```rust,ignore silently
disabled AC2: the quickstart stopped being compiled AND run, yet all six
drift tests stayed green and CI passed. Verified by doing it — the
doctest went from `1 passed; 1 ignored` to `0 passed; 2 ignored`.
spec.md section 5 names this as the first forbidden shortcut and
plan.md GC16 pinned it by hand, but nothing enforced it. Added
readme_rust_fences_are_exactly_the_two_expected, which requires the
Rust-family info strings to be exactly ["rust", "rust,no_run"] in order.

P3s:
- examples: replace graph.resolution_km() with const RESOLUTION_KM. The
  accessor returns 0 for a Graph::from_bytes handle — the very
  substitution this file's doc comment recommends for wasm — so a reader
  following that advice would have served "resolution": 0.
- readme_contract: the pass-tag check was a whole-file contains(), so
  swapping two rows' Source cells passed. Now parses (group, source)
  rows and checks the pairing.
- README: the sample response showed 16354.073, the CLI's {:.3} format;
  the example serialises a raw f64. Now shows what it actually emits.
- e2e: is_fresh only compared against examples/axum_server.rs, so a
  binary stale w.r.t. src/ was trusted. Now takes the newest mtime
  across src/, examples/ and Cargo.toml.
- e2e: build the Server guard immediately after spawn — Child does not
  kill on drop, and two panic sites sat before the guard existed.
- e2e: stderr is inherited, not piped-and-never-drained; a child panic
  now reaches the test output instead of filling a pipe.
- e2e: memoise example_binary() behind OnceLock so four tests do not
  each spawn a cargo build against one target dir.

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
Re-review confirmed all 8 prior findings fixed and raised 3 more, all
real:

- e2e: is_fresh() covered src/, examples/ and Cargo.toml but not
  build.rs, build/** or vendor/**, so the exact staleness class the
  previous round filed was still reachable — editing PASS_GROUPS reruns
  build.rs for the lib but not for examples, so a filtered run would
  assert against a binary whose graph still had the old edge groups.
  Now seeded with the build inputs too. Verified: touching
  build/groups.rs triggers a rebuild where it previously did not.
- readme_contract: the row-wise rewrite in the last commit made the
  pass-tag assertion strictly weaker on what a row claims — it checked
  only for the backticked tag, so "bbox around `suez`" passed. Restored
  the full "`pass` tag `{tag}`" phrase. Verified it now fails.
  Also make the read_dir entry error handling consistent with the branch
  above it: an unreadable entry means rebuild, not skip.
- readme_contract: module doc still said five claims / six tests and
  routed AC2 to lib_rs_compiles_readme_as_doctests — the weak check the
  new fence test exists to backstop. Updated, with a note saying which
  of the two is load-bearing so nobody deletes the wrong one.

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
Found in exploratory QA: `block=suezCanal,` returned
400 {"error":"unknown edge group: "} — an error naming no group,
because the trailing comma produced an empty name.

The CLI deliberately does the opposite and its comment names this exact
input: dropping empties "covers --block \"\", a trailing comma
(--block suezCanal,), and whitespace-only entries — treating those as
'nothing to block' is kinder than reporting an unknown group whose name
prints as nothing" (src/bin/rustyroute.rs, run_route). The two surfaces
of the crate disagreed, and the HTTP one produced precisely the error
the CLI's author had called out.

The example now trims and drops empties before edges_for_groups.
Regression-tested by blank_block_entries_are_ignored_not_rejected, which
asserts each variant routes identically to a bare block=suezCanal rather
than merely returning 200.

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
readme_edge_group_table_matches_edge_groups_exactly walked the
edge-group table inline for column 1, while edge_group_rows() — added in
the review round for the row-aware pass-tag check — did the same walk
and returned both columns. Two parsers over one table format is what
rots: the next person to change the table updates one and leaves the
other silently wrong.

The test now derives its column-1 vector from edge_group_rows(). 22
lines removed, behaviour identical (both walks trimmed and stripped
backticks the same way; edge_group_rows additionally requires a Source
cell, which all 13 rows have).

Discrimination re-verified after the dedup — a cleanup that quietly made
the assertion vacuous would be worse than the duplication it removed.

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
Copilot AI review requested due to automatic review settings July 27, 2026 10:19
@codspeed-hq

codspeed-hq Bot commented Jul 27, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing worktree-ENG-4683 (fe2f4f9) with main (c248547)

Open in CodSpeed

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

Updates the crate’s OSS-facing documentation and adds automated “docs-as-contract” tests to keep the README and quickstarts from drifting away from the actual API and examples.

Changes:

  • Replaces the placeholder README with an executable quickstart (doctested) plus an axum HTTP server quickstart.
  • Adds contract tests that pin README sections, edge-group tables, and the axum snippet to code/constants.
  • Adds an end-to-end integration test that builds/spawns the axum example and validates runtime behavior over a real socket.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
README.md New OSS README content, including library + axum quickstarts and architecture/feature sections.
src/lib.rs Adds a #[cfg(doctest)] README include anchor so README fences are compiled as doctests.
examples/axum_server.rs Adds the axum HTTP server example that the README embeds/pins to.
tests/readme_contract.rs Adds tests that assert README structure and keep README snippets/tables in sync with code.
tests/axum_example_e2e.rs Adds a process-level E2E test that boots the example and validates GeoJSON/output semantics.
tests/cli_smoke.rs Adds an invariant test for single-line axum/tokio dev-deps (to protect version parsing).
tests/downstream_consumer/src/lib.rs Adds a downstream-consumer helper that runs the README quickstart route via default features.
tests/downstream_consumer/tests/smoke.rs Adds a downstream-consumer smoke test asserting the quickstart produces a plausible route.
Cargo.toml Adds axum and tokio as dev-dependencies for the example/tests.
.gitignore Ignores the nested downstream-consumer target/ directory.
Comments suppressed due to low confidence (1)

README.md:66

  • This dependency snippet implies crates.io (rustyroute = "0.1"), but earlier in the README you state the crate is not yet published. To keep the README self-consistent, use a git/path dependency here until the first crates.io release (or remove the “not yet published” statement).
rustyroute = "0.1"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread README.md Outdated
Comment thread examples/axum_server.rs
Comment thread examples/axum_server.rs Outdated

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 9 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

README.md:27

  • The README says the crate is not on crates.io yet, but the installation command shown (cargo add rustyroute) will fail unless the crate is published. Use a git/path dependency form here (or explicitly label this command as “after first release”).
cargo add rustyroute

README.md:66

  • This dependency snippet uses a crates.io version (rustyroute = "0.1") even though the README states the crate is not published yet. Consider switching this line to a git (or path) dependency to keep the quickstart copy/pasteable today.
rustyroute = "0.1"

Comment thread README.md
- README: `cargo add rustyroute` cannot work — the crate is not on
  crates.io, which the status callout says two paragraphs earlier. Show
  the git form that works today, and note the plain form for after the
  first release. Same for the axum dependency block.
- examples: bind 127.0.0.1 by default, not 0.0.0.0. This is code people
  paste from a README; it should not expose a routing service on every
  interface because someone tried the quickstart. HOST=0.0.0.0 opts in.
- examples: RouteError::NoRoute returned bare text while every other
  failure returned {"error": ...} JSON, forcing clients to branch on
  status before parsing. All errors now leave by one door. Asserted by
  no_route_returns_a_json_error_like_every_other_failure.
- README/tests: the claim "This prints 16354.1 km over 106 points" was
  unasserted anywhere. Rather than soften it to a vague approximation,
  readme_quickstart_output_matches_reality now parses the numbers out of
  the README and runs the same route to check them. Verified it fails on
  both a wrong distance and a wrong point count.

Also fixes a weakness the heading test exposed in itself: it scanned
every line for `#`, so the new TOML comment inside the dependency fence
read as an h1 and failed the inventory. Heading extraction is now
fence-aware via prose_lines(); re-verified it still catches a renamed
heading.

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>

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 9 out of 10 changed files in this pull request and generated 1 comment.

Comment thread tests/axum_example_e2e.rs Outdated
The comment showed the listening line as `http://0.0.0.0:34567`, which
stopped being true when the example switched to loopback-by-default in
a14db49. Misleading exactly when it matters — while debugging a failed
port parse. Caught by Copilot; my own change should have updated it.

Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>

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 9 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tests/axum_example_e2e.rs:190

  • start_server inherits the parent process environment, so if a developer has HOST set (common in some shells), the example may bind an unexpected address/hostname and the test can fail/flap. Make the test deterministic by explicitly removing HOST (or setting it to 127.0.0.1) for the spawned child.
    let child = Command::new(example_binary())
        .env("PORT", "0")
        .stdout(Stdio::piped())

…start

The Unreleased section already records ENG-4690 and ENG-4692 but had no
line for this change. Adds one covering the README rewrite, the runnable
axum example, and the drift tests that pin both to the API.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Jimbo Freedman <jimbo@spot-ship.com>
@jimbofreedman

Copy link
Copy Markdown
Contributor Author

Handoff verification — fe2f4f9

Added the missing CHANGELOG.md [Unreleased] entry for ENG-4683 (the
section already recorded ENG-4690 and ENG-4692 but not this change).
No other change in this push.

Gate Result
cargo test (full suite, local) ✅ exit 0, 0 failed
pre-commit hooks (commit-time, staged files) ✅ all passed
GitHub checks @ fe2f4f9 ✅ 21/21 pass
DCO ✅ signed off
Review threads ✅ 5/5 resolved, 0 unresolved
mergeable MERGEABLE, 0 commits behind main

mergeStateStatus is BLOCKED on REVIEW_REQUIRED only — this is a
draft PR awaiting human approval, which is the intended state.

🤖 Generated with Claude Code

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 10 out of 11 changed files in this pull request and generated 2 comments.

Comment thread examples/axum_server.rs
Comment on lines +146 to +150
let listener = tokio::net::TcpListener::bind(format!("{host}:{port}"))
.await
.unwrap();
println!("listening on http://{}", listener.local_addr().unwrap());
axum::serve(listener, app).await.unwrap();
Comment thread README.md
Comment on lines +222 to +226
let listener = tokio::net::TcpListener::bind(format!("{host}:{port}"))
.await
.unwrap();
println!("listening on http://{}", listener.local_addr().unwrap());
axum::serve(listener, app).await.unwrap();
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