Skip to content

docs: align architecture with the tree and document the oracles - #315

Open
OmarAlJarrah wants to merge 1 commit into
mainfrom
docs/readme-architecture-refresh
Open

docs: align architecture with the tree and document the oracles#315
OmarAlJarrah wants to merge 1 commit into
mainfrom
docs/readme-architecture-refresh

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

Two related gaps in the docs, grouped because both land in README.md around the Building
section.

The docs describe a tree that has moved on. Corrected here:

  • docs/architecture.md §2.1 declared the compiler contract method as Parse. It is Compile
    (compilers/compilers.go:40). The preamble also disclaimed the whole signature as
    "illustrative"; it now names the file that declares it, and disclaims only the abbreviated
    parameter types.
  • The §3 package tree omitted compilers/openapi/internal/overlay/, which landed after the tree
    was written. Added, in the dependency position internal/archtest puts it in.
    docs/micro-compiler-design.md:4 carried the same stale count in words ("thirteen internal
    packages") and now derives instead of counting, exactly as docs/micro-compiler-design: the internal-package count is stale #299 asks.
  • pass/ exports one symbol, Validate. README.md, docs/architecture.md (twice — the pipeline
    diagram and the §3 tree) and docs/emitter-design.md all listed six passes with nothing marking
    five of them as unbuilt. Each now says which one exists; the designs stay, marked (planned), in
    the same style the emitters/* rows already used. The pass framework itself is pass: introduce the documented pass framework #67 and is
    untouched here.
  • docs/emitter-design.md §2.1 prescribed a package-level var registry populated by each
    target's init(), citing "mirroring the compiler registry" as its authority —
    compilers/compilers.go:43 says the opposite in its own words, and a mutable package global is
    banned outright. Replaced with the engine-composed instance the compiler registry actually uses,
    plus the reasoning, so the next reader does not re-derive the rejected shape. The §7 package tree
    annotated golang/emitter.go the same way; fixed too.
  • README.md documented one diagnostic rendering. There are two structural shapes, and the
    pointer inside the first takes three forms — a JSON pointer, a line:col from the upstream
    validator, and an empty pointer — all three of which come out of one compile. Documented, with
    the advice to match on the code rather than on the pointer's shape.
  • README.md's Building block ran gofmt -l .; the gate runs gofmt -l $(git ls-files '*.go')
    (gate.yml:15). The two differ by every Go file under a git-ignored directory, so the documented
    command prints hundreds of lines where it promises none. It also claimed
    check-coverage.sh enforces 100% "overall and per package" — the script computes one global
    hit/total over the profile and never groups by package. Both corrected, and the redundant
    go test ./... step dropped, since check-coverage.sh runs it.
  • docs/architecture.md §6 restated README.md's milestone table. The table now lives in one
    place; §6 keeps what only it says — why the milestones run in that order, each chosen to falsify
    a specific claim the IR makes.
  • README.md's design-doc table listed six of the eight files in docs/. Both micro-compiler
    documents added.

No verification workflow had a runnable command anywhere.
grep -rn 'go test -run\|go run ./cmd/morphic-harness\|-fuzz' README.md docs/*.md returned
nothing: the oracle sweep, golden regeneration, the conformance recipe, the fuzz targets and
scripts/verify-atomic-output.sh were each documented only by the code implementing them, and the
sole copy of the regen command was a comment in conformance_test.go. A new
Testing and verification section covers all of them, plus testdata/dangling/, which no
document named at all. docs/architecture.md §5 keeps describing what each mechanism is for and
now points at the commands rather than duplicating them.

Two things deliberately left out. Renaming engine/engine.go's front/fronts identifiers and
the user-visible "engine: parse %q" error prefix — the last of the ParseCompile residue — is
a code change touching a package under the coverage gate, so it belongs in its own PR rather than
riding along with prose. And the count in danglingcheck_test.go's danglingDir comment rotted
the same way micro-compiler-design.md's did; filed as #314 rather than fixed here.

Test plan

Documentation only — no production code changed. The full gate passes (gofmt, go vet,
golangci-lint run, go build ./..., ./scripts/check-coverage.sh at 100%).

Every command the new section documents was run in this branch before it was written down:

  • go run ./cmd/morphic-harness testdata/conformance/openapi and against a single spec; exit
    codes confirmed directly against the built binary — 0 all-pass, 1 on
    testdata/dangling/openapi/f04-composition.yaml, 2 with no args and on a missing path.
  • go test ./internal/harness, go test ./ir -run TestNewBigVal_AcceptsDecimalForms,
    go test ./compilers/openapi -count=1.
  • go test ./compilers/openapi -run TestConformance -update, and the claim that it is not
    global: go test ./... -update fails with flag provided but not defined. The command that
    derives which packages accept it returns exactly cmd/morphic, compilers/openapi, ir/irtest.
  • go test ./compilers/openapi -run '^$' -fuzz FuzzCompile -fuzztime 30s — PASS. -fuzz Fuzz
    refuses with "matches more than one fuzz test", which is why the documented form names one.
  • ./scripts/verify-atomic-output.sh — all checks passed.

A regeneration that changes nothing is indistinguishable from a broken -update, so both
directions were checked rather than assumed:

  • Deleting x-team from allof-inheritance.yaml reddened TestConformance with a named capability
    failure, and -update refused to rewrite the golden while that assertion was failing — so the
    flag cannot paper over a broken lowering. Adding a property the assertion does not name reddened
    the golden comparison and -update then rewrote it correctly.
  • The add-a-case recipe was walked end to end with a throwaway spec: the first -update run fails
    in TestConformance_TableNamesEveryCorpusSpec, the second mints the golden and passes — which is
    why the section says to re-run.
  • Every fixture was restored from a copy and git status confirmed clean before committing.

Counts and lists in the diff are derived, not transcribed: the fourteen internal packages from
ls -d compilers/openapi/internal/*/, the overlay/ gap from comm against the tree in §3, the
sole pass export from grep '^func [A-Z]' pass/*.go, and the docs table from ls docs/*.md. All
internal links were checked to resolve.

Closes #64
Closes #65
Closes #299

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

Labels

None yet

Projects

None yet

1 participant