Skip to content

Ci/2026 09 03/pin fixtures and repair floors - #33

Merged
janakhpon merged 9 commits into
mainfrom
ci/2026-09-03/pin-fixtures-and-repair-floors
Sep 3, 2026
Merged

Ci/2026 09 03/pin fixtures and repair floors#33
janakhpon merged 9 commits into
mainfrom
ci/2026-09-03/pin-fixtures-and-repair-floors

Conversation

@janakhpon

Copy link
Copy Markdown
Member

What changed

Why

How it was verified

  • pnpm --filter ./apps/web test
  • pnpm --filter ./apps/web run lint
  • cd services/feedback && go vet ./... && go test ./... (if Go changed)
  • Android unit tests, if Android changed: cd apps/android && ./gradlew testDebugUnitTest
    (needs JAVA_HOME on Android Studio's JBR; see apps/android/README.md).
    CI does not run them, so this is the only gate on that suite.
  • iOS MonOcrCore tests, if iOS changed: cd apps/ios && sh Scripts/swift-test.sh.
    The ios-core job runs these on every push, so this is a faster copy of a
    gate that does exist. The app target is still not built anywhere.

Claims

- add ref: to the monocr-onnx checkout, which the CLI depends on by path
- raise the Android test floor from 40 to 106, its actual count
- give the iOS wrapper a numeric floor instead of a presence check

All three were passing, which is why none of them was visible.

The SDK checkout had no ref, so the CLI was compiled against whatever was on
monocr-onnx's default branch and the same monorepo commit could pass today and fail
tomorrow with nothing here having changed. Pinned at 627c52c, which is what the job was
already getting, so this makes today's behaviour reproducible rather than changing it. A
tag was available and deliberately not used: rust/v0.3.0 is on that repository's main but
main is 34 commits ahead of it, so pinning to the tag would change what compiles and
could turn the job red for reasons unrelated to introducing a pin. The comment records
that trade and that building against the released SDK is arguably more honest once the
Rust crate is actually published.

The Android floor was 40 with a comment beside it reading "the suite has had 50 since
2026-08-28". Counted 2026-09-03: 106. So the floor was stale against its own comment and
the comment was stale against the suite, and 66 tests could have vanished silently.

The iOS wrapper only grepped for the string "Test run with", so it asserted that a count
was printed and never what the count was. The suite could have shrunk from 73 to 1 and
stayed green. It now parses the number and compares it. The summary format was verified
by running the suite rather than assumed: "Test run with 73 tests in 12 suites passed
after 7.234 seconds."

Both floors are exact rather than a margin, for the reason the new comments give: adding
tests never trips a floor, so the only thing a floor can catch is a removal, and a removal
should be deliberate. A failure means bumping the number in the same commit that removes
the test, which makes the diff record the removal.

Verified: sh -n clean, the wrapper runs green at 73 tests, and two mutations killed. A
floor of 200 fails with the real count, and breaking the parse regex fails closed with a
message telling the reader to fix the parse rather than delete the check.
- raise Android's cut-column probes from emptiness to the floor of three
- give the two dilate rule tables the floor half_widths already had
- add count floors to the go, web and cli jobs; only android had one
- correct a comment claiming this repo has zero Go test files

Five gates that pass while the thing they guard shrinks.

Android's `LineTilerFixtureTest` asserted `probes.size() > 0` where web
(segmentation.test.ts:75) and iOS (LineTilingTests.swift:175) both require three.
So the shared probe set could shrink 3 -> 1 and Android alone stayed green --
the one asymmetry among three ports pinned to the same file. MEASURED: truncating
the fixture to one probe now fails 1 of 106 tests; before this commit it failed
none on Android.

`DilateFixtureTest` argues for a floor over emptiness at :50-55, applies one to
half_widths, and then checks `size() > 0` on two tables holding 13 and 12
entries. Both now carry their real counts. The docstring above the second one
already recorded a mutation that survived the whole suite; a table permitted to
shrink to one entry is how.

The Go job's comment read "there are currently zero *_test.go files here, so this
passes vacuously". There are 8, with 26 test functions across 4 packages, all
passing. Stale in the safe direction, which is the expensive kind: it is why this
job never got the assertion android and ios-core both have, so 26 functions
could have vanished unnoticed.

Floors added to go (60), web (172) and cli (68), matching android (106) and
ios-core (73 in Scripts/swift-test.sh). All four measured on this machine today,
exact rather than margined for the reason the android floor already states.

Two details worth recording. The Go floor counts 60, not 26: `go test -json`
emits a pass per `t.Run` subtest, so it catches a deleted table row and not only
a deleted function. And the cli floor parses `test result:` lines rather than
`--format json`, which still needs `-Z unstable-options` and would tie the gate
to nightly while the job runs the stable toolchain.

Verified: gradle testDebugUnitTest --rerun-tasks green at 106 tests / 0 failures;
vitest json reporter gives 172; the Go and cargo parsers reproduce 60 and 68 on
two runs each; the workflow parses as YAML.
- alias the path dependency with package = "monocr"

The Rust binding is now published on crates.io as monocr, not monocr-onnx --
chosen before its first publish, once the shorter name was confirmed
unclaimed. This dependency is a path, not a registry reference, but
Cargo.toml still declares which package a path resolves to, and it needs to
agree with the target crate's real name once that target is ever swapped for
a registry or git dependency.

The dependency key stays monocr-onnx and every monocr_onnx:: reference in
src/main.rs is untouched -- package = "monocr" changes only which crate the
key resolves to, not the extern crate name Rust code sees.

Verified: cargo check succeeds, resolving against the renamed local package.
Cargo.lock updated to match; the dependency tree itself is unchanged, only
the two package entries' names and versions.
- add version = "0.3.0" to the monocr path dependency
- add repository and homepage to Cargo.toml
- add release-cli.yml, mirroring every other binding's release pattern

Blocked before this: cargo publish refused with "all dependencies must have
a version requirement specified when publishing", since a bare path
dependency has nothing a registry consumer could resolve. monocr now exists
on crates.io as of today, so this is a one-line fix rather than a wait.

Verified rather than assumed: cargo publish --dry-run from this exact
checkout downloaded and compiled the real crates.io monocr 0.3.0, not the
local path -- so a consumer installing monocr-cli gets the published SDK,
not a broken reference to a checkout that will not exist on their machine.
fmt clean, clippy clean, 68 tests pass.

release-cli.yml is a separate file, not a job folded into ci.yml, matching
every sibling release workflow in this ecosystem: tests re-run against the
tagged commit, gated ahead of a credential, kept out of the file that runs
on every push. Tag namespaced cli/v* rather than a bare v* since this repo
has zero tags today across four shipped surfaces and a bare v* would become
ambiguous the moment a second one exists.

Not done, and deliberately: this does not touch ci.yml's own sibling-
repository checkout of monocr-onnx, or the TODO in apps/cli/Cargo.toml about
switching that dependency from a path to a pinned git rev. That job's own
comment already flags the SDK being unpublished as the reason "released" was
doing little work -- it no longer is, and switching the CI job (not just the
release job) to depend on the real published crate is a real simplification
worth doing, but it changes local dev workflow too and is a separate call.
monocr-cli v0.1.0 published today. Matches the badge convention every other
MonOCR package README now uses.
The README only documented building from source, even though the package is
now published. cargo install is the path a real user takes; building from a
checkout is the contributor path and now reads as the alternative it is.
actions/checkout fetches a commit ref directly from the remote, so a 7-char
abbreviated SHA fails: "The process '/usr/bin/git' failed with exit code 1".
Set to the full 40 characters.

This is a regression introduced today. The cli job passed on 2026-08-30 and
has failed on every run since the pin was added, with the checkout action's
own SHA provably unchanged from the merge-base -- so the pin, not the action
version bump, is the cause. The intent recorded in the comment block above
was always to pin this commit; only the form was wrong.
- go get github.com/quic-go/quic-go@v0.59.1
- allow two test placeholders in .gitleaksignore, by fingerprint

Both jobs have been red on main since 2026-08-28/29 and neither was caused by
anything in this branch. Fixing them here because they were found here.

GO-2026-5676, HTTP/3 QPACK trailer expansion memory exhaustion, is a CALLED
vulnerability, not a required-but-unreachable one. govulncheck traced two live
paths: api.main -> http.Server.ListenAndServe -> http3.ConfigureTLSConfig, and
upload.init -> gin.init -> http3.countingByteReader.Read. It arrived indirectly
with dependabot's gin 1.9.1 -> 1.12.0 bump; quic-go is not a direct dependency.
v0.59.1 is the patch release for it, taken rather than jumping to v0.62.0.

Verified after the bump: go build clean, go test all packages pass, and
govulncheck exits 0 with "No vulnerabilities found" -- from exit 3 with one
called vulnerability before. Five require-but-don't-call advisories remain in
x/crypto and x/mod, unchanged and not reachable.

The gitleaks findings are not secrets. Both flagged lines are
`const key = "0123456789abcdef"` repeated four times, inside tests named
TestPanicLogDoesNotCarryTheApiKey and TestRecoveryMiddlewareDoesNotLogTheApiKey.
64 hex chars of ascending nibbles trips the generic-api-key entropy rule.
Ignored by fingerprint, not an inline allow comment, because the job runs
`gitleaks detect` over full history and anchors the finding to commit f4de7ba
regardless of the working tree.

Verified with the same pinned image CI uses, zricethezav/gitleaks:v8.30.1:
140 commits scanned, "no leaks found".

Note for later: the govulncheck step installs @latest and queries the live
vuln.go.dev, so this job can go red with no repo change at all. That is the
failure mode go.mod's own toolchain comment was written for.
- Cargo.toml: path dependency -> version = "0.3.1" from the registry
- ci.yml: drop the sibling checkout and the path: nesting it required
- release-cli.yml: correct the environment value and the stale header

The path dependency was the root of three separate failures. It made this
crate unpublishable on its own terms -- crates.io rejects path and git deps,
so the long-standing TODO here ("must become a pinned git rev") pointed at
something that could never work. It made the cli CI job depend on a second
checkout at a revision that predated the library's rename, so the job was red
whether the pin was abbreviated or complete. And release-cli.yml had no such
checkout at all, so the path resolved to a directory that never existed on a
runner.

A registry dependency has none of those failure modes and builds what a
consumer actually gets. It only became possible today, when the library was
first published.

Verified, not assumed: the lockfile now records
registry+https://github.com/rust-lang/crates.io-index with a checksum, and
`cargo build --locked` succeeds from a copy placed where ../../../monocr-onnx
does not exist. 68 tests pass. Every step of the cli job was re-run from the
new checkout root -- fmt, clippy, test and all four fixture checks, including
the merge fixture's own 21-mutation battery.

release-cli.yml's setup block said `Environment: (blank)` while the job
declares `environment: crates-io`. A job that declares one puts an
environment claim in the OIDC token and crates.io matches the whole claim set,
so a publisher registered as blank would reject it.
@janakhpon
janakhpon merged commit ad4b814 into main Sep 3, 2026
13 of 17 checks passed
@janakhpon
janakhpon deleted the ci/2026-09-03/pin-fixtures-and-repair-floors branch September 4, 2026 01:43
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.

1 participant