Reviewer Phase 3: three-way diff with canonical path normalization#48
Merged
Reviewer Phase 3: three-way diff with canonical path normalization#48
Conversation
Closes the reviewer arc. With canonicalization, the three platforms'
different URL encodings reduce to the same comparable string:
Rails OpenAPI: /clinics (server = /api/v1/vet)
iOS Request: /vet/clinics/\(id)
Android Retrofit: {account_id}/api/v1/vet/clinics/{id}
→ all canonicalize to /clinics/{*}
canonicalizeEndpoint(endpoint, {role}) strips:
1. Tenant segment ({account_id} placeholder, or runtime UUID — see
/Users/adachi/pg/ruby/pg/nativeapptemplateapi/lib/middleware/
account_middleware.rb for the Rails side that handles this
transparently via path-info rewrite)
2. /api/v<N>/ prefix
3. role segment (lowercased Shopkeeper rename target — vet, etc.)
4. Path-param names: {id}, {shopId}, \(id) all become {*}
diffContracts(rails, ios, android, ctx) produces a ContractDiff:
- railsOnly: in Rails, no mobile client uses (informational —
admin/server-only routes are expected)
- iosOrphan: iOS calls endpoint not in Rails (FAIL — 404 risk)
- androidOrphan: Android calls endpoint not in Rails (FAIL)
- iosOnly: in Rails + iOS but not Android (FAIL — mobile
clients must agree per project USP)
- androidOnly: in Rails + Android but not iOS (FAIL)
reviewer.contractParity = "fail" iff any of orphan/only counts > 0.
runJudge.overallPass now requires reviewer.contractParity === "pass"
in addition to layer1+layer2+visual.
Real finding against out/vet-clinic-queue/:
ios-orphan: DELETE /clinics/{*}/reset
The iOS substrate calls a reset endpoint Rails OpenAPI doesn't
document — either Rails is missing the spec for that route, or iOS
expects an endpoint that doesn't exist. This is exactly the
contract-drift class of bug the reviewer is supposed to catch and
the project's "no contract drift" USP from ROADMAP.md depends on.
Tests: 19/19 npm run ci green.
- canonicalizeEndpoint reduces three encodings to same canonical ✓
- diffContracts surfaces ios-orphan correctly ✓
- existing dispatch e2e still passes (stub-mode reviewer) ✓
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Summary
Closes the reviewer arc. With canonicalization, the three platforms' different URL encodings reduce to the same comparable string:
The Rails-side
{account_id}is handled transparently byAccountMiddleware, which path-info-rewrites the URL before the rest of the routing layer sees it — same logical endpoint, three different framings.canonicalizeEndpoint(endpoint, {role})Strips:
{account_id}placeholder or runtime UUID)./api/v<N>/prefix.vet,shopkeeper, etc.).{id},{shopId},\(id)all become{*}.diffContracts(rails, ios, android, ctx)→ContractDiffrailsOnlyiosOrphanandroidOrphaniosOnlyandroidOnlyreviewer.contractParity = "fail"iff any of orphan/only counts > 0.runJudge.overallPassnow requiresreviewer.contractParity === "pass"in addition to layer1+layer2+visual.Real finding against
out/vet-clinic-queue/The iOS substrate calls a reset endpoint Rails OpenAPI doesn't document — either Rails is missing the spec for that route, or iOS expects an endpoint that doesn't exist. Exactly the contract-drift class of bug the reviewer is supposed to catch and the project's "no contract drift" USP from ROADMAP.md depends on.
This is a substrate-side finding — recommend investigating the Rails route table for
DELETE /shops/{shopId}/resetand adding it todocs/openapi.yamlif it exists, or removing the iOS call if it doesn't.Test plan
npm run ci— 19/19 green.canonicalizeEndpointreduces three encodings to same canonical (new test).diffContractssurfaces ios-orphan correctly (new test)./clinics/{*}/resetfinding in the substrate.🤖 Generated with Claude Code