Skip to content

refactor: route runtime status marker helper#241

Closed
ndycode wants to merge 1 commit intorefactor/pr4-runtime-status-marker-helperfrom
refactor/pr4-runtime-status-marker-direct2
Closed

refactor: route runtime status marker helper#241
ndycode wants to merge 1 commit intorefactor/pr4-runtime-status-marker-helperfrom
refactor/pr4-runtime-status-marker-direct2

Conversation

@ndycode
Copy link
Copy Markdown
Owner

@ndycode ndycode commented Mar 21, 2026

Summary

  • remove the remaining inline runtime status-marker wrapper from index.ts
  • route status glyph selection directly through the extracted helper

What Changed

  • deleted the inline getStatusMarker() wrapper
  • updated current call sites to use getRuntimeStatusMarker(...) directly while preserving the same v1/v2 marker behavior

Validation

  • npm run lint
  • npm run typecheck
  • npm run build

Risk and Rollback

  • Risk level: low
  • Rollback plan: revert faf62e3 to restore the inline runtime status-marker wrapper in index.ts

Additional Notes

  • this removes another small dispatcher-only runtime wrapper after the helper extraction landed

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

removes the inline getStatusMarker() pass-through wrapper from index.ts and routes all 10 call sites directly to getRuntimeStatusMarker(ui, status) — a clean dead-code elimination with no behavioral change.

  • change is semantically identical: the deleted wrapper was (ui, status) => getRuntimeStatusMarker(ui, status) with no extra logic
  • import was already present: getRuntimeStatusMarker was imported at line 189 before this PR, so no new dependency is introduced
  • no concurrency concerns: all affected call sites are sequential ui string-formatting operations over a local ui reference — no shared mutable state
  • no windows filesystem or token safety impact: change is purely display-layer string selection
  • missing vitest coverage: lib/runtime/status-marker.ts has no test file and no existing suite exercises getRuntimeStatusMarker — both the v1 and v2 branches across all three status values are untested; follow-up coverage is recommended given the 80% project threshold

Confidence Score: 5/5

  • safe to merge — pure dead-code removal with no behavioral change
  • the deleted wrapper was a trivial one-liner delegating to the helper unchanged; all 10 substitutions are mechanically correct; the only follow-up is adding vitest coverage for getRuntimeStatusMarker, which is a non-blocking p2
  • no files require special attention — consider adding test/runtime-status-marker.test.ts as a follow-up

Important Files Changed

Filename Overview
index.ts removed the 5-line getStatusMarker pass-through wrapper and replaced 10 call sites with direct getRuntimeStatusMarker(ui, ...) calls — behavior is identical, import was already in place, no logic changed

Sequence Diagram

sequenceDiagram
    participant C as index.ts (call site)
    participant H as lib/runtime/status-marker.ts

    note over C,H: before this PR
    C->>C: getStatusMarker(ui, status)
    C->>H: getRuntimeStatusMarker(ui, status)
    H-->>C: glyph string

    note over C,H: after this PR
    C->>H: getRuntimeStatusMarker(ui, status)
    H-->>C: glyph string
Loading

Fix All in Codex

Prompt To Fix All With AI
This is a comment left during a code review.
Path: lib/runtime/status-marker.ts
Line: 3-15

Comment:
**missing vitest coverage for `getRuntimeStatusMarker`**

`lib/runtime/status-marker.ts` has no test file and neither `test/ui-runtime.test.ts` nor any other suite exercises `getRuntimeStatusMarker`. now that the inline wrapper is gone this is the sole implementation — the project's 80% coverage threshold means the v1/v2 branch pairs (ok/warning/error × v1/v2) should be covered.

suggest adding a dedicated `test/runtime-status-marker.test.ts`:

```ts
import { describe, it, expect } from "vitest";
import { getRuntimeStatusMarker } from "../lib/runtime/status-marker.js";
import type { UiRuntimeOptions } from "../lib/ui/runtime.js";

// minimal v1 / v2 stubs — adjust to match real UiRuntimeOptions shape
const v1 = { v2Enabled: false } as UiRuntimeOptions;
const v2 = { v2Enabled: true, theme: { glyphs: { check: "", cross: "" } } } as unknown as UiRuntimeOptions;

describe("getRuntimeStatusMarker", () => {
  it.each([
    [v1, "ok",      ""],
    [v1, "warning", "!"],
    [v1, "error",   ""],
    [v2, "ok",      ""],
    [v2, "warning", "!"],
    [v2, "error",   ""],
  ])("v%s status=%s → %s", (ui, status, expected) => {
    expect(getRuntimeStatusMarker(ui, status as "ok" | "warning" | "error")).toBe(expected);
  });
});
```

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: "refactor: route runt..."

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 21, 2026

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • skip-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: eac6a394-89e2-4a2d-9004-75e3cd718b86

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/pr4-runtime-status-marker-direct2
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch refactor/pr4-runtime-status-marker-direct2

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 and usage tips.

@ndycode ndycode added the passed label Mar 21, 2026
ndycode added a commit that referenced this pull request Mar 23, 2026
@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 23, 2026

Closing because this work is now included in main via #318 and #319.

@ndycode ndycode closed this Mar 23, 2026
@ndycode ndycode deleted the refactor/pr4-runtime-status-marker-direct2 branch March 24, 2026 18:34
ndycode added a commit that referenced this pull request Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant