Repository navigation
fix(ci): clear the three defects every open PR is inheriting (#16089, #16104, #16122) - #16129
Merged
Merged
Conversation
…16104, #16122) Eight red check-runs across nine PRs reduce to three causes, none of which any of those PRs introduced. Fixing them one per PR means three CI suites and three review passes to unblock a queue that is already blocked; fixing them together means one. 1. DEPENDENCY SECURITY SCAN -- js-yaml ~4.3.1 -> ~4.3.2. The advisory range is 4.0.0-4.3.1, so 4.3.2 is outside it and IS the fix. With the redocly allowance already on base, this is the half that clears the scan. Affects #15980, #16088, #16123. 2. PYTHON-SUITE SHARD 7/12 -- a comment citing a line number. comment_line_number_citations_test forbids it, and security.yml carried `security_scan_gate.py:100` from a merge earlier today. Mine. A line number is invalidated by edits to code it does not describe and fails silently into whatever later occupies it -- and it was written inside a paragraph warning that the allowance is coarser than it looks. Symbol name only now. Affects #15980, #16106, #16107, #16117, #16125. 3. CHECK SAME-SCOPE BATCHING -- an ATX heading needs `#` followed by a space. check_pr_issue_batching.py treated any line starting with `#` as the next heading, so a rationale opening `#15961 alone...` made a FILLED section parse as empty. The message now also distinguishes "no section found" from "section found but empty", quoting the line that ended the parse -- because a red only self-corrects when it names its real cause, and every previous author here reworded until green, leaving no trace. Affects #16117, #16119, #16123, #16127. TAKING DEPENDABOT'S WHOLE package.json WOULD HAVE REGRESSED SIX PACKAGES, and that is the reason this is assembled by hand rather than merged from its branch. #16088 was cut before several upgrades landed, so adopting the file wholesale downgrades apexcharts 7.1.0->6.10.0, cytoscape 3.34.2->3.34.1, onnxruntime-web 1.29.0->1.26.0, vega-embed 7.2.0->7.1.0, vue-i18n 11.4.10->11.4.8 and vue-router 5.3.1->5.2.0. Caught by reading the diff rather than the intent. So the frontend change is exactly one line of package.json and three lines of the lockfile -- version, resolved and integrity for node_modules/js-yaml, copied verbatim from dependabot's verified entry rather than invented. The nine remaining `4.3.1` strings in the lockfile are debug, @types/esrecurse, ansis and polished; dependabot's own lockfile has the same nine. 44 tests pass: the batching parser's full suite including the four reproduction cases, and the citation guard that shard 7 was failing on.
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe batching gate now accepts rationale text that starts with an issue reference and reports whether rationale is missing or empty. Tests cover heading parsing and diagnostics. The frontend updates its ChangesRationale parser correction
Frontend dependency override
Security workflow clarification
Priority: ⚪ Not assessed Estimated code review effort: 3 (Moderate) | ~20 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to This change corrects rationale heading parsing, updates the js-yaml override, and clarifies a security-workflow comment. The supplied coverage indicates the intended parser behavior and diagnostics are preserved, with no remaining merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 64.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 2 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
mrveiss
added a commit
that referenced
this pull request
Sep 9, 2026
…ted (#16126) Review observation from the session verifying #16129, and it is a principle the page did not have. The cheapest way to turn a red green is usually to delete the thing being measured. It works, it is fast, and it destroys the value the check existed to defend -- a green worse than the red. Worked example from that PR. A comment failed the line-number citation guard, and deleting the citation would have passed. It would also have removed the reader's route to WHERE the allowance is coarser than it looks, which was the whole reason the sentence existed. The guard forbids line numbers because they rot silently; it does not ask anyone to stop saying where to look. Keeping the symbol and the file and dropping only the number satisfies the rule and keeps the pointer -- invalidated now only by a rename, which is loud. Generalised to the three shapes this repo actually produces: a weak assertion is fixed by asserting the REASON rather than by deleting the assertion, a flagged exemption by RECORDING it rather than by narrowing the pattern until the tree looks clean, and a stuck ratchet by doing the work rather than by raising the baseline. The operative question is not "what makes this pass" -- deletion always makes it pass. It is "what does this check exist to preserve, and does my fix still preserve it?"
… it (#16089) The gate failed on the allowance itself: allowance(s) ['@redocly/openapi-core'] name findings the scanner no longer reports. Remove them; an allowance that matches nothing tolerates whatever appears next. Not the advisory returning. The advisory GOING. This is the self-cleaning property that the allowance was justified against, arriving and doing exactly what it was described as doing -- and it is the third time today that a check I verified has then been right about my own work. Worth being precise about why an empty match is a failure rather than a shrug. An allowance keyed on a package name admits ANY future advisory in that package, so one that currently matches nothing is not inert -- it is a standing exemption for whatever appears there next, with nobody watching. Nothing else in this system asks whether an allowance is still needed, so without this check the entry sits until someone happens to reread the block. The npm gate now carries NO allowances, which is the state to defend. Adding one back needs the evidence the last one had: no reachable fix, checked against the REGISTRY rather than the advisory range -- a fix at a major behind a third party's dependency range is still a fix, which is the distinction that nearly made the js-yaml allowance permanent -- and the package does not ship. #16122's subject was pointing the allowance at a removal tracker. There is no allowance to track, so #16123 is now moot as well as superseded.
Owner
Author
Correction — there was only ever one advisory, and the allowance should never have existedThe scanner stopped reporting From the audit output: Redocly had no advisory of its own. It was listed only because it depended on vulnerable js-yaml. So item 1 of this PR — the js-yaml bump — is what cleared it: One bump cleared both highs. What this means about the work that went into the allowanceI established that redocly 1.34.19 is the newest 1.x, that 2.51.2 exists, and that I answered "is there a fixed redocly?" correctly while the operative question was "why is redocly listed at all?" — and the audit output had already said, in the line directly beneath the range. So the honest sequence is worse than one mistake:
The allowance was never needed. Fixing the one real advisory clears the whole surface. What survivesThe change itself is right and unchanged: js-yaml Consequences
Found by the reviewing session pulling the job log after mine returned empty twice. |
4 tasks done
mrveiss
added a commit
that referenced
this pull request
Sep 9, 2026
This was referenced Sep 9, 2026
mrveiss
added a commit
that referenced
this pull request
Sep 9, 2026
…#16131) (#16132) * security(slm): the js-yaml advisory is live in an unaudited workspace (#16131) We traced GHSA-2883-xcg3-v3hh through autobot-frontend for hours today, merged a fix in #16129, and watched it clear the transitive redocly finding -- while the SAME ADVISORY sat in a sibling workspace the whole time, unaudited. autobot-frontend/package.json "js-yaml": "~4.3.2" fixed by #16129 autobot-slm-frontend/package.json "js-yaml": "4.3.0" inside 4.0.0-4.3.1 4.3.0 is an EXACT PIN, not a range, so nothing resolves it upward. It was never going to age out. WHY IT READ AS CLEAN. security.yml runs `( cd autobot-frontend && npm audit )` -- one directory. There are TWELVE tracked package.json files. A green Dependency Security Scan means "autobot-frontend has no unallowed findings" and is read as "npm dependencies are clean". The summary header even reported "0 advisories across 1191 deps" with no scope named: one workspace's dependency count, phrased as the repository's. Same shape as #13044, where a duplication guard scanned two of five trees and reported nothing rather than a passing number over a partial population. A gate whose scope is narrower than its name is indistinguishable from a clean result. Two changes: The bump. js-yaml 4.3.0 -> 4.3.2 in autobot-slm-frontend, package.json override and lockfile. The integrity hash is copied verbatim from the entry #16129 landed in autobot-frontend's lockfile -- a value already verified against dependabot's, not constructed. An invented integrity hash fails at install time on someone else's machine with no obvious cause, which is worse than the advisory. The scope statement. The summary header now says autobot-frontend ONLY and names the eleven unaudited files, so the next reader cannot take that number for the repository's. This does not widen the audit -- #16131 tracks that -- it stops the gate claiming coverage it does not have. NEW PR BECAUSE NO OPEN PR COULD CARRY IT, which is worth stating since the standing rule is to append rather than open. #16088 is the only open PR touching autobot-slm-frontend, and it cannot merge: 274 commits behind, and `smoke-test` has never reported on it, so a required context can never arrive. A live vulnerability should not wait behind a PR that cannot land. Found by the session preparing #16088's closure -- looking at what closing it would DROP, rather than at whether it should close. * security(deps): the same advisory has a 3.x range, live in two more unaudited workspaces (#16131) GHSA-2883-xcg3-v3hh has TWO ranges. We read one of them all day. >= 4.0.0, < 4.3.2 patched 4.3.2 <- the one we chased >= 3.0.0, < 3.15.2 patched 3.15.2 <- never read by anyone Measured on base: autobot-frontend 4.3.2 fixed, audited autobot-slm-frontend 4.3.0 fixed by this PR mcp-structured-thinking 3.15.1 VULNERABLE libs/autobot-sdk-ts 3.15.0 VULNERABLE Both 3.x instances are TRANSITIVE -- neither package.json declares js-yaml, so there is no pin to read and no override to trip over. They exist only in the lockfiles, in directories no gate inspects. Integrity hashes copied verbatim from dependabot's verified entries on #16088, not constructed. That PR is the only place these two bumps existed, which is why closing it would have discarded them into workspaces nothing would re-report. AND THE WORKSPACE COUNT IN THIS FILE WAS WRONG THREE TIMES, in a comment that exists specifically to stop a reader trusting a scope-less number. "Six workspaces" came from the top-level layout by eye, "eleven" and "twelve" from two different partial counts of package.json files. Measured: 14 tracked package.json (non-nested) 7 with a lockfile -- npm audit needs one 1 audited So SIX auditable workspaces are never audited. Stated with its denominator now, because the count without one is the defect the comment was written about. * security: state the denominator that matters, not the one that is easy to count (#16131) The workspace total has now been stated four times by three sessions and been wrong three of them: 6, then 11/12, then 14, against a measured 13. Four different causes -- counted from the top-level layout by eye, two partial counts, and mine, where the glob `*package.json` matched browser-package.json, a filename that merely ENDS with it. Every one produced by someone looking straight at the number, immediately after watching the previous count fail. THE TELL IS THAT BEING WRONG NEVER CHANGED A CONCLUSION. 13 or 14 tracked package.json files was never the relevant denominator: libs/autobot-ui, autobot-plugins/* and autobot-browser-worker have no lockfile, so npm audit could not run in them whatever the total says. It kept being counted because it was the easiest thing to count. So the total comes out. The comment now says SEVEN workspaces can be audited and ONE is -- the two figures that were correct in every version, and the only two that bear on whether the gate's scope is honest. A number that keeps being wrong and never changes a conclusion does not belong in the sentence. Its presence is not neutral: it is the part a reader checks, and getting it right feels like having checked the claim.
mrveiss
added a commit
that referenced
this pull request
Sep 9, 2026
…criteria (#16126, #15953) (#16127) * docs(process): four corrections found in review, two of them missing criteria (#16126, #15953) A closure audit on #15953 found three of five acceptance criteria met, so it stays open and these are the two that were not, plus two review findings on the merged text. AC4 -- INSTANCES ARRIVE WITH EVIDENCE. The document cited three issues against roughly twenty described instances, which is assertion wearing the clothes of evidence and is the shape the page argues against. Citations added where one was filed: #15987 for both ticked-criterion cases, #16089 for js-yaml, #16124 for --author @me, #16087 for the required-contexts premise. Where no issue exists the absence is now stated rather than hidden, and it says something: an instance with no issue number was found and fixed by whoever caused it, in the same change. That is the rarest outcome on the page and never the one to plan for. AC5 -- THE COUNTERWEIGHT, and the one I most wanted back. The whole page trains a reader to distrust a low count. AC5 is the case where the low count is RIGHT: grep "self.config = config" finds 7 and flake8 --select=F821 finds 5, and neither is blind -- they count assignments and undefined names, and the two they differ on are exactly the two files that bind the name. Without it the document teaches a reflex -- "the lower number is the broken instrument" -- that is wrong often enough to cause work. A mismatch is a prompt to compare DEFINITIONS, and either instrument may be the one telling the truth. FAMILY ORDER. The letters run A B C D F, then E. E was written first and is cited by number on #16100 and #13148, so renaming breaks those; the sequence is now stated where F sits instead of left for a reader to trip over. On this page above all, a label that does not match what it labels is the subject. FAMILY D GAINS ITS MECHANISM, supplied by the reviewer from their own gate. D said a correct output can support two causes; it did not say why nobody checks. A merge gate reported STALE PR OBJECT truthfully for an hour while the reviewer supplied a cause it had never reported -- the output correct, the explanation invented on top of it and unexamined, because a refusal feels like it has already been investigated. Nobody audits a "no". That example also draws the line between D and the ladder's second rung: rung 2 is a GUARD answering an adjacent question, D is a PERSON answering one on the guard's behalf. Both #16126 findings came from the session reviewing the document rather than from its author, which is the document's own closing claim arriving as evidence about the document. * docs(process): a provenance claim describes a gesture, not an artefact (#16126) New instance for family F, and it is mine from an hour ago. I told a reviewer I had taken two files wholesale from another branch "precisely so there would be no difference to miss", offering that as a reason the supersession did not need checking. They checked anyway. The files DIFFER: black reflowed five regions between the copy and the commit -- a raw string rejoined, a quote style flipped, a signature collapsed, a docstring spaced. The conclusion held. The _ATX_HEADING regex was byte-identical and all four reproduction cases were present, including the two that must keep returning None. But the claim I used to argue against checking was false. "I copied it wholesale" is a correct answer to WHAT DID YOU DO. It was read as an answer to ARE THE FILES IDENTICAL, and those come apart the moment any automated step sits between intent and artefact -- which, in this repo, is always, because the formatter runs on commit. Worth recording because of what the claim was FOR: it was offered as grounds to skip a comparison. A provenance claim that discourages verification is the most expensive kind to get wrong, and the reviewer's line is the rule -- a duplicate that is NEARLY the same is worse than one that is identical, because the difference is what nobody reads. * docs(process): satisfying a check must not cost what the check protected (#16126) Review observation from the session verifying #16129, and it is a principle the page did not have. The cheapest way to turn a red green is usually to delete the thing being measured. It works, it is fast, and it destroys the value the check existed to defend -- a green worse than the red. Worked example from that PR. A comment failed the line-number citation guard, and deleting the citation would have passed. It would also have removed the reader's route to WHERE the allowance is coarser than it looks, which was the whole reason the sentence existed. The guard forbids line numbers because they rot silently; it does not ask anyone to stop saying where to look. Keeping the symbol and the file and dropping only the number satisfies the rule and keeps the pointer -- invalidated now only by a rename, which is loud. Generalised to the three shapes this repo actually produces: a weak assertion is fixed by asserting the REASON rather than by deleting the assertion, a flagged exemption by RECORDING it rather than by narrowing the pattern until the tree looks clean, and a stuck ratchet by doing the work rather than by raising the baseline. The operative question is not "what makes this pass" -- deletion always makes it pass. It is "what does this check exist to preserve, and does my fix still preserve it?" * docs(process): check why an exemption exists, not the thing it names (#16126) Two instances from the last hour, one of which is the sharpest exemption lesson this page has. AN ALLOWANCE WAS WRITTEN FOR A PACKAGE THAT HAD NO ADVISORY. redocly was justified by a careful upgrade-path analysis -- 1.34.19 is the newest 1.x, a fix exists at 2.51.2, openapi-typescript carets ^1.34.6 so 2.x is unreachable. Every clause true. And beside the point: the audit output said, one line beneath the range, "Depends on vulnerable versions of js-yaml". Redocly was listed only for depending on vulnerable js-yaml, and one patch bump cleared both highs. Author and reviewer both verified the allowance. Both checked the package the exemption NAMED rather than the reason it was NAMED. The justification was rigorous, which is what made it convincing -- and rigour applied one level away from the question is indistinguishable from rigour applied to it. The operative rule: a finding that arrives THROUGH another finding is fixed at the source, and an exemption written for it is a permanent exemption for a condition that was never going to persist. AND AN EMPTY STDOUT WITH AN UNREAD STDERR IS NOT A MEASUREMENT. gh run view --log writes "the response contains terminal escape sequences" to stderr and nothing to stdout, so a piped read returns empty. Two sessions independently read that as the logs being gone -- one published "the logs are expired", the other tried twice and reported an unresolvable gap. --allow-escape-sequences returns 212KB. A tool refusing to answer looks identical to a tool answering nothing. * docs(process): a detector built from what was found inherits its vocabulary (#16126) Recorded as its own entry rather than folded into name-the-selector, on the handing-off session's argument that collapsing them loses the second. They are mirrors: name-the-selector governs EMPTY results, this governs POSITIVE ones. A guard forbidding refusal-suppression in shipped prompts was written from the six strings in the tree. Probed by a session that had not seen the pattern, against fourteen phrasings absent from it: 6/6 live found, 0/14 unseen found. Every branch was anchored on the lemma refuse/refus, and the fourteenth case was the shipped line WITH ONE WORD CHANGED. After rewriting to two factors in proximity, 17/17. Two rules follow. Measure recall against phrasings not yet in the tree, because a detector matching everything it was built from proves only that it can find its own training set. And keep the corpus in a SEPARATE FILE from the pattern -- a detector holding its own exam invites the fix that makes the test pass, and split, editing the detector and editing what it must catch are two files in the diff. The general form: "broader than the strings I found" is not the same as "broad enough". The first is measured against a population the instrument already saw. * docs(process): delete the number that never changes a conclusion (#16126) One workspace count was stated four times by three sessions and was wrong three of them -- 6, 11/12, 14, against a measured 13 -- with four different causes. The last was mine: the glob *package.json matched browser-package.json, a filename that merely ENDS with it. Every one of those was produced by someone looking straight at the number, immediately after watching the previous count fail. Care was not the variable. It was maximal every time. THE TELL IS THAT BEING WRONG NEVER CHANGED A CONCLUSION. The total was never the relevant denominator -- three of those workspaces have no lockfile, so npm audit could not run in them whatever it said. Seven auditable and one audited were correct in every version. So the remedy is not to count more carefully, which had already been tried three times. It is to DELETE THE NUMBER. A figure that cannot change the decision does not earn its place beside one that can, and its presence is not neutral: it is the part a reader checks, and getting it right feels like having checked the claim. The operative question before publishing a count: which decision moves if this is wrong? If none does, it is decoration with the texture of evidence. Diagnosis from the reviewing session, which noticed the pattern across all four attempts rather than only correcting the fourth. * docs(process): a scoped absence degrades into an absolute one in transit (#16126) My error, through two relays, and the direction of the drift is the finding. An issue reported "a scoped absence" in those words: within autobot-backend/judges/, nothing imports ResponseQualityEvaluator. The author anticipated the misreading and named two real importers in the body. I relayed it and the path fell out -- "six judge modules and not one importing it". A second session forwarded that, and it arrived as NOTHING IMPORTS IT. Measured on base: eight importers, including knowledge/pipeline/cognifiers/recursive_summarizer.py, the production compaction path that #16110 and #16111 are about. Zero within judges/, exactly as filed. THE REMEDIES ARE OPPOSITE. Nothing imports it is dead code -- revive or delete. The judge modules reimplement it is duplicated live code -- converge onto the one that works. The relay substituted the weaker argument while making it sound more dramatic, which is why nobody along the chain felt they had lost anything. The drift is directional. The scoped claim is longer, needs a qualifier and invites "which directory?"; the absolute one is shorter, striking and self-contained. Everything that helps a sentence survive retelling also strips its scope, and nobody adds a path back in. So the selector travels with the result or the result does not travel. Name the selector applies with MORE force to a forwarded claim than a published one: every hop can shed the qualifier and none can recover it. * docs(process): a superlative endpoint answers a question about position (#16126) Third instrument-level instance of one family today, and the sharpest mechanism. A watcher polled /pages/builds/latest waiting for one build and reported "still building" for its whole life. That build had ERRORED. A second push re-pointed latest, and the watcher -- which had never held a SHA -- saw an unbroken run of building: first one build, then a different one. 12:13:36 a8fad32 errored <- never observed 12:22:01 e1cc415 building <- what latest became The failure was not missed through inattention. It was OVERWRITTEN. The watcher read a pointer rather than a thing, and nothing in the response says which build a status belongs to unless you compare .commit yourself. Related to an endpoint returning a superset of what you meant, and worse in one respect: there the extra rows are visible and filterable, here the row you wanted is gone from the answer and replaced by one that looks identical. The direction is also the dangerous one -- a superseded failure reads as STILL WORKING, which prompts waiting rather than investigating. So key a watch to the identity you care about, never to a positional alias. Any endpoint whose name is a superlative -- latest, head, current -- answers what is at this position NOW, and the position is not what you were asking about. Found by the session running the deployment, when its own watcher timed out without ever seeing the error it was armed to catch. * docs(process): a slow queue and a hung job are the same status field (#16126) Three deploys of one page, no live site, and not one failed on content. a8fad32 queued ~17 min for a runner, then marked errored at the exact second the next push arrived -- my push e1cc415 build SUCCEEDED, deploy job CANCELLED two minutes later by a POST asking for a build, sent because the row looked stuck Pages deploys one at a time, so EACH ATTEMPT TO HELP CANCELLED THE WORK ALREADY IN FLIGHT. The API reported every cancellation as "Page build failed.", duration 0, error detail null -- indistinguishable from a real failure, and it sent two sessions hunting a defect in a page that had built cleanly. The status field cannot separate a slow queue from a stuck job: both show a row that has not moved. And the intuitive response to not-moving -- retry, re-request, re-push -- is exactly the action that destroys the in-progress attempt. The instinct is not useless here, it is the mechanism of the failure. So the question is not "has this stopped moving?" but "is anything working on it right now?" -- a question about a RUNNER, which a status field does not answer. gh run list answers it; pages/builds actively misleads. Where an operation is exclusive, a wrong nudge does not waste a call, it destroys the attempt that was about to succeed. Diagnosed by the session that made the second cancellation and said so, including retracting its own earlier speculation about a first-push race once the Actions runs disconfirmed it. * docs(process): a sample from the head confirms only the head (#16126) Corrected instance -- and the correction is the case, not the original claim. GitHub reported NOASSERTION for two repositories whose LICENSE files are Apache-2.0. First reading: the classifier is wrong, the files are fine, reached by running head -3, seeing "Apache License / Version 2.0", and generalising to the file. Both files are 736 bytes -- the Apache-2.0 NOTICE BOILERPLATE, not the licence text. The same project's main LICENSE is 11343 bytes and classifies fine. GitHub matches full licence texts, so NOASSERTION was the CORRECT output for a stub. The instrument was right and our files are incomplete: the exact inverse of the conclusion drawn. A truncated read answers a question about the truncation. head -3 establishes what three lines say and nothing about line four onward, and its danger is that on a file whose opening looks canonical it returns something shaped like confirmation -- so the reader stops. Stopping is the failure, not the command. Two properties would have settled it in one call, neither requiring the file to be read: its SIZE against a known-good example of the same kind, and WHAT THE CONSUMING INSTRUMENT MATCHES ON. Recorded for when it happened. This was committed while writing up a finding about instruments answering narrower questions than intended, by the session that had spent the day finding that failure in other people's guards. Knowing the pattern by name does not exempt the next command you type. Filed separately as #16138 to replace both LICENSE files with full text. * docs(process): compression drops the qualifier, relay or not (#16126) Reframing, because my version named the wrong mechanism. I wrote this as a transmission defect -- a scoped claim losing its qualifier between people, each hop shedding it. The tidiest instance has no relay at all: a research document stated its claim bounded, six named axes with file:line evidence, and the index row summarising it said "our loop design is ahead on every axis". SAME SESSION, SAME DOCUMENT, SAME SITTING. Nothing was retold and the bound was still lost. So the mechanism is compression, and a relay is only the case where it happens repeatedly. Any act of shortening selects for the version without the qualifier, because the qualifier is the part that costs words and invites a question. That widens where to look. Not just messages between sessions, but any summary of something more careful -- and an INDEX ROW is the worst case, because it is the surface people scan to decide whether to open the document, so the scoping ends up in the file they then do not read. Remedy unchanged in substance, wider in scope: the summary carries the bound or it is not a summary of that claim. Where the short form cannot hold the qualifier, quote the original sentence rather than compress it -- a longer row is cheaper than an unbounded claim on the surface most likely to be read alone. Observation from the session that fixed the row rather than merging past it. * docs(process): a zero is a sample, not a state (#16126) gh run list --status in_progress returned 0 while 47 runs sat queued. Read as a condition that is a fleet-wide Actions stall. It was not one -- in_progress is short-lived, the query caught an instant between hand-offs, and a count minutes later showed 5. The value was correct about the moment it was taken. The conclusion was about the system. Any instantaneous count of a transient state is a sample, and a ZERO sample is the most likely to be read as a fact -- "none" sounds like a property of the system where "five" obviously invites "right now?". The misreading is most seductive exactly when the number is alarming. Two defences, neither requiring any understanding of the system: sample twice separated in time, and check the COMPLEMENT -- runs completing proves the pool works far better than runs in flight proves it, and completions are durable where in-flight is not. The general form covers most of this page: prefer the durable signal to the momentary one. A completed run, a merged commit, a file on disk are still true when you look again. A queue position, an in_progress count and a build's latest pointer are photographs of something moving. Caught by the session that made it, before publishing -- the only one of three instrument misses today that did not have to be corrected by someone else. * docs(process): a wrong reference that resolves (#16126) Two sessions wrote an issue number before filing the issue, on the same day. One predicted #16135 for a follow-up and filed as #16139. #16135 exists -- an unrelated open issue about a 404ing landing page -- so a reader following that citation arrives somewhere real, plausible and wrong. The other was me. I wrote (#16135) into a commit message before filing, then filed and HAPPENED TO GET #16135. Same act, opposite outcome, and the difference was which id was next rather than anything either of us did. A broken reference announces itself. A valid reference to the wrong thing does not: it renders normally, carries a plausible title, and gets believed. It is the more expensive of the two by a wide margin and the one that looks fine in review. So: file first, cite second. Never write an identifier that has not been read back from the system that issues it. Same rule as the rest of this page, applied to a number you PRODUCED rather than one you measured -- a predicted id is a claim about the world, and the world assigns ids without consulting you. The lucky case is the instructive one. Getting away with it produced no error, no correction and nothing to learn from, which is why the habit survives until the day the number is taken. * docs(process): a rule that costs something when it applies gets skipped then (#16126) The predicted-identifier rule got a better diagnosis than the rule itself, from the session that broke it one hour after stating it. They wrote "file first, cite second -- never write an issue number I have not read back from the API", then in the NEXT artefact wrote "Filed as #16140" before filing. It filed as #16140. Correct again, and correct is the worst available outcome: a wrong number gets caught, a lucky one leaves a citation that resolves, a closure that reads as rigorous, and a habit with fresh evidence that it works. Their diagnosis is the part worth keeping. The rule was not forgotten -- it failed where it costs something. A citation gets written WHILE COMPOSING THE REASONING THAT NEEDS IT, so obeying means breaking off mid-argument to go and file. A rule whose price is highest exactly when it applies gets skipped exactly when it applies, and agreeing with it beforehand does not change that. So the version that survives contact is not a stronger commitment but a cheaper action: write a placeholder, finish the argument, fill it in after filing. This is the third time today someone committed the error they were in the middle of documenting. The page already says knowing the pattern by name does not exempt the next command you type; this says WHY, and what to do instead.
2 of 5 tasks
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.
Closes #16104, #16122. Delivers #16089's remaining half.
Thinking Path
Eight red check-runs across nine PRs reduce to three causes — and none of those PRs introduced any of them.
Dependency Security Scan— js-yamlpython-suite shard 7/12— a line-number citationCheck same-scope batching— ATX heading parsingFixing them one-per-PR costs three CI suites and three review passes to unblock a queue that is already blocked. Together it costs one.
What Changed
1. js-yaml
~4.3.1→~4.3.2. The advisory range is4.0.0 - 4.3.1, so 4.3.2 is outside it and is the fix. With the redocly allowance already on base, this is the half that clears the scan.2. A comment citing a line number.
comment_line_number_citations_testforbids it, andsecurity.ymlcarriedsecurity_scan_gate.py:100from a merge earlier today. Mine. A line number is invalidated by edits to code it does not describe and fails silently into whatever later occupies it — and it was written inside a paragraph warning that the allowance is coarser than it looks. Symbol name only now.3. An ATX heading needs
#followed by a space.check_pr_issue_batching.pytreated any line starting with#as the next heading, so a rationale opening#15961 alone…made a filled section parse as empty. The message now also distinguishes no section found from section found but empty, quoting the line that ended the parse — a red only self-corrects when it names its real cause, and every previous author here reworded until green, leaving no trace.Verification
Taking dependabot's whole
package.jsonwould have regressed six packages, and catching that is why this is assembled by hand rather than merged from #16088's branch. That branch was cut before several upgrades landed:Found by reading the diff rather than the intent. So the frontend change is one line of
package.jsonand three of the lockfile —version,resolved,integrityfornode_modules/js-yaml, copied verbatim from dependabot's verified entry rather than invented.The nine remaining
4.3.1strings in the lockfile aredebug,@types/esrecurse,ansisandpolished. Dependabot's own lockfile has the same nine — checked, rather than assumed.Model Used
Claude Opus 5 (1M context).
Batch rationale
Three fixes, one PR, by design — this is the "unblock the base" shape rather than a convenience batch. Each defect is inherited by PRs that did not cause it, so the value is entirely in landing them together and first: one CI suite instead of three, and the queue rebases onto a green base once rather than three times.
They also share a review question — is this the minimal change that clears the inherited red — which is why one pass is honest here and would not be for unrelated features.
After this merges: #16088 becomes redundant for js-yaml and should be re-swept before any decision; #16107 and #16123 are superseded by items 3 and 2 and can close. Then every open PR needs its base updated once, against a green base.
Summary by CodeRabbit
Bug Fixes
Chores