refactor(api): Prepare the public surface for v1 - #50
Merged
Merged
Conversation
LatencyStats, StageMedians, Confidence and StabilityParams were aliases of internal/engine types. pkg.go.dev cannot document a type it cannot reach, so callers saw "type StabilityParams = engine.StabilityParams" and no fields, and a rename inside internal/engine silently broke every importer while apidiff on the public package reported nothing at all. Declare the four in this package, with the field documentation the aliases hid, and convert at the five call sites that already crossed the seam. Two exports go with them: LatencySample, which nothing in Result reaches and which run.go and probe.go now name through the engine, and DefaultStabilityParams, whose values a zero StabilityParams already selects. JSON tags are copied verbatim, so the wire contract is unchanged and schema_version stays 1 (INV-7); TestResultSchemaGolden passes untouched. The seam needs guarding both ways: a renamed engine field fails to compile in convert.go, and an added one fails TestPublicTypesMirrorEngineTypes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuMYXaDafWUk2fAjyEvFDa
Run and RunWithEvents differed by one argument, and Run was a one-line forward. A caller who wanted progress had to switch entry points rather than set a field, and the pair had to be kept in step for ever once v1 is tagged. Options.Events carries the sink now and Run is the only entry point. The concurrency contract (RES-8) moves onto the field, where a caller reads it while writing the thing it constrains. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuMYXaDafWUk2fAjyEvFDa
ParseServerConfig had no caller outside this package and its own tests. Run performs discovery itself, and a caller who wants to parse a document without running a test is not a use case anyone has asked for; exporting it at v1 would commit us to the signature for ever. ErrInvalidConfig stays exported: Run wraps it, so errors.Is remains the way to tell an invalid document from a transport failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuMYXaDafWUk2fAjyEvFDa
Nothing told us when an exported symbol changed shape. v0.3.0 quietly made Options non-comparable, and the only reason we know is that apidiff was run by hand, once, a year later. scripts/api-compat.sh compares every public package against the newest version tag and exits non-zero on an incompatible change. apidiff always exits 0, so the empty report is the pass condition rather than its status. The tool is pinned and run through `go run`, like govulncheck, so it stays out of go.mod (INV-5). The job is advisory while breaking changes are still allowed. It turns blocking when v1.0.0 is tagged, by dropping continue-on-error. It cannot see everything: a type aliased out of an internal package is beyond its reach, which is the other half of why those aliases are gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuMYXaDafWUk2fAjyEvFDa
architecture.md described the public latency and confidence types as aliases of engine types, which was the deliberate choice this branch reverses, and RES-7 named RunWithEvents. Both now describe what the code does. The release steps gain the API-compatibility script and say what tagging v1.0.0 costs: the advisory job turns blocking, and an incompatible change needs a /v2 module path from then on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuMYXaDafWUk2fAjyEvFDa
The first version of this guard asserted that the public types and their engine counterparts were field-for-field identical, names included. That re-couples the two declarations the branch had just separated: an engine-only rename, correctly absorbed by the converter, would still fail the test with nothing actually wrong. What matters is that no value is lost crossing the seam. Filling every engine field with a distinct value and round-tripping it catches a dropped field without caring what anything is called, and matching result fields by name rather than position lets either declaration be reordered freely. A rename still fails to compile in convert.go, which is where it belongs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuMYXaDafWUk2fAjyEvFDa
BenchmarkPhaseStateSamples landed on master while this branch was removing LatencySample from the public API. The benchmark measures phaseState, which is internal, so it follows run.go and probe.go in naming the engine's type. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuMYXaDafWUk2fAjyEvFDa
continue-on-error keeps a job from failing the workflow, but the check still renders red. Every pull request between here and v1.0.0 would carry a red X for a break that is allowed, which is how teams learn to stop reading red. The step reports the break as a warning annotation with the full diff in the job summary, and exits 0. The script still exits non-zero, so running it by hand and gating on it after v1 both behave correctly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuMYXaDafWUk2fAjyEvFDa
korya
marked this pull request as ready for review
September 11, 2026 04:14
Twenty-one em and en dashes, most of them clamping an aside that works better as its own sentence. The signed-URL formula was buried between two of them. Four table cells used a bare en dash to mean "the draft says nothing here", which they now say outright. Also fixes a non-breaking hyphen hiding inside percent-encode, where it is invisible and breaks search, and names the server as the actor in the byte-credit paragraph instead of leaving the checks to happen by themselves. No content rewritten: the numbers, the deviations table and the documented limitations are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuMYXaDafWUk2fAjyEvFDa
This was referenced Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Tagging v1 freezes the public API under Go's compatibility promise, and four things
should not be frozen in their current shape.
The worst is invisible.
LatencyStats,StageMedians,ConfidenceandStabilityParamswere aliases ofinternal/enginetypes, so pkg.go.dev showed callerstype StabilityParams = engine.StabilityParamsand none of the thirteen fields behindit. Worse,
internal/enginewas not actually internal: renaming a field there brokeevery importer's build. I verified this by renaming one field and building a consumer
program against it, which failed with
unknown field ProbeCapacityPercent in struct literal of type netquality.StabilityParamswhileapidiffon the public packagereported no changes at all. Pointing
apidiffat the internal package returnsIgnoring internal packageeven with-allow-internal, so no CI configuration cancatch this class of break. The fix had to be structural.
The rest is ordinary surplus: two entry points differing by one argument, and three
symbols exported by accident rather than intent.
Solution
The four types are declared in this package now, carrying the field documentation the
aliases hid, and the engine's values are converted at the five call sites that already
crossed the seam.
LatencySampleis gone from the public API rather than mirrored,because nothing in
Resultever reached it.Options.Eventscarries the progress sink,leaving
Runas the only entry point.ParseServerConfigandDefaultStabilityParamsare unexported.
JSON tags are copied verbatim, so the wire contract is untouched and
schema_versionstays 1 (INV-7).
TestResultSchemaGoldenpasses without the golden file changing, whichis the proof.
Un-aliasing introduces one new way to fail: two declarations drifting apart. A renamed
engine field now fails to compile in
convert.go, and one added but not carried acrossfails the conversion tests. I verified both by making each change and watching the right
guard fire.
Finally,
scripts/api-compat.shand a CI job report incompatible API changes against thenewest tag. It is advisory while breaking changes are still allowed, and turns blocking
when v1.0.0 is tagged. The tool is pinned and run through
go run, like govulncheck, soit never enters
go.mod(INV-5).Other Changes
LimitListenerstays public: itsClosereleasing anAcceptwaiting at the cap is areal improvement over the
x/netequivalent, and a self-hoster building their own binarywants it.
mutex_bench_test.goarrived from #49 mid-branch and names the engine's sample type now.All benchmarks from that PR still run.
The last commit is unrelated to the API and can be dropped on its own if you would rather
keep this PR pure: a style pass over the README that replaces twenty-one em and en dashes
with sentences, spells out four table cells that used a bare dash to mean "the draft says
nothing here", and removes a non-breaking hyphen hiding inside
percent-encode. Nofigures, deviations or documented limitations were rewritten.
No screenshots: nothing rendered changed.
Related: INV-5, INV-7, RES-8, #49
🤖 Generated with Claude Code
https://claude.ai/code/session_01GuMYXaDafWUk2fAjyEvFDa