Skip to content

refactor: UI view splitting + code cleanup (P1-6, P1-7, P2-8)#14

Merged
ZhaoChaoqun merged 1 commit into
mainfrom
crew/refactor-dev-3
Mar 23, 2026
Merged

refactor: UI view splitting + code cleanup (P1-6, P1-7, P2-8)#14
ZhaoChaoqun merged 1 commit into
mainfrom
crew/refactor-dev-3

Conversation

@ZhaoChaoqun

Copy link
Copy Markdown
Owner

Summary

  • Split SettingsView.swift (399→~160 lines): Extracted SettingsASRView, SettingsDualEngineStatusView, and SettingsGeneralView as standalone sub-views
  • Split ChineseSpellingCorrector.correctSpelling() (157→~30 lines): Extracted runInference(), applyCorrections(), and evaluateCorrection() as focused helper methods
  • Cleaned up dead code: Removed 3 blocks of commented-out debug logging in KeyMonitor.swift

Test plan

  • Build succeeds (xcodebuild -scheme Typeless -configuration Debug)
  • Verify Settings UI renders correctly (model picker, download buttons, about section)
  • Verify CSC spelling correction still works end-to-end
  • Verify Fn key monitoring still triggers recording

🤖 Generated with Claude Code

@claude claude 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 Summary

Clean refactoring PR — no bugs or logic issues found.

What was reviewed

  • SettingsView.swift split: UI extracted into SettingsASRView, SettingsDualEngineStatusView, SettingsGeneralView. All logic preserved identically. @StateObject / @ObservedObject ownership pattern is correct.
  • ChineseSpellingCorrector refactoring: correctSpelling() split into runInference(), applyCorrections(), and evaluateCorrection(). Memory ownership of OrtValue is handled correctly — caller releases via defer, error paths in runInference release before returning nil.
  • KeyMonitor cleanup: Removed 3 blocks of commented-out debug logging. No behavioral change.

No issues flagged.

@ZhaoChaoqun

Copy link
Copy Markdown
Owner Author

Code Review: PR #14 — UI View Splitting + Code Cleanup

Overall Verdict: ✅ APPROVE

Clean, well-executed refactoring. All logic is preserved exactly, SwiftUI property wrappers are used correctly, and no meaningful code was lost. Ready to merge.


1. ChineseSpellingCorrector.swift — Method Extraction ✅

Logic preservation: Perfect.

The 157-line correctSpelling() was split into three focused methods:

  • correctSpelling() (~30 lines) — orchestrator
  • runInference(inputIds:seqLen:) — ONNX tensor setup + inference
  • applyCorrections(chars:inputIds:seqLen:logitsPtr:originalText:) — per-position correction loop
  • evaluateCorrection(tokenIndex:originalId:logitsPtr:vocabSize:) — single-position argmax + threshold logic

Detailed verification:

Aspect Status Notes
Argmax loop Identical logic in evaluateCorrection
Logit diff threshold (5.0) Preserved exactly
Softmax probability threshold (0.9) Preserved exactly
Chinese-only correction guard BertTokenizer.isChinese() check preserved
UNK token guard maxIdx != tokenizer.unkId preserved
20% correction ratio sanity check Preserved in applyCorrections
return textreturn originalText Correct parameter rename for clarity

Memory management — the critical part:

The outputValue (OrtValue) ownership transfer was handled carefully:

  • Original: defer { api.pointee.ReleaseValue(outputValue) } inside correctSpelling()
  • Refactored: runInference() returns the outputValue as part of the tuple, and the defer in correctSpelling() releases it. The defer that was on the original outputValue inside runInference was correctly removed.
  • Error paths in runInference: When GetTensorMutableData fails or logitsPtr is nil after outputValue has been obtained, the code now explicitly calls api.pointee.ReleaseValue(outputValue) before returning nil. This is correct — the defer on outputValues[0] was removed so it must be released manually on these error paths.

This is the trickiest part of the PR and it's done right. No leaks, no double-frees.


2. SettingsView.swift Split ✅

Original (399 lines) → Refactored (~160 lines) + 3 new files.

The body of SettingsView is now just:

Form {
    SettingsASRView(downloadManager: downloadManager)
    SettingsGeneralView()
}

Property wrapper correctness:

View Property Wrapper Correct?
SettingsView downloadManager @StateObject ✅ Owner — creates the instance
SettingsASRView downloadManager @ObservedObject ✅ Receiver — observes without owning
SettingsDualEngineStatusView downloadManager @ObservedObject ✅ Same — passed down from ASRView
SettingsGeneralView (none) ✅ No state needed — static content

UI behavior preservation:

  • All Section headers, footers, and content are character-for-character identical
  • The Picker binding with custom get:/set: calling switchModel(to:) is preserved
  • Model status views (single model, dual engine, punctuation, CSC) — all identical
  • The rename from dualEngineStatusView() (private method) → SettingsDualEngineStatusView (struct) is clean

3. KeyMonitor.swift Cleanup ✅

Three blocks of commented-out debug logging removed:

  1. // let keyCode = event.getIntegerValueField(...) + associated logger.debug
  2. // logger.debug("Fn 键状态: ...")

All removed lines were 100% commented-out code with no active logic. The remaining active logger.info calls for "Fn 键按下" / "Fn 键松开" are preserved. No functional change whatsoever.


4. project.pbxproj ✅

Three new files properly registered:

  • PBXBuildFile section: 3 entries (1ASV0001, 1ASV0002, 1ASV0003)
  • PBXFileReference section: 3 entries (1ASV0010, 1ASV0020, 1ASV0030)
  • PBXGroup (Sources): 3 file references added next to SettingsView.swift
  • PBXSourcesBuildPhase: 3 build file entries added

ID naming convention (1ASV*) is consistent with the project's existing pattern. All three new .swift files are included in the build.


Summary

Criterion Result
Refactoring preserves all existing UI behavior? ✅ Yes
SwiftUI @Binding/@ObservedObject correctly used? ✅ Yes
New files properly added to project.pbxproj? ✅ Yes (3 files, all 4 pbxproj sections)
ChineseSpellingCorrector logic preserved exactly? ✅ Yes (including memory management)
No accidental removal of meaningful code? ✅ Only commented-out debug logs removed

No issues found. Good refactoring — clean splits at natural boundaries, no behavioral changes.

…(P1-6, P1-7, P2-8)

- SettingsView: Extract SettingsASRView, SettingsDualEngineStatusView,
  SettingsGeneralView as standalone sub-views
- ChineseSpellingCorrector: Split 157-line correctSpelling() into
  runInference(), applyCorrections(), and evaluateCorrection()
- KeyMonitor: Remove commented-out debug logging statements

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ZhaoChaoqun ZhaoChaoqun force-pushed the crew/refactor-dev-3 branch from ca216c8 to 88d21b3 Compare March 23, 2026 08:37
@ZhaoChaoqun ZhaoChaoqun merged commit 209cb26 into main Mar 23, 2026
5 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.

1 participant