Repository navigation
refactor(comments): slim verbose comments in shared/cli/relay/preload - #9544
Conversation
Collapse multi-line explanatory comment blocks into single-line "why" statements
per AGENTS.md ("Document the Why, Briefly"): drop restatements of the code and
mechanism narration; keep the non-obvious reason, external refs, and directives.
Comments-only — verified no code changed via a Babel/esbuild comment-strip
token-equality gate against origin/main; typecheck and oxlint clean.
Area: shared, cli, relay, preload. 26 files changed, 1039 insertions(+), 3300 deletions(-).
Co-authored-by: Orca <help@stably.ai>
📝 WalkthroughWalkthroughThe pull request adds orchestration compatibility behavior, including lifecycle sender metadata and legacy keepalive markers. It extends terminal output tracking for transient facts and synthetic frames. Git status, worktree, relay, PTY, agent-hook, preload, telemetry, settings, keybinding, and shared type documentation is revised throughout. A text-search argument-building fallback is repositioned, and new Git status helper exports are introduced. Tests primarily receive explanatory comment updates. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f208597a-26be-447f-b74c-8d0b8b0d7978
📒 Files selected for processing (26)
src/cli/handlers/orchestration.tssrc/preload/api-types.tssrc/preload/index.tssrc/relay/agent-hook-server.tssrc/relay/dispatcher.tssrc/relay/git-handler-status-ops.tssrc/relay/git-handler.test.tssrc/relay/git-handler.tssrc/relay/pty-handler.tssrc/relay/relay-handshake.tssrc/relay/relay.tssrc/shared/agent-hook-listener.tssrc/shared/agent-status-types.test.tssrc/shared/agent-status-types.tssrc/shared/constants.tssrc/shared/git-remote-error.tssrc/shared/gitlab-types.tssrc/shared/keybindings.tssrc/shared/quick-open-filter.tssrc/shared/secure-file.tssrc/shared/telemetry-events.tssrc/shared/terminal-output-side-effects.tssrc/shared/terminal-title-status.tssrc/shared/text-search.tssrc/shared/tui-agent-config.tssrc/shared/types.ts
| /* eslint-disable max-lines -- Why: the preload bridge is the audited contract between | ||
| renderer and Electron. Keeping the IPC surface co-located in one file makes security | ||
| review and type drift checks easier than scattering these bindings across modules. */ | ||
| /* eslint-disable max-lines -- Why: preload is the audited renderer/Electron IPC contract; co-locating the surface eases security and type-drift review. */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win
Remove the max-lines ESLint disable directive.
As per coding guidelines, ESLint or Oxlint max-lines rules must never be disabled, including line-specific disables. Please remove the eslint-disable max-lines directive while keeping your explanation intact.
🐛 Proposed fix to align with guidelines
-/* eslint-disable max-lines -- Why: preload is the audited renderer/Electron IPC contract; co-locating the surface eases security and type-drift review. */
+// Why: preload is the audited renderer/Electron IPC contract; co-locating the surface eases security and type-drift review.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* eslint-disable max-lines -- Why: preload is the audited renderer/Electron IPC contract; co-locating the surface eases security and type-drift review. */ | |
| // Why: preload is the audited renderer/Electron IPC contract; co-locating the surface eases security and type-drift review. |
Source: Coding guidelines
| // last-tool caches) lives on `HookListenerState`. The cache is bounded to one | ||
| // entry per paneKey — see docs/design/agent-status-over-ssh.md §5 (Path 3, | ||
| // request-driven replay) for the rationale. | ||
| /* eslint-disable max-lines -- Why: parsing, replay cache, endpoint writing, and retry state are one lifecycle unit; splitting obscures cleanup ordering across reconnects. */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove ESLint/Oxlint max-lines disables.
As per coding guidelines, you must never disable ESLint or Oxlint max-lines, including line-specific disables. Please remove these directives across all affected files.
src/relay/agent-hook-server.ts#L1-L1: Remove theeslint-disable max-linescomment.src/relay/dispatcher.ts#L1-L1: Remove theeslint-disable max-linescomment.src/relay/git-handler.test.ts#L1-L1: Remove theeslint-disable max-linescomment.src/relay/git-handler.ts#L1-L1: Remove theeslint-disable max-linescomment.src/relay/relay.ts#L2-L4: Remove theoxlint-disable max-linesandeslint-disable max-linescomments.src/shared/text-search.ts#L1-L3: Remove theoxlint-disable max-linescomment.src/shared/agent-hook-listener.ts#L1-L1: Remove theeslint-disable max-linescomment.
📍 Affects 7 files
src/relay/agent-hook-server.ts#L1-L1(this comment)src/relay/dispatcher.ts#L1-L1src/relay/git-handler.test.ts#L1-L1src/relay/git-handler.ts#L1-L1src/relay/relay.ts#L2-L4src/shared/text-search.ts#L1-L3src/shared/agent-hook-listener.ts#L1-L1
Source: Coding guidelines
| /* eslint-disable max-lines -- Why: the central shortcut registry, parser, | ||
| * formatter, and conflict detector must stay in one shared module so main, | ||
| * renderer, browser guests, and Settings cannot drift apart. */ | ||
| /* eslint-disable max-lines -- Why: keep the shortcut registry, parser, formatter, and conflict detector in one shared module so main/renderer/browser/Settings can't drift. */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the max-lines disable directive.
As per coding guidelines, you must never disable ESLint or Oxlint max-lines, including line-specific disables. Please remove this directive entirely.
🛠 Proposed fix
-/* eslint-disable max-lines -- Why: keep the shortcut registry, parser, formatter, and conflict detector in one shared module so main/renderer/browser/Settings can't drift. */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* eslint-disable max-lines -- Why: keep the shortcut registry, parser, formatter, and conflict detector in one shared module so main/renderer/browser/Settings can't drift. */ |
Source: Coding guidelines
Resolved 6 adjacency conflicts with the comment-slimming refactors (stablyai#9542/stablyai#9544/stablyai#9545/stablyai#9546): kept Mission additions and the useCustomActivationOnly activation guard, adopted main's slimmed comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…stablyai#9544) Collapse multi-line explanatory comment blocks into single-line "why" statements per AGENTS.md ("Document the Why, Briefly"): drop restatements of the code and mechanism narration; keep the non-obvious reason, external refs, and directives. Comments-only — verified no code changed via a Babel/esbuild comment-strip token-equality gate against origin/main; typecheck and oxlint clean. Area: shared, cli, relay, preload. 26 files changed, 1039 insertions(+), 3300 deletions(-). Co-authored-by: Orca <help@stably.ai>
Slim verbose code comments — shared, cli, relay, preload
Part of a codebase-wide sweep to slim over-verbose comments per
AGENTS.md→ Document the "Why", Briefly. Comments should state the non-obvious why, not narrate the what/how when the code is already clear; multi-line blocks collapse to a single line.This PR — shared, cli, relay, preload: 26 files changed, 1039 insertions(+), 3300 deletions(-).
What changed
Why:paragraphs → one-line why-statements.eslint-disable,@ts-expect-error,oxlint-disable, …),TODO/FIXME, JSDoc tags, and every genuine non-obvious reason / external reference (issue numbers, platform quirks, race & ordering constraints).useEffect/ regex / bitwise / concurrency: kept a ≤1-line summary where it aids reading.{/* … */}comment text shortened; comments inside template-literal strings left untouched.Safety
Every file in this PR is comments-only — enforced deterministically by stripping all comments (incl. JSX
{/* */}) from bothorigin/mainand the change and confirming the remaining code tokens are identical.pnpm typecheck✅ ·oxlint✅ (no new findings vsmain).Part of a set of 6 area PRs from one sweep.
🤖 Generated with Claude Code
Made with Orca 🐋