Skip to content

Split runtime settings slices - #30

Merged
makoMakoGo merged 2 commits into
mainfrom
codex/runtime-settings-slices
Jun 10, 2026
Merged

makoMakoGo merged 2 commits into
mainfrom
codex/runtime-settings-slices

Conversation

@makoMakoGo

@makoMakoGo makoMakoGo commented Jun 9, 2026 •

Copy link
Copy Markdown
Owner

变更

  • 在设置 Module 中新增 ASR / LLM 运行设置切片与对应 normalize 函数。
  • asr-transcription 继续导出 normalizeAsrRunSettings,保持现有调用方兼容,但实现回到设置 Module。
  • usePolishFlow 的 settings Interface 收窄为 LLM 运行设置切片,不再消费完整 Settings。
  • 增加测试证明完整 Settings 能分别归一化为 ASR 和 LLM 运行切片。

为什么

#22 的目标是保留设置持久化 Module 的 Locality,同时避免 ASR/LLM 运行调用者继承完整 Settings Interface。这个 PR 把运行所需设置切片集中在设置 Module 中,让后续转录运行和润色运行重构可以依赖更窄的 Interface。

验证

  • npm test -- --run tests/unit/utils.test.ts tests/unit/asr-transcription.test.ts tests/unit/settings-persistence.test.ts tests/unit/polish-stream.test.ts
  • npm test
  • npm run build

Closes #22

Summary by Sourcery

Split ASR and LLM runtime settings normalization into dedicated slices while keeping storage normalization centralized in the settings module.

New Features:

  • Expose ASR and LLM runtime settings slice types and corresponding normalization helpers from the app settings module.

Enhancements:

  • Refine polish flow to depend only on the LLM runtime settings slice instead of the full settings object.
  • Re-export ASR runtime normalization and type from the transcription module to preserve existing public API shape.

Tests:

  • Add unit coverage to verify full settings can be normalized independently into ASR and LLM runtime slices.

@sourcery-ai

sourcery-ai Bot commented Jun 9, 2026 •

Copy link
Copy Markdown

Reviewer's Guide

Splits runtime-related settings into ASR and LLM slices in the app settings module, updates ASR transcription and polish flow to depend on these narrower interfaces, and adds tests to ensure normalization of full settings into the new runtime slices while preserving storage behavior.

File-Level Changes

Change Details Files
Introduce explicit ASR and LLM runtime settings slices and normalization helpers in the settings module, and redefine full-settings normalization in terms of them.
  • Define AsrRunSettings and LlmRunSettings types as Picks over Settings for ASR and LLM runtime needs respectively.
  • Add normalizeAsrRunSettings and normalizeLlmRunSettings functions that trim values and fall back to DEFAULT_SETTINGS for missing/blank fields in their respective slices.
  • Refactor normalizeSettingsForStorage to delegate to normalizeAsrRunSettings and normalizeLlmRunSettings and return a merged Settings object.
lib/app-settings.ts
Move ASR runtime normalization responsibility from the ASR transcription module into the settings module while preserving the public API surface.
  • Stop importing normalizeSettingsForStorage in the ASR transcription module and instead import normalizeAsrRunSettings and AsrRunSettings from the settings module.
  • Export normalizeAsrRunSettings and AsrRunSettings from the ASR transcription module to keep existing callers working.
  • Remove the local normalizeAsrRunSettings implementation that previously wrapped normalizeSettingsForStorage and rely on the shared settings module implementation instead.
lib/asr-transcription.ts
Narrow the polish-flow hook to depend only on the LLM runtime settings slice and to normalize via the new LLM-specific helper.
  • Change the usePolishFlow hook props type from full Settings to LlmRunSettings so it no longer requires unrelated ASR fields.
  • Replace usage of normalizeSettingsForStorage with normalizeLlmRunSettings when computing effective LLM settings before starting the polish stream.
hooks/use-polish-flow.ts
Extend unit tests to validate independent normalization of ASR and LLM runtime slices from full Settings.
  • Add a test that constructs a full Settings object with whitespace and custom values, and asserts that normalizeAsrRunSettings returns a cleaned ASR slice with appropriate defaults and trimmed values.
  • Add a corresponding test asserting that normalizeLlmRunSettings returns a cleaned LLM slice, including defaults for blank URL/model and trimming of API key and customInstructions.
  • Keep the existing storage normalization test to ensure secrets and behaviors for full Settings normalization are preserved.
tests/unit/utils.test.ts

Assessment against linked issues

Issue Objective Addressed Explanation
#22 Introduce narrower ASR and LLM runtime settings slices so ASR runs only consume ASR-related settings and LLM polish runs only consume LLM-related settings, instead of the full Settings interface. ✅
#22 Keep settings persistence behavior (env mapping, defaults, trimming, secret handling, and storage) centralized in the settings module without changing the existing save/reload/discard behavior of the settings page. ✅
#22 Add tests demonstrating that when the full Settings object is valid, the derived ASR and LLM runtime settings slices are each correctly normalized. ✅

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Jun 9, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@makoMakoGo, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 46 minutes and 9 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 954574e5-50d9-4cce-8a69-033ceb93f3f7

📥 Commits

Reviewing files that changed from the base of the PR and between 317d25b and 1fe5a48.

📒 Files selected for processing (4)
  • hooks/use-polish-flow.ts
  • lib/app-settings.ts
  • lib/asr-transcription.ts
  • tests/unit/utils.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/runtime-settings-slices

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors settings management by splitting the monolithic Settings type and its normalization logic into two smaller, independent runtime slices: AsrRunSettings and LlmRunSettings. This allows hooks and utilities to depend only on the specific configurations they require. The feedback suggests narrowing the parameter type of hasAsrApiKey to AsrRunSettings to fully align with this decoupled design, and adding defensive optional chaining in the normalization functions to prevent potential runtime TypeError crashes when handling partially missing configuration fields.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread lib/asr-transcription.ts Outdated
Comment thread lib/app-settings.ts

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@makoMakoGo

Copy link
Copy Markdown
Owner Author

Bot review 跟进:

  • Gemini 关于 hasAsrApiKey 输入类型收窄的建议已修复并 resolve。
  • Gemini 关于 optional chaining + fallback 的建议已 rebuttal 并 resolve:设置 payload 已由服务端完整校验,运行切片应暴露非法 shape,而不是静默 fallback。
  • Sourcery 二轮 review 通过。
  • CodeRabbit 返回 rate-limit 模板,没有 actionable 代码意见。

已重新验证:

  • npm test -- --run tests/unit/utils.test.ts tests/unit/asr-transcription.test.ts tests/unit/settings-persistence.test.ts tests/unit/polish-stream.test.ts
  • npm test
  • npm run build

@makoMakoGo
makoMakoGo merged commit 2463631 into main Jun 10, 2026
3 checks passed
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.

拆出 ASR / LLM 运行设置切片

1 participant