Mark the operation count, so it stops being six hand-maintained copies - #686
Merged
Conversation
250 is one jq away from openapi.json and was restated in prose six times across four files. Adding one operation left five of them stale, and it took three rounds of review on #683 to reconcile — each round finding the copies the previous round had not been shown. @operation-count joins @api-version, @bc3-pin and @assertion-types in spec/doc-constants.json's existing machinery: `make sync-api-version` rewrites every marked span from the source, and `make doc-constants-check` fails on any that drifted. markerCounts pins six, so deleting one fails the gate rather than silencing it. The span must state the count as a BACKTICKED integer, and exactly one. Those sentences are full of other numbers — SECURITY.md names 125 GETs and 83 mutations beside the total — and a checker that read bare prose integers as the claim would fail on numbers it has no source for. Backticks are how the prose says "this is the derived constant", which is the device @bc3-pin already uses for the SHA. A line needing a second backticked integer cannot carry the marker, and the error says so. Counting is (path, HTTP method) pairs. Path items also carry parameters, summary and servers, so the verb list is explicit rather than "every key". Derived lazily. The gate's own fixtures are minimal OpenAPI documents with no .paths, and computing eagerly turned each into an error about a constant it never mentions. Six self-test cases, each shown to fail against the un-built feature: neutering the checker fails the three negative cases, neutering the rewriter fails the writer case, and counting every path-item key fails the positive control. The fixture's real operations live in the --openapi source with a five-operation decoy in the checkout, so a gate reading the wrong file reports 5 against a span that says 3. End-to-end: drifting all six spans to 999 flags six errors, and the writer restores all three files byte-identical.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34437ebade
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…n it Codex found a real hole, and it was the dangerous kind: the writer corrupted data and the checker then certified the corruption. --write returns before the per-kind checkers run. So a blanket gsub over every backticked integer on the marked line ran FIRST, and the later check — comparing values that were by then all identical, because it deduplicated — went green over the damage. Reproduced on the real sentence before fixing. Backticking SECURITY.md's 125 GETs and 83 mutations, then `--write`: before: all `250` operations: the `125` GETs ... and `83` mutations after: all `250` operations: the `250` GETs ... and `250` mutations check: passes Two changes. The count is now found by one helper both the checker and the writer call, so they cannot disagree about which integer is the claim; and that helper counts OCCURRENCES rather than distinct values, because two spans both reading `250` today would both be rewritten the day the count moves and only one of them is the count. An ambiguous span is now left exactly as written. That is deliberate rather than a silent skip: untouched, it fails the next --check with a message naming the integers it found and what to do about them. The self-test drives --write over an ambiguous span, asserts the other integer survived, and then runs --check in the same directory to assert the span is still rejected. Restoring the blanket gsub fails it.
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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.
Stacked on #683 — base is
upload-versions-api, because the spans this marks are the very lines #683 changes. Retarget tomainafter #683 merges.Why
250is onejqaway fromopenapi.json, and it was restated in prose six times across four files. Adding a single operation left five of them stale, and reconciling took three rounds of review on #683 — each round surfacing copies the previous round hadn't been shown:That's the failure mode
spec/doc-constants.jsonalready exists to retire, for the bc3 pin and the API version. This adds the operation count to it.What
@operation-countjoins@api-version,@bc3-pinand@assertion-typesin the existing machinery:make sync-api-versionrewrites every marked span from the source,make doc-constants-checkfails on drift, andmarkerCountspins six — so deleting one fails the gate rather than silencing it.The span must state the count as a backticked integer, and exactly one. These sentences are full of other numbers — SECURITY.md names 125 GETs and 83 mutations in the same breath as the total — so a checker reading bare prose integers would fail on numbers it has no source for. Backticks are how the prose says this one is the derived constant, the device
@bc3-pinalready uses for the SHA. A line needing a second backticked integer can't carry the marker, and the error says so rather than guessing.Counting is
(path, HTTP method)pairs, with the verb list explicit — path items also carryparameters,summaryandservers, so "every key" would over-count.Derivation is lazy. The gate's own fixtures are minimal OpenAPI documents with no
.paths; computing eagerly turned every one into an error about a constant it never mentions.Verification
Six new self-test cases, each shown to fail against the un-built feature:
check_operation_countreturns[]rewrite_line's armThe fixture's real operations live in the
--openapisource with a five-operation decoy in the checkout, so a gate reading the wrong file reports 5 against a span that says 3 — the same deviceDECOY_API_VERuses.End-to-end on the real repo: drifting all six spans to
999flags exactly six errors, andmake sync-api-versionrestores all three files byte-identical to their pre-drift state.makepasses clean.Note
This does not mark
MIGRATING.md's counts, deliberately. Those are as-of facts about shipped releases (247 → 249for v0.13.0) and must never be rewritten — the same distinctionwriterExcludesandunmarkedPinCitationsdraw for the pin.Summary by cubic
Adds
@operation-countas a doc constant and marks six prose mentions so the total stays synced withopenapi.json. The writer now refuses ambiguous spans to avoid corrupting other numbers.New Features
@operation-counttoscripts/sync-doc-constants.rb; counts path × HTTP method using an explicit verb list; derived lazily from--openapi.AGENTS.md,SECURITY.md, andSPEC.mdwith backticked250; pinned six citations inspec/doc-constants.jsonso deletions fail.--openapi(with a decoy in-repo file), and writer behavior.<!-- @operation-count -->.Bug Fixes
--checkthen fails with a clear error.Written for commit 32f454e. Summary will update on new commits.