Conversation
|
Rebased onto current The web path now uses NextChat’s same-origin proxy instead of requiring browser CORS from the transcription server; desktop builds continue to call local endpoints directly. Blank model values are rejected before upload, and the settings copy now follows the existing English/Simplified Chinese locale structure. Exact-head validation:
The only full-lint limitation remains the repository tooling crash in unchanged |
codeCraft-Ritik
left a comment
There was a problem hiding this comment.
Really solid work. I appreciate that the optimization focuses on eliminating unnecessary allocations while maintaining the same parser behavior. The additional regression tests around slicing, concatenation, and lazy evaluation make this a well-rounded change. 👏 @LauraGPT
46f6736 to
640c0b8
Compare
Signed-off-by: zhifu gao <18321252+LauraGPT@users.noreply.github.com>
640c0b8 to
4fd2ab7
Compare
|
@Leizhenpeng, could you review this configurable voice-transcription integration when convenient? It addresses #6803 without bundling a speech runtime: web deployments use the existing same-origin proxy, desktop builds can call local OpenAI-compatible /v1/audio/transcriptions endpoints directly, and the default behavior remains unchanged. I revalidated the unchanged signed+DCO head 4fd2ab7 against the current main@defdcdb55d850cd12c4c657eb83729fd66e215c0 on 2026-08-30:
I tried GitHub’s formal reviewer-request API first, but fork authors do not have RequestReviewsByLogin permission. I will respond promptly to any maintainer feedback and will not repeat the ping. |
|
Revalidated the current PR head locally against the current
The only lint diagnostic is an existing |
Summary
/v1/audio/transcriptionsendpoint, model name, and optional bearer token.http://localhost:8000/v1and modelsensevoice, without bundling a speech runtime. Endpoint availability, accepted recording formats, and model-name support are deployment requirements, not verified interoperability with every compatible server.Closes #6803.
Outstanding Issues
This PR is not ready to claim complete recorder cleanup or the server-held credential boundary requested in #6803.
Recorder Lifecycle
On published head
4fd2ab76508367bba254b5dda307fe36f0538264,startAudioRecording()registersonerrorandonstoponly when the caller invokesstop(). If the recorder errors or stops before that call, its acquired stream tracks are not released by this utility. A subsequentstop()rejects withAudio recording is not activebefore releasing them.A deterministic, device-free probe invoked the published utility with injected recorder/stream doubles, then delivered terminal events before calling
stop(). Both the error and spontaneous-stop cases observed zero track releases instead of one. This is a reproduced utility-level cleanup failure, not a claim of real-browser/device testing. A bounded lifecycle correction and regression tests are pending; no source fix has been pushed.Credential and Deployment Boundary
The same-origin proxy removes a browser-to-service CORS requirement for the web path; it does not make configured credentials server-only. The browser sends the endpoint in
x-base-urland the optional token inAuthorization. The token resides in the client access store, whose persistence uses IndexedDB. It is not hidden from the user's browser. The existing proxy also has provider-specific authorization behavior, so universal token-forwarding compatibility is not established by this PR's mocked tests.For web deployments,
localhostrefers to the NextChat server/container's network context, not the browser user's computer. The service must be reachable from that server. The desktop path uses its own device's network context. Microphone access still depends on browser/device support and permissions.The server-held credential requirement in #6803 remains unmet by the current client-configured implementation. Do not treat this description as relaxing that requirement or as a completed security review.
Verification
Fresh Checks: 2026-09-22
Published head is unchanged. Checks used the existing checkout dependencies, Node 24.18.0, and repository-pinned Yarn 1.22.19:
yarn test:ci --runInBand --runTestsByPathfor the four transcription suites: 4 suites, 19 tests passed. Existing IndexedDB hydration warnings were printed.No fresh full-suite, TypeScript, lint, build, browser microphone, or real transcription-service validation was performed this pass.
Historical Checks Only
Earlier comments reported 38 suites / 176 tests, TypeScript validation, targeted ESLint, and production build success on this head. They also described an
unused-imports/no-unused-importslint-rule crash in unchangedapp/constant.ts. Those are historical results, not rerun or reconfirmed by the current focused checks.This PR was developed with AI coding agents; the present pass is a self-review, not independent maintainer approval.