Skip to content

🤖 refactor: auto-cleanup#3213

Open
mux-bot[bot] wants to merge 6 commits intomainfrom
auto-cleanup
Open

🤖 refactor: auto-cleanup#3213
mux-bot[bot] wants to merge 6 commits intomainfrom
auto-cleanup

Conversation

@mux-bot
Copy link
Copy Markdown
Contributor

@mux-bot mux-bot Bot commented Apr 30, 2026

Summary

Long-lived auto-cleanup PR that accumulates low-risk, behavior-preserving refactors picked from recent main commits.

Changes

Extract pushStreamErrorRow helper in StreamingMessageAggregator

buildDisplayedMessagesForMessage now has two branches that synthesize a stream-error DisplayedMessage: the existing message.metadata?.error path and the finishReason === "length" path added in #3223. Both pushes were structurally identical, differing only in id suffix, error string, and errorType — the parent-message-derived fields (historyId, historySequence, model, routedThroughGateway, timestamp) were duplicated across both call sites.

Extract a local pushStreamErrorRow closure that captures the shared fields once. Each branch reduces to a single call passing the three differing values. The model access switches from message.metadata.model (which relied on the outer if (message.metadata?.error) narrowing) to message.metadata?.model, which is functionally identical when metadata is defined and falls through to undefined otherwise — same emitted value either way.

This shrinks the surface area for future drift: the next branch added (e.g. a different finish-reason synthesis) can't accidentally pass a stale model or forget routedThroughGateway. Pure refactor — emitted DisplayedMessage objects are identical, and the existing 77-test SMA suite (including the 6 new max-tokens tests) passes unchanged.

Previous cleanups

Drop redundant GuardAnchors type alias in file_edit_insert

In src/node/services/tools/file_edit_insert.ts, GuardAnchors was defined as Pick<InsertContentOptions, "insert_before" | "insert_after">, but InsertContentOptions itself is already Pick<FileEditInsertToolArgs, "insert_before" | "insert_after"> after the .nullish() strict-mode refactor in #2250 stripped the InsertContentOptions interface down to those same two fields. The two aliases became structurally identical, so GuardAnchors is dead.

Drop the alias and use InsertContentOptions for the two callers (insertWithGuards, resolveGuardAnchor). Both names were file-local; no exports change. The function names already convey "guard" context, so the parameter type doesn't need to repeat it. This noise was especially visible to the new guardless-empty-file path added in #3220 since it sits next to insertContent which uses InsertContentOptions.

Pure type-level cleanup — emitted JS, runtime behavior, and the public tool surface are all unchanged.

Extract ResolvedWorkspaceAiSettings type alias in taskService

The shape { model: string; thinkingLevel?: ThinkingLevel } (used internally to read per-agent AI settings off partial workspace metadata) was duplicated 7 times across the parameter and return types of resolveWorkspaceAISettings, resolveTaskAISettings, and resolveParentAutoResumeOptions in src/node/services/taskService.ts. The new sub-agent defaults split (#3215) made this duplication especially visible because it added a third method copying the same inline shape.

A private ResolvedWorkspaceAiSettings interface at module scope replaces all 7 inline occurrences. Pure type-level cleanup — emitted JS, runtime behavior, and the schema-derived WorkspaceAISettings type (where thinkingLevel is required) are all unchanged. The new interface is intentionally local to this file (not exported) since it captures the looser internal-reader shape, distinct from the canonical schema-derived type.

Extract shared ServiceTier type from ServiceTierSchema

The OpenAI service-tier literal union ("auto" | "default" | "flex" | "priority") was duplicated in three places: the CLI options interface (src/cli/run.ts), the providerModelFactory cast at the providers.jsonc boundary (src/node/services/providerModelFactory.ts), and a local OpenAIServiceTier alias in ProvidersSection.tsx. ServiceTierSchema (src/common/config/schemas/providersConfig.ts) already defines the same enum as the runtime source of truth, so this change derives a TypeScript ServiceTier alias via z.infer once and imports it at each site.

The Settings UI keeps its OpenAIServiceTier local alias (used by OpenAIServiceTierSelectValue and the isOpenAIServiceTier type guard) but it is now type OpenAIServiceTier = ServiceTier, so the disambiguating name and call sites stay intact.

Drop unused appendSpace literals on skill/model alias suggestions

In src/browser/utils/slashCommands/suggestions.ts, the SuggestionDefinition literals built for agent skills and model alias one-shots set appendSpace: true, but the build callbacks for those two paths hardcode the trailing space and never read definition.appendSpace. Only the top-level command and subcommand build callbacks consult the field. Remove the no-op appendSpace: true from the skill and model alias mappings and leave a short comment near each so a future reader doesn't assume the field is consulted on these paths.

Behavior is preserved — the field is unread on these paths, and the existing suggestions.test.ts / inlineSkillSuggestions.test.ts / suggestionMatching.test.ts suites all still pass.

Route ThemeContext color-scheme through isLightThemeMode

Replace the inline theme === "light" || theme === "flexoki-light" check in getColorScheme (in src/browser/contexts/ThemeContext.tsx) with the shared isLightThemeMode helper from src/browser/utils/highlighting/shiki-shared.ts.

The helper was just introduced as the single source of truth for the -light suffix → light-theme mapping, and the highlighting call sites (MarkdownComponents, HighlightedCode, highlightDiffChunk) already use it. This change extends that convention to ThemeContext so a future palette like solarized-light would automatically map to colorScheme: "light" without revisiting this site.

Behavior is preserved: for the four valid ThemeMode values ("light", "dark", "flexoki-light", "flexoki-dark"), isLightThemeMode returns the same result as the previous explicit comparison.

Validation

  • bun test src/browser/utils/messages/StreamingMessageAggregator.test.ts — 77/77 pass.
  • make static-check — eslint, tsgo (×2), prettier all pass; only shfmt fails because the binary is unavailable in this environment, and no shell files are touched.

Risks

Minimal — purely a local closure extraction inside a single private method. The emitted JS and the resulting DisplayedMessage objects are unchanged: same fields, same values, same array push order. No exported types or APIs are touched. The 6 new max-tokens tests added in #3223 (which are the most direct coverage of the reorganized code path) all pass without modification.


Auto-cleanup checkpoint: e5d4fee


Generated with mux • Model: anthropic:claude-opus-4-7 • Thinking: xhigh

@mux-bot mux-bot Bot force-pushed the auto-cleanup branch 4 times, most recently from 708166d to 1312f5a Compare May 2, 2026 05:04
@mux-bot
Copy link
Copy Markdown
Contributor Author

mux-bot Bot commented May 2, 2026

⚠️ Auto-fixup could not resolve CI failure: flaky tests, not caused by the cleanup commit.

Failed job: Test / Integration (run 25244368462)

Failures (both in tests/ui/compaction/compaction.test.ts):

  1. force compaction triggers during streaming — timed out at 120000 ms waiting for "Compaction boundary" transcript text.
  2. /compact with Ctrl+Enter (turn-end) does NOT auto-background foreground bashwaitFor in tests/ui/helpers.ts:144 could not locate the workspace in the sidebar.

What I checked:

  • The PR diff is type-only refactors (extract ResolvedWorkspaceAiSettings / ServiceTier aliases, drop unused GuardAnchors / appendSpace literals, route ThemeContext color-scheme through isLightThemeMode). None of it touches compaction, streaming, the workspace consumer manager, or the test harness.
  • Ran both failing tests locally on 1312f5a68:
    • force compaction triggers during streamingPASS in 6.7s.
    • /compact with Ctrl+Enter ...PASS in 4.7s.
  • The streaming-compaction test has a 120s budget and clearly hit a slow/contended runner; the workspace-not-found timeout looks like the usual sidebar race in the harness.

No fix pushed. A re-run of Test / Integration should clear it; if it recurs on this PR or others, the test harness itself likely needs a stability pass (separate from this cleanup PR).

mux-bot Bot and others added 6 commits May 2, 2026 20:09
Replace the inline `theme === "light" || theme === "flexoki-light"` check in
`getColorScheme` with the shared `isLightThemeMode` helper from
`shiki-shared.ts`, so the `-light` suffix convention has a single source of
truth and stays consistent with the syntax-highlighting call sites that
already use it.
…stions

The skill and model alias build callbacks in getSlashCommandSuggestions
hardcode the trailing space, so the appendSpace: true property on those
SuggestionDefinition literals is dead. Remove it and add a brief comment
explaining why we don't propagate appendSpace through these paths so a
future reader doesn't assume the field is consulted there.
The OpenAI service-tier literal union ('auto' | 'default' | 'flex' |
'priority') was duplicated in three places: the CLI options interface
(src/cli/run.ts), the providerModelFactory cast at the providers.jsonc
boundary, and a local OpenAIServiceTier alias in ProvidersSection.tsx.

ServiceTierSchema (src/common/config/schemas/providersConfig.ts)
already defines this enum as the runtime source of truth, so derive a
TypeScript ServiceTier alias via z.infer once and import it at each
site. Pure type-only refactor; the emitted JS and the schema remain
unchanged.
The shape `{ model: string; thinkingLevel?: ThinkingLevel }` (used
internally to read per-agent AI settings off partial workspace metadata)
was duplicated 7 times across the parameter and return types of
`resolveWorkspaceAISettings`, `resolveTaskAISettings`, and
`resolveParentAutoResumeOptions`. The new sub-agent defaults split
(#3215) made this duplication especially visible because it added a
third method copying the same inline shape.

Introduce a private `ResolvedWorkspaceAiSettings` interface at module
scope and use it everywhere. Pure type-level cleanup — emitted JS,
runtime behavior, and the schema-derived `WorkspaceAISettings` type
(where `thinkingLevel` is required) are all unchanged.

🤖 _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `xhigh`_
In `src/node/services/tools/file_edit_insert.ts`, `GuardAnchors` was
defined as `Pick<InsertContentOptions, "insert_before" | "insert_after">`,
but `InsertContentOptions` itself is already
`Pick<FileEditInsertToolArgs, "insert_before" | "insert_after">` after
the `.nullish()` strict-mode refactor in #2250 stripped the
`InsertContentOptions` interface down to those same two fields. The two
aliases are now structurally identical, so `GuardAnchors` is dead.

Drop the alias and use `InsertContentOptions` for the two callers
(`insertWithGuards`, `resolveGuardAnchor`). Both names were file-local;
no exports change. The function names (`insertWithGuards`,
`resolveGuardAnchor`) already convey "guard" context, so the parameter
type doesn't need to repeat it.

Pure type-level cleanup — emitted JS, runtime behavior, and the public
tool surface are all unchanged.
…ator

Both stream-error rows in `buildDisplayedMessagesForMessage` (the existing
`message.metadata?.error` branch and the new `finishReason === "length"`
branch added in #3223) push structurally identical objects, differing only in
`id` suffix, `error` string, and `errorType`. The shared parent-message-derived
fields (`historyId`, `historySequence`, `model`, `routedThroughGateway`,
`timestamp`) were duplicated across both pushes.

Extract a local `pushStreamErrorRow` closure that captures the shared fields
once. Each branch now reduces to a single call passing the three differing
values. Pure refactor — emitted DisplayedMessage objects are identical.
@mux-bot mux-bot Bot force-pushed the auto-cleanup branch from 1312f5a to 34c8ebc Compare May 2, 2026 20:19
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