Skip to content

Added 60dB integration#20

Merged
steipete merged 2 commits into
steipete:mainfrom
manishEMS47:main
Jun 10, 2026
Merged

Added 60dB integration#20
steipete merged 2 commits into
steipete:mainfrom
manishEMS47:main

Conversation

@manishEMS47

Copy link
Copy Markdown

No description provided.

@clawsweeper

clawsweeper Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed June 9, 2026, 1:39 AM ET / 05:39 UTC.

Summary
The PR adds a provider-neutral TTS interface, a 60db HTTP client, provider auto-selection from API-key environment variables, command/docs updates, and provider-focused tests.

Reproducibility: yes. for the review finding by source inspection: the PR forces MP3 for 60db playback before the shared stream can be written to --output. I did not run a live 60db account test.

Review metrics: 2 noteworthy metrics.

  • Diff surface: 15 files, +1072/-96. This is a broad provider integration rather than a narrow compatibility patch.
  • Provider contract changes: 1 new shared interface, 1 new provider client, 2 commands rerouted. The merge changes auth/provider routing for both speak and voices.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted live terminal output or logs for 60db provider selection, sag voices, and one synthesis command against a real 60db account.
  • [P1] Fix or explicitly reject the 60db --play plus non-MP3 --output path so saved files match their requested format.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No PR body, comment, terminal output, logs, or media show real after-change 60db behavior; the contributor should add redacted terminal/live output or a recording and update the PR body to trigger a fresh review, or ask a maintainer for @clawsweeper re-review if needed.

Risk before merge

  • [P1] With 60db selected, --play -o out.wav can save MP3 bytes under a WAV filename because playback forces opts.outputFmt to MP3 before the shared stream is written to disk.
  • [P1] The 60db wire mapping is covered by mocked httptest cases only; there is no real provider proof for voice listing, synthesis, or streaming.
  • [P1] Provider auto-selection and shared --base-url change auth/provider routing, so maintainers should explicitly accept that policy before landing a second backend.

Maintainer options:

  1. Fix format handling and require live proof (recommended)
    Before merge, preserve or reject mixed 60db --play plus non-MP3 output combinations and require redacted live 60db terminal/log proof for provider selection, voice listing, and synthesis.
  2. Accept auto-detection as policy
    Maintainers can intentionally accept env-key auto-selection and the shared --base-url behavior after deciding that no explicit provider flag is needed.
  3. Pause provider expansion
    If 60db is not a core provider direction, pause or close this PR and ask for a narrower external integration path instead.

Next step before merge

  • [P1] Needs maintainer provider-policy review, contributor real behavior proof, and a targeted fix for the 60db output-format bug before merge.

Security
Cleared: No concrete security or supply-chain issue was found; the diff adds no dependencies or workflows, though provider credential routing remains a merge policy risk.

Review findings

  • [P2] Preserve the requested output format when saving — cmd/speak.go:128
Review details

Best possible solution:

Land the provider abstraction only after saved audio formats stay honest, provider routing is an explicit maintainer-approved policy, and the PR includes redacted live 60db proof.

Do we have a high-confidence way to reproduce the issue?

Yes for the review finding by source inspection: the PR forces MP3 for 60db playback before the shared stream can be written to --output. I did not run a live 60db account test.

Is this the best way to solve the issue?

No, not yet: the provider abstraction is plausible, but the current implementation can save mislabeled audio and still needs live API proof plus maintainer approval for provider auto-selection.

Full review comments:

  • [P2] Preserve the requested output format when saving — cmd/speak.go:128
    When 60db is active, this branch sets opts.outputFmt to MP3 whenever playback is enabled. Because streamAndPlay tees the single response into both the player and --output, sag --play -o out.wav ... will save MP3 bytes in a .wav file instead of honoring extension inference; either reject that combination or keep the saved format honest.
    Confidence: 0.84

Overall correctness: patch is incorrect
Overall confidence: 0.78

AGENTS.md: not found in the target repository.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 325fbff796c5.

Label changes

Label changes:

  • add P2: This is a normal-priority feature PR with a concrete output-format bug and provider-routing review risk, but no emergency user impact.
  • add merge-risk: 🚨 compatibility: The PR can write audio bytes in a format that does not match the user's requested output filename when 60db playback is enabled.
  • add merge-risk: 🚨 auth-provider: The PR changes provider selection and credential routing based on new and existing API-key sources.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No PR body, comment, terminal output, logs, or media show real after-change 60db behavior; the contributor should add redacted terminal/live output or a recording and update the PR body to trigger a fresh review, or ask a maintainer for @clawsweeper re-review if needed.

Label justifications:

  • P2: This is a normal-priority feature PR with a concrete output-format bug and provider-routing review risk, but no emergency user impact.
  • merge-risk: 🚨 compatibility: The PR can write audio bytes in a format that does not match the user's requested output filename when 60db playback is enabled.
  • merge-risk: 🚨 auth-provider: The PR changes provider selection and credential routing based on new and existing API-key sources.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No PR body, comment, terminal output, logs, or media show real after-change 60db behavior; the contributor should add redacted terminal/live output or a recording and update the PR body to trigger a fresh review, or ask a maintainer for @clawsweeper re-review if needed.
Evidence reviewed

What I checked:

  • Target policy check: No AGENTS.md exists under the target repository, so no target-specific repository policy changed the review.
  • Current main is ElevenLabs-only: Current main still exposes ElevenLabs-focused API-key/base-url configuration and has no 60db provider symbols. (cmd/root.go:46, 325fbff796c5)
  • PR adds provider auto-selection: The PR introduces provider selection based on ElevenLabs and 60db key resolution and returns either an ElevenLabs or 60db client. (cmd/provider.go:60, e01f63afad6c)
  • Output-format defect in PR branch: When 60db is selected, the PR forces MP3 output whenever playback is enabled before the same stream can be written to the requested output path. (cmd/speak.go:128, e01f63afad6c)
  • 60db behavior is mock-tested only: The new 60db tests cover mocked voice listing, conversion, and NDJSON streaming with httptest rather than a real 60db account or live API output. (internal/sixtydb/client_test.go:84, e01f63afad6c)
  • No contributor proof or review discussion: The PR body is empty and the only issue comment is the ClawSweeper placeholder; there are no human review comments, logs, screenshots, recordings, or linked artifacts showing real 60db behavior. (e01f63afad6c)

Likely related people:

  • steipete: Current-main blame and log history for the core TTS command, ElevenLabs client, README, and docs site all point to Peter Steinberger’s release/docs commits in this area. (role: original implementation author and recent area contributor; confidence: high; commits: 55138a6fc03a, 937a619bac7d, 325fbff796c5; files: cmd/speak.go, internal/elevenlabs/client.go, cmd/voices.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. labels Jun 9, 2026
Co-authored-by: manishEMS47 <manish.prasad@engineermaster.com>
@steipete steipete merged commit a2259e1 into steipete:main Jun 10, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants