Skip to content

refactor: tidy useGuidedPhraseSegments after TT-7661 - #771

Merged
gtryus merged 2 commits into
developfrom
guided-phrase-segments-cleanup
Oct 21, 2026
Merged

gtryus merged 2 commits into
developfrom
guided-phrase-segments-cleanup

Conversation

@nabalone

Copy link
Copy Markdown
Collaborator

Follow-up cleanup from the #762 (TT-7661) review. No behavior change.

  • useGuidedPhraseSegments no longer returns the redundant phraseSegString; callers read phraseSegs.segments (the media-stamped value).
  • Internal setter renamed to setPhraseSegsForCurrentMedia to make clear it stamps the current recording's id (still exposed as setPhraseSegString).
  • Removed the unused useCarefulSpeechSegments wrapper and its re-export file useCarefulSpeechSegments.ts.
  • Test mocks updated to match.

Test plan

  • npm run typecheck
  • npx jest src/components/PassageDetail — 337/337 pass
  • Smoke Careful Speech / Phrase BT in the app: segments still load and switching recordings still reloads them

🤖 Generated with Claude Code

Follow-up cleanup from the #762 review. Drop the redundant phraseSegString
return value (callers read phraseSegs.segments), rename the internal setter
to setPhraseSegsForCurrentMedia so it's clear it stamps the current media id,
and delete the unused useCarefulSpeechSegments wrapper and re-export file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

It is a small, verified no-behavior-change cleanup where all removed symbols are confirmed unused and all callers/mocks are consistently updated.

No open issues 🎉

What changed in this PR

This is a follow-up cleanup to PR #762 (TT-7661) with no intended behavior change. It tidies useGuidedPhraseSegments by removing a redundant return value and dead code, and renames an internal setter for clarity. The hook powers Careful Speech / Phrase Back Translation segment loading in the PassageDetail recording flow.

Changes:

  • Dropped the redundant phraseSegString from the hook's return (callers now read phraseSegs.segments, the media-stamped value), and derived clauseSegString locally in PassageDetailGuidedPhraseRecord.tsx.
  • Renamed the internal setter to setPhraseSegsForCurrentMedia (still exposed publicly as setPhraseSegString) to clarify it stamps the current recording's id.
  • Removed the unused useCarefulSpeechSegments wrapper and deleted its re-export file useCarefulSpeechSegments.ts; updated five test mocks to match the new return shape.
File Description
carefulSpeech/​useGuidedPhraseSegments.ts Renames internal setter, drops redundant phraseSegString return, removes unused useCarefulSpeechSegments wrapper
carefulSpeech/​useCarefulSpeechSegments.ts Deletes the now-unused re-export file
PassageDetailGuidedPhraseRecord.tsx Derives clauseSegString from clauseSegs.segments instead of the removed return field
carefulSpeech/​useGuidedPhraseSegments.test.tsx Reads phraseSegs.segments in assertions
PassageDetailPhraseBackTranslate.undo.test.tsx Drops phraseSegString from the hook mock
PassageDetailGuidedPhraseRecord.stepScope.test.tsx Drops phraseSegString from the hook mock
PassageDetailGuidedPhraseRecord.nextDuringPlayback.test.tsx Drops phraseSegString from the hook mock
PassageDetailCarefulSpeech.test.tsx Drops phraseSegString from the hook mock

I verified that the deleted useCarefulSpeechSegments.ts file has no remaining importers, that no caller reads .phraseSegString off the hook result, that GuidedPhraseSegmentsOptions is only consumed in-file, and that the in-hook imports remain in use. The derived clauseSegString = clauseSegs.segments is equivalent to the previously returned value, so behavior is preserved. No issues found.

Review effort: Balanced

@nabalone
nabalone marked this pull request as ready for review October 19, 2026 21:49

@gtryus gtryus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One issue remains, and both reviewers flagged it. The #762 reviewer (nabalone) asked for the setter to be renamed "so it doesn't look like a state setter". The PR renames it only inside the hook. It still returns it as setPhraseSegString (useGuidedPhraseSegments.ts:317). The caller then renames it again to setClauseSegString (PassageDetailGuidedPhraseRecord.tsx:448) and wraps it in setClauseSeg, so one setter goes by four names. This breaks CODING_STANDARDS "Naming conventions" (a judgement call, a Mysterious Name smell). The goal is to expose the honest name at the API boundary.

Return the setter under its honest name instead of setPhraseSegString,
and drop the caller's extra setClauseSegString alias. The caller's
ref-syncing wrapper becomes setClauseSegsForCurrentMedia, so the setter
goes by one name in the hook and one in the caller.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@gtryus
gtryus merged commit 2d5aca2 into develop Oct 21, 2026
2 checks passed
@gtryus
gtryus deleted the guided-phrase-segments-cleanup branch October 21, 2026 14:24
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.

3 participants