Skip to content

chore(ratchet): one ratchet baseline library - #27788

Merged
OrcaWin merged 1 commit into
mainfrom
OrcaWin/p399-2-7
Oct 12, 2026
Merged

OrcaWin merged 1 commit into
mainfrom
OrcaWin/p399-2-7

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​126 $\color{#cf222e}{\Huge{\mathbf{−}}}$​161 $\color{#cf222e}{\Huge{\mathbf{−}}}$​35
Prod 8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​279 $\color{#cf222e}{\Huge{\mathbf{−}}}$​534 $\color{#cf222e}{\Huge{\mathbf{−}}}$​255

ELI5

Orca has several "ratchet" checks: each keeps a list of known exceptions (files allowed to skip a rule) and fails CI if the list grows. Five of them carried their own copy of the same list-reading, list-diffing and list-pruning code. This PR moves that code into one small library and deletes the copies. One check whose list was already empty becomes a plain "never allowed" check.

What Changed

  • New config/scripts/ratchet-baseline.mjs: reading tracked files, entry baselines (one path per line) and count baselines (<count> <path> [# note]), diffing, pruning, and the shared --init / --prune / check flow for entry ratchets.
  • check-max-lines-ratchet and check-ts-nocheck-ratchet now only define what they scan and what advice they print; the shared flow does the rest.
  • check-focus-read-ratchet, check-layout-writer-ratchets and check-process-host-imports use the library's baseline code instead of their own (layout-writer no longer imports it from the focus-read script).
  • check-runtime-electron-ratchet: its baseline file was empty and its test pinned it empty. The baseline file, its --write mode and its diff code are gone; any module reachable from the runtime that imports electron now simply fails.
  • The focus-read ratchet keeps its three banned helper names (getActiveRuntimeTarget, legacyRouteFromSettings, settingsForRuntimeOwner) unchanged.
  • Baseline unit tests that were copied across three test files now live once in ratchet-baseline.test.mjs, which also covers the shared check/prune flow.

Before/after for developers: same gates, same pass/fail decisions, same baseline file formats. The failure text for max-lines and ts-nocheck is shorter (no ASCII box) but still names each offending entry and the fix command.

Why

The copies had already drifted (three different parseBaseline shapes, two different prune paths), so a fix to one ratchet silently missed the others. One library keeps the behaviour in one place. An empty "may only shrink" list carries no information, so a plain ban states the rule directly.

Net lines: production −255 (including the new +170-line library), tests −35.

Linked Issue

N/A (maintainer cleanup, Phase 3.99 item 2.7).

Visual Proof

N/A — CI scripts only, no app behaviour change.

Testing

  • All six gates pass on this tree with the current baselines unchanged: check:max-lines-ratchet (4), check:ts-nocheck-ratchet (159), check:runtime-electron-ratchet (0), check:focus-read-ratchet (1120 reads), check:layout-writer-ratchets (211 / 700), check:process-host-imports (127).

  • --prune on each count/entry ratchet leaves every baseline byte-identical.

  • Manually seeded a stale ts-nocheck baseline entry (fails, prune removes it) and a new @ts-nocheck file (fails with the advice).

  • pnpm test on the 8 ratchet test files: 105 passed. pnpm lint (all ratchets), oxlint and check:code-quality:changed pass.

  • Dead-code check: rg for every removed export (parseBaseline, diffBaseline, parseBaselineNotes, formatBaseline, readBaseline, diffAgainstBaseline, readProcessHostBaseline, runtime-electron-baseline) across src, config, scripts, tests, mobile and packages finds no remaining caller.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Cross-platform: file listing still goes through git ls-files and path.join; the CLI entry checks still use pathToFileURL. No SSH, mobile or wire impact.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e5086893-ffcd-46d7-a3e0-a7360d3da677

📥 Commits

Reviewing files that changed from the base of the PR and between 4275ee3 and 78b6976.


📒 Files selected for processing (14)
  • config/runtime-electron-baseline.txt
  • config/scripts/check-focus-read-ratchet.mjs
  • config/scripts/check-focus-read-ratchet.test.mjs
  • config/scripts/check-layout-writer-ratchets.mjs
  • config/scripts/check-max-lines-ratchet.mjs
  • config/scripts/check-max-lines-ratchet.test.mjs
  • config/scripts/check-process-host-imports.mjs
  • config/scripts/check-process-host-imports.test.mjs
  • config/scripts/check-runtime-electron-ratchet.mjs
  • config/scripts/check-runtime-electron-ratchet.test.mjs
  • config/scripts/check-ts-nocheck-ratchet.mjs
  • config/scripts/check-ts-nocheck-ratchet.test.mjs
  • config/scripts/ratchet-baseline.mjs
  • config/scripts/ratchet-baseline.test.mjs

💤 Files with no reviewable changes (1)
  • config/runtime-electron-baseline.txt

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.



📝 Walkthrough

Walkthrough

Ratchet scripts now share utilities for reading tracked sources and parsing, comparing, formatting, and pruning entry and count baselines. The runtime Electron ratchet checks current importers directly instead of comparing them with a checked-in baseline. The Electron baseline file and tests for removed local baseline functions are deleted.


Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 78b69

This change consolidates the ratchet baseline code into one shared module without changing which checks pass or fail. No merge-blocking risk was identified.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 42.42% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 13 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main change: consolidating ratchet baseline logic into one shared library.
Description check Passed The description follows the repository template and explains the change, rationale, testing, scope, compatibility considerations, and checklist status. It correctly marks the issue as N/A for a mainta…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@OrcaWin
OrcaWin merged commit 231cf2c into main Oct 12, 2026
30 checks passed
OrcaWin added a commit that referenced this pull request Oct 12, 2026
…ary (#28054)

#27756's script imported baseline helpers from check-focus-read-ratchet.mjs
after #27788 had moved them into ratchet-baseline.mjs, so it crashed on start
and failed every PR that merges main. Use the library's helpers, and record the
two daemon-process launches from #27769 that the broken check never saw.

Co-authored-by: Orca Worker <orca-worker@localhost>
brennanb2025 pushed a commit that referenced this pull request Oct 12, 2026
…ary (#28054)

#27756's script imported baseline helpers from check-focus-read-ratchet.mjs
after #27788 had moved them into ratchet-baseline.mjs, so it crashed on start
and failed every PR that merges main. Use the library's helpers, and record the
two daemon-process launches from #27769 that the broken check never saw.

Co-authored-by: Orca Worker <orca-worker@localhost>
brennanb2025 added a commit that referenced this pull request Oct 12, 2026
…27875)

* fix(browser): annotation card and outline follow the selected element

The grab overlay keeps re-measuring the picked element after the click
(scroll in any box, resize, element size changes), and the isolated
viewport bridge reports that live box so the pending card follows it,
including page zoom. The card's side pick uses its measured height.

* fix(browser): bind annotation card to selected element

* fix(browser): keep document annotation card on selection

* fix(ci): point the main sync-spawn ratchet at the shared ratchet library (#28054)

#27756's script imported baseline helpers from check-focus-read-ratchet.mjs
after #27788 had moved them into ratchet-baseline.mjs, so it crashed on start
and failed every PR that merges main. Use the library's helpers, and record the
two daemon-process launches from #27769 that the broken check never saw.

Co-authored-by: Orca Worker <orca-worker@localhost>

* fix(browser): follow selected element outer-box size

* refactor(chat): give the start-refusal resumable table its own module (#28085)

#27946 and #26684 together put agent-session-failure-words.ts at 304 lines,
over max-lines, which fails lint and the pre-commit hook on every merge of
main. The refusal-code table is standalone with one reader, so it moves out.

Co-authored-by: Orca Worker <orca-worker@localhost>

---------

Co-authored-by: OrcaWin <alpha-eng@stably.ai>
Co-authored-by: Orca Worker <orca-worker@localhost>
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