Skip to content

feat: return tri-state outcome from inject_text instead of bool - #225

Merged
goodroot merged 2 commits into
mainfrom
inject-text-outcome
Jul 29, 2026
Merged

feat: return tri-state outcome from inject_text instead of bool#225
goodroot merged 2 commits into
mainfrom
inject-text-outcome

Conversation

@goodroot

Copy link
Copy Markdown
Owner

Follow up to #224

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown

Continue PR Review

Automated review via Continue CLI run through opub.

Summary

The tri-state refactor looks mostly consistent and the updated tests cover the main behavior changes. I did not find any obvious security issues in the diff. The main remaining concern is backward compatibility for any callers that still treat TextInjector.inject_text() as boolean, plus a missing direct regression test around main._inject_text()/capture mode semantics.

Findings

  • lib/src/text_injector.py: TextInjector.inject_text() now returns an InjectionOutcome enum instead of bool. This PR updates the known call sites in main.py and longform_controller.py, but if there are any other internal or downstream callers comparing the result with is True / is False or serializing it, they will silently misbehave because enum truthiness differs from strict booleans. Before merge, grep for all callers and either update them or document this as a breaking API change.
  • Missing regression test for lib/main.py::_inject_text() capture mode and consumed outcome handling. The new behavior treats both INJECTED and CONSUMED as non-failures and also maps capture-subscriber mode to InjectionOutcome.INJECTED; that’s reasonable, but currently only AST-shape tests verify main.py. A behavioral unit test should assert:
    • capture subscriber bypasses TextInjector and returns InjectionOutcome.INJECTED
    • InjectionOutcome.CONSUMED does not trigger failure/error UI paths
    • InjectionOutcome.FAILED still does

Suggested follow-ups

  • Add a repository-wide grep/test pass for all inject_text( callers to confirm no boolean-only consumers remain.
  • Add direct behavior tests for hyprwhsprApp._inject_text() and _process_audio() using mocked injector outcomes rather than only AST inspections.

@goodroot
goodroot merged commit 64af741 into main Jul 29, 2026
2 checks passed
@goodroot
goodroot deleted the inject-text-outcome branch July 29, 2026 17:32
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