Repository navigation
Make the processing loop testable, give config one owner, fix review bugs - #12
Merged
Merged
Conversation
added 30 commits
September 28, 2026 17:58
In test mode the input handler returned before the recorder ran and before it published the status. A recording in progress then stayed open for the whole test: the GUI showed it as recording, no StopRecording turned off the VRChat typing indicator, and after the test the recording went on and joined the audio from before and after the test. When test mode starts, the input handler now calls Recorder::discard. It drops the samples of the recording in progress, resets its state and closes the noise gate, so the next loud buffer after the test starts a new recording. If a recording was in progress, the handler sends RecordingDiscarded, which the processing side logs, and then StopRecording. The status it publishes in test mode shows no recording.
save_recording aborted process_audio with `?` whenever the debug recording write failed, for example because the recordings folder was not writable. That dropped the transcription and repeated the same failure on every utterance. RecordingManager now keeps a FailureLog, the way PriceEstimator logs a failed cost save: a failed save is logged once until it fails differently, a save that works again logs a recovery line, and save_recording itself returns nothing, so process_audio has no error to propagate and always goes on to translate.
run_processing_loop and finish_test_recording were in main.rs, so no test could run the loop: its select order, the handling of each audio event, the typing indicator rules, the translation toggle, the test recording and the shutdown were not tested. They are now in processing_loop.rs, with two parameters in place of the devices: - start_audio returns an AudioInput, the startup result and the event channel of the audio input. The loop calls it after it opens the OSC socket and creates the HTTP client, as before, so a failure there does not leave audio capture running. - open_output opens a PlaybackOutput for each test recording playback. AudioOutput implements this new trait in audio_playback.rs. main.rs keeps the thread that starts the cpal input stream and gives AudioOutput::open_default as the output. The loop itself does not change.
The tests run run_processing_loop with fakes for the devices: the audio input is a channel that the test fills, VRChat is a local UDP socket, and the output records what it plays. No test opens a device or sends an HTTP request. Each test waits for an effect of the loop (an OSC message, an activity log line, a playback or the end of the loop) with a timeout, and requests shutdown only after the effect it checks. They cover the typing indicator for a recording that is too short to send, for a discarded recording and for an input error; speech while translation is off; a startup error of the audio input, and a busy OSC port that ends the loop before the audio input starts; the test recording played at Stop and at its 30 s limit, with the playback kept until the loop ends; shutdown while process_audio waits, and shutdown before events that wait in the channel. The tests of the short recording, speech while translation is off, the discarded recording, the input error and shutdown also check that the loop turns the typing indicator off when it ends. The tests of the audio start error, the busy port and the playback do not check the typing indicator.
`OpenAi` in api_client.rs holds the HTTP client and the base URL, and sends JSON or multipart requests with the API key as a bearer token. Transcription and translation use it instead of building their own requests with hardcoded URLs and two ways to set the auth header. A failed request is an `ApiError`: a transport error, or an HTTP status with its body. Its `Display` gives the message for the user that `Logger::error_api` used to get by searching the error text for JSON. The mappings and the character based cut are unchanged. A body without an OpenAI error message now shows its status and a shortened body. Both functions are removed. The processing loop logs an `ApiError` with its message in the activity log and the status and raw body on stderr. Other errors, such as a chatbox send error, are logged as they are and are no longer parsed for JSON. A transport error now starts with "Request to the OpenAI API failed" instead of "Error". `run_processing_loop` takes the base URL; main.rs passes `OPENAI_BASE_URL`. Tests use a local HTTP server (`test_server`) in place of the proxy that closed every connection. The loop tests use port 1 of the local host by default, so no test request leaves the machine.
The loop tests now send their requests to a local HTTP server: - An utterance is transcribed, translated and sent to /chatbox/input. The total cost holds both requests, and typing goes on and then off. - A part of a long recording is sent, and typing goes on again after it until the rest of the recording is sent. - An API error shows its message for the user in the activity log, and nothing reaches /chatbox/input. - An error that is not from the API, such as a transcription body without text, is logged with "Error: " and its text. `Driver::logged` returns the line it found, so a test can check its level and whole text.
The GUI wrote AppState.config on Save and then sent UpdateConfig. The loop read the lock once per utterance, but the rate limiter, the prices, the recording manager and the audio parameters changed only when the command was handled, and the typing indicator read the lock on every send. One utterance could use the new model with the old prices, and typing could go on at one address and off at another. Remove AppState.config. run_processing_loop takes the settings at start, and only the UpdateConfig arm replaces them, together with everything that depends on them. Each utterance borrows the settings of the loop, and a command is handled only between utterances. The loop also sets the audio parameters before the audio input starts, so AppState::new takes no settings. The GUI gets its settings as an argument. TypingIndicator takes the settings with each call, as Chatbox does, so it holds no copy that can differ from the settings of the loop. When the address or output port changes, the loop turns typing off at the old destination, because the StopRecording goes to the new one.
The loop bound the OSC socket once at start, so a new Address or Input Port in the settings had no effect until a restart, and the GUI did not say so. When UpdateConfig changes osc.address or osc.input_port, the loop binds a new socket and gives it to the typing indicator and the chatbox, which drop the old one. The activity log shows the new address. If the bind fails, the old socket stays in use and the error says so. The loop compares with the address of the socket in use, not with the old settings, so a port that was busy opens when the settings are saved again, and a change back to the port in use opens nothing.
send_command used blocking_send on the 32 command channel. The loop reads a command only between utterances, so a long request and many queued commands could stop the GUI thread until the loop caught up. send_command now uses try_send: a full or closed channel is an error in the status bar, as a closed channel was before. Save still writes the file first; if the send fails, the status says the settings were saved to the file but not applied.
AppState held ten separate Arc fields for the audio callback. SharedAudioState cloned them one by one, publish spread RecorderStatus over six atomics, and the GUI loaded them one by one, so a frame could show fields of two different buffers. AppState now holds one Arc<AudioShared> with the hot reload params, the input level, the recorder status and the test mode flag and buffer. The callback gets the same Arc. The status is one Mutex<RecorderStatus>: the callback publishes with try_lock and skips the publish while the GUI copies it; the GUI reads it with lock and recovers a poisoned lock. The callback now publishes on every test mode buffer too, so a publish that was skipped when test mode started is not lost. Every f32 atomic is an AtomicF32. AudioParams gives the recorder its settings in one call. Tests: the GUI shows the texts of one published status; the callback returns while the status lock is held and the next buffer publishes; a status skipped at the start of test mode is published later; changed gate threshold and hold time reach the recorder; concurrent reads never see a status mixed from two publishes.
process_audio (audio_processing.rs) carried the crate's only too-many-arguments allow, and several of its arguments were already grouped in ProcessingServices in the processing loop. DeliveryFixture in tests.rs built nearly the same set of values again. Rename audio_processing.rs to pipeline.rs (git mv) and add Pipeline, which owns the OpenAI client, the chatbox, the typing indicator, and ProcessingServices, plus a reference to AppState. process_audio becomes Pipeline::process(&mut self, upload, extent, &Config); accept_transcription and deliver_translation become methods with the same signature reduction. ProcessingServices keeps its own new and apply_config, now defined in pipeline.rs next to its only user, so its existing unit tests are untouched. The processing loop builds one Pipeline after the audio stream starts and calls pipeline.process, pipeline.typing_indicator and pipeline.services.apply_config where it used to call process_audio, typing_indicator and services.apply_config directly; a new Pipeline::set_socket keeps the OSC rebind path to one call. Remove the #[allow(clippy::too_many_arguments)]; no #[allow] is left in src. DeliveryFixture now builds a Pipeline instead of duplicating its fields, and calls accept_transcription/deliver_translation as methods. Mutation checked by hand: skipping the cost add on the no-translation path, skipping the enabled check, and skipping the final stop_typing each turn tests.rs and processing_loop.rs tests red; reverted after confirming. processing_loop tests run 19/19 green over 10 repeats.
on_exit already requests shutdown through Shutdown, and the loop checks shutdown first in its biased select, so Quit was a second way to stop the loop. The None arm of the command channel still ends the loop. The comments that this commit adds in on_exit and in the None arm say that the channel closes when the GUI drops its sender. That is not correct: the sender is AppState::command_tx, and the loop holds app_state. A later commit corrects the comments.
The Logger already owns a clone of the sender; AppState.log_tx was kept only to give test_app() a sender to inject log entries with. test_app() now builds its own channel and AppState directly instead of reading the sender back off AppState.
Neither field is read anywhere in the codebase.
Both computed the same interleaved-sample count for a duration. recorder::samples_in is now pub(crate) and TestRecording::start calls it instead of a duplicate local function.
The rate limiter's timing test previously worked around the real clock advancing slightly during a paused-clock test by asserting waits[2] >= 59s. Switching RateLimiter to tokio::time::Instant makes all its timing consistent with the paused test clock, so the third wait is exactly 60s; the test now asserts the exact durations.
When binding osc.address:input_port fails because the port is
already in use, both the startup error and the rebind error now
append a hint that another OSC app may be listening on the port and
name the setting to change (OSC Settings, Input Port). The existing
message prefixes ("cannot open OSC port ..." at startup, "Cannot
open OSC port ... The old port ... stays in use." on rebind) are
unchanged.
input_port stays a separate setting from output_port for a future
OSC passthrough, per the 2026-09-29 decision recorded in
IMPLEMENTATION_PLAN.md item 11.8.
calculate_audio_duration re-read the duration from the WAV the app had just encoded. CapturedAudio::duration() now computes it directly from the captured samples (frames / sample_rate), the same source the recorder itself uses, with the sample rate and channel count both clamped to 1 as Recorder::new clamps the sample rate, instead of dividing by zero. encode_for_upload now returns the WAV together with this duration, and Pipeline::process takes the duration as a parameter instead of deriving it from the upload bytes. min_transcription_duration and its checks are unchanged. The zero sample rate WAV test is replaced by a direct test of CapturedAudio::duration() with a zero sample rate, since the guard now lives there instead of in a WAV read that no longer happens.
src/tests.rs held tests for many modules in two large blocks (regression_tests, gui_tests) while other modules already had their own #[cfg(test)] mod tests. Move every test to sit next to the module it tests, in its existing test module where one exists. Add #[cfg(test)] mod test_support (src/test_support.rs) for helpers shared by two or more modules: a log capture (Logger plus a receiver with an entries() drain, replacing separate new_entries copies in price_estimator.rs, typing_indicator.rs and recording_manager.rs), an AppState builder, a self cleaning TempDir (removed on drop, including after a panic, and made writable first so a read-only fixture can still be removed), an OSC receiver with decode (the Osc enum and decode that used to live only in processing_loop.rs's Driver), sine and zero_crossings (previously duplicated in upload_audio.rs, resample.rs and audio_playback.rs), the Chat Completions response body builders and usage fixtures, the price estimator and pricing constants, and the minimum transcription duration check that audio_recording.rs used to import from crate::tests::regression_tests. gui.rs's tests (gui_tests) move inline into gui.rs, matching the existing convention of processing_loop.rs, which already carries a large inline test module rather than a child file. Narrow back to private every item on BabbleBoopApp (config_draft, ui, audio_settings_ui, send_command, set_status_info, config_to_save, note_replaced_values) and on Pipeline (chatbox field, accept_transcription, deliver_translation) that pub(crate) had widened only so src/tests.rs could reach them; they are only reached from their own module's test child now. api_client.rs's with_timeouts also goes back to private. Every remaining pub(crate) item was checked against a real cross-module caller, production or test, and kept. recording_manager.rs is no longer unix only as a whole: only the two tests that need PermissionsExt keep #[cfg(unix)], so the recording name tests run on every platform, as they did in src/tests.rs. Delete src/tests.rs and its mod declaration in lib.rs. Test count is unchanged at 251, confirmed by name-for-name diff of `cargo test -- --list` before and after. Full gate (fmt, clippy -D warnings, cargo test) run clean 5 times in a row. Confirmed no babble_boop_* leftovers under the temp directory after the run, and separately confirmed by a local, uncommitted test that TempDir's Drop removes its directory when the test body panics.
Documents the threads (GUI, processing loop, audio input thread and its real time callback, cpal output callback, spawn_blocking work), the data flow of one utterance from the audio callback through the Pipeline to VRChat OSC, the thread communication mechanisms in a table (AudioShared atomics and try_lock, the bounded mpsc channels, the audio start oneshot, the Shutdown watch, GuiWaker), the rules a contributor must follow, the test layout and loop harness, and where BabbleBoop keeps its files at run time. Links it from the README near the build instructions.
RecordingManager::save_recording ran create_dir_all, read_dir, metadata and remove_file inline on the processing loop's async task, mixed with a tokio::fs::File. PriceEstimator::add_cost wrote total_cost.txt with fs::write from Pipeline::process, and the loop read that file at startup in ProcessingServices::new. A disk that does not answer stopped the loop and its shutdown check, although ARCHITECTURE.md says the save runs through spawn_blocking. The recording save, the cost save and the startup cost load now run on the blocking pool with std::fs. The startup load is wrapped in shutdown.run_until. A panic in a save task is logged as a failed save, with a message without the task id, so FailureLog still logs it once. The tokio::fs::File write returned Ok before the data was written and the file was never flushed, so a failed write (a full disk) was not logged. std::fs::write reports it, and the partly written file is removed so the cleanup does not count it as a recording. A test checks the full disk case with /dev/full. No test checks that the file work stays off the async task. ARCHITECTURE.md now names the file work that runs on the blocking pool, and says that nothing checks this rule automatically; it is enforced by code reading.
Clippy for x86_64-pc-windows-gnu with all targets failed with three errors after the tests moved next to their modules (acb4497). recording_manager.rs imported LogLevel and LogCapture for every platform, but only the tests gated on unix (and the Linux tests, a subset) use them; the imports now have the same unix gate. make_writable in test_support.rs calls set_readonly(false) in its branch for platforms other than Unix, which trips clippy::permissions_set_readonly_false. The lint warns that on Unix this makes the file writable by all users; the branch is not built for Unix, and on Windows std only clears FILE_ATTRIBUTE_READONLY, so the call keeps an expect with that reason. CI ran clippy on Linux only and a plain cargo build on Windows, which does not build the tests. The Windows job now runs the same clippy command as the Linux job before the build. ARCHITECTURE.md names the Windows clippy command next to the gate.
Save writes the file and then sends UpdateConfig with try_send. When the command channel was full, the status said "not applied", but Save was disabled and the unsaved indicator was gone, so the processing loop kept the old settings until another change was saved or the app was restarted. The GUI now records that the saved settings are not applied. Save stays enabled and sends them again, and the indicator shows "Settings not applied" when the file itself is up to date. A successful send clears the flag. Reset still discards only the draft edits.
The GUI stores the translation toggle before it sends SetEnabled, and since the GUI uses try_send the command is lost while the command channel is full. The loop relied on SetEnabled(false) to turn the VRChat typing indicator off, so it could stay on: - A recording that started while translation was on ended after it was switched off. The loop ignored every audio event while translation was off, including the StopRecording. - A part of a long recording was processed while translation was switched off. After the part the loop turned typing on again without checking the toggle. The loop now handles StopRecording also while translation is off and turns typing on after a part only while translation is on. The off at a StopRecording while translation is off also goes out when BabbleBoop did not turn typing on, and such an off can clear the typing indicator of another app that uses the chatbox. Two loop tests switch translation off without sending SetEnabled, one for each case.
The loop decided to rebind by comparing address strings, and binds the new socket while the old one is still open. So a change of only the spelling of the address, such as 127.0.0.1 to localhost on the same input port, failed against BabbleBoop's own socket. The activity log then said that another OSC app may be listening on the port, and every later save logged the same error again. The rebind now resolves the new address first, under shutdown.run_until as a host name can wait for DNS. If one of the addresses is the address of the socket in use, the socket stays and the new address is recorded as the one in use, so a later save does not try again. The bind then uses the resolved addresses, so DNS is asked once. When the bind fails because the port is in use, and the socket in use has the same port with a wildcard address on either side (for example 127.0.0.1 and 0.0.0.0), the socket in use can be what holds the port. The log then says that BabbleBoop has this port open at the old address and that a restart applies the new address. The old socket stays open until a new one is open, as before. The startup message and the message for a port that another app holds are unchanged. Tests: a change to localhost logs no error on two saves and every message comes from the socket of the start; on Linux, a change to 0.0.0.0 on the same port does not blame another app and keeps the old socket open. Windows allows that second bind, so the test is Linux only.
Since the cost save runs on the blocking pool, the process can exit while it runs: main waits at most 1 s for blocking tasks. The save uses fs::write, which empties the file before it writes, so an exit in between leaves total_cost.txt empty. At the next start an empty file loaded as 0 with nothing in the activity log, and the next save replaced it, so the lifetime total was lost. A cost file that exists but cannot be read, or that does not hold a finite total of 0 or more, is now renamed to total_cost.txt.unreadable, the error goes to the activity log, and the total starts at 0. No save replaces the kept file, so the user can add the old total back. If the cost file is a symbolic link, on Unix the link is moved aside and the file that it names stays as it is. The save stays an in place fs::write, as on main. A write to a temporary file and a rename over the cost file would keep the old total on an exit during the save, but the rename brings faults of its own: a symbolic link needs a special case, a hard linked cost file stops updating its other name, and a folder that allows writing the file but not making new files makes every save fail. The docs of add_cost and save_total_cost say that a save cut short can leave the file empty. PriceEstimator::new no longer reads the file; PriceEstimator::load does, with the logger, and ProcessingServices::new uses it. Tests: an empty, text, NaN, negative or not UTF-8 cost file is logged at start and kept; a save updates a hard linked name of the cost file and works in a folder without write permission.
The comments in on_exit, in the None arm of the command channel and on run_processing_loop said that the command channel closes when the GUI sender is dropped and that the loop stops then. The sender is AppState::command_tx, and the loop holds app_state for its whole run, so the channel cannot close while the loop runs. The loop owns the receiver, cmd_rx, and drops it when it returns: after a startup error (busy OSC port, HTTP client, audio start), on shutdown, and on a panic, all while the window can stay open. Shutdown goes through Shutdown: on_exit requests it, and main.rs requests it again after run_gui returns. The on_exit comment now says that on_exit stops the threads through Shutdown and does not close the command channel, and that the channel closes when processing stops, which can come first. The None arm stays as a guard. Its comment says that app_state.command_tx is a sender (the loop tests hold a second one), which keeps the channel open while the loop runs. The select comment no longer says the command channel helps the loop quit. Comments only, no behaviour change.
When the OSC socket cannot open at startup because the port is in use, the error read "... Address already in use (os error 98) Another OSC app may be listening on this port. To use a different port, change Input Port in OSC Settings." There was no sentence break after the OS error. Also, this error ends the processing loop, so a new port that the user saves is written to config.toml but is not applied: the send fails because the channel is closed. The hint did not say that. The startup hint now starts with a period and tells the user to click Save Settings and restart BabbleBoop. osc_port_busy_hint takes where the bind failed (startup, or a rebind with or without the port open here), and the rebind texts do not change. The startup test now compares the whole error, with the OS error text taken from a second bind of the busy port.
Several statements were false or too strong: The status lock row named the processing loop as a reader; only the GUI calls status(). The atomics row said both sides read them; the callback reads params and test_mode, the GUI reads level and test_mode. The shutdown watch row named the GUI as a reader; the readers are the processing loop and hold_audio_stream. The audio callback rule said it only touches atomics, try_send and try_lock. It now says that is how it shares state, and points to the InputHandler doc comment for the known allocations, the lock inside tokio and the wake system calls. The GUI rule said the GUI thread never blocks; Save Settings writes config.toml on it, and the status read takes a lock. The long await rule now names the unwrapped typing indicator sends and the startup bind, which can wait for DNS, and the final stop_typing after shutdown. The spawn_blocking rule names the inline device open, play and stream drop in finish_test_recording. The shutdown rule names the processing stopped flag, which also stops hold_audio_stream. Docs only, no behaviour change.
A total_cost.txt that cannot be read is renamed so that no save replaces it. The name was always total_cost.txt.unreadable, and fs::rename replaces an existing target on Unix and on Windows. A second start with a bad cost file therefore deleted the file kept on the first start, which can hold the only copy of the old total. The file now goes to the first of total_cost.txt.unreadable, total_cost.txt.unreadable.1, .2 and so on that is not in use, and the activity log names that file. The test loads several bad files in turn and checks that each earlier kept file stays as it was.
added 8 commits
September 29, 2026 16:41
When the OSC address or output port changed during a recording, the processing loop turned typing off at the old destination and sent nothing to the new one. VRChat at the new destination showed no typing indicator for the rest of the utterance. TypingIndicator now keeps the state of its last call. On a destination change the loop still turns typing off at the old destination, and if typing was on and translation is on, it turns typing on at the new destination after the settings are replaced. A new name for the same destination, such as localhost for 127.0.0.1, also counts as a change: typing goes off and on again at the same place. Finding that both names are the same place takes a DNS lookup in the arm, and the brief off does not leave VRChat in a wrong state. The test of a new output port during a recording also changes the port back after the recording ends. Typing goes off at the port it leaves and nothing turns it on at the port of the start. Without the check of the last typing state, the loop would turn typing on at every destination change.
ARCHITECTURE.md said that the InputHandler doc comment lists all the places where the audio callback allocates, locks or wakes a thread, and that the callback never prints. The list did not have the panic path. BabbleBoop sets no panic hook, so when the callback panics, the panic machinery of std runs on the audio thread before catch_unwind in PanicGuard::run returns: the default hook and the unwinding take locks, allocate memory and write the panic message to stderr. That code belongs to std and does more than a short list can show. For example, on the first panic the default hook reads RUST_BACKTRACE, which takes a lock of std on Unix and can allocate. A complete list of it is not practical to keep. The list in the InputHandler doc comment is now complete for the normal path only. The panic path is one item, in place of the PanicGuard item. It says that the default hook and the unwinding take locks, allocate and write to stderr, gives examples verified against Rust 1.97, says that it does not list all that std does, and says that later callbacks do not run process. The PanicGuard doc says that the message still goes to stderr. ARCHITECTURE.md says the same. Comments and docs only, no behaviour change.
The hover text of "Settings not applied" told the user to click Save Settings to apply the saved settings. After the processing loop ends, for example after a startup error, the command receiver is gone. Each click on Save then writes the file again and fails with "channel closed", so the settings are never applied. A restart applies them, since main.rs loads the same config file at start. While processing has stopped, the hover text now says that Save Settings cannot apply the settings and that a restart uses them. While processing runs, it keeps the old text. Two GUI tests save through a send that fails, rest the pointer on the indicator and read the tooltip: one with processing stopped and a closed channel, one with a full channel.
The rebind kept the socket in use when any address that the new name resolves to was the address of that socket. But the typing indicator and the chatbox send to the name, and tokio 1.48 UdpSocket::send_to sends to the first address only. For a name that resolves to ::1 first and 127.0.0.1 second, a change from 127.0.0.1 to that name kept the IPv4 socket, and every OSC message to ::1 failed with "Address family not supported by protocol" until a restart. Now the socket stays in use only when the first address is its address. Otherwise the rebind binds the name like a start does, which tries the addresses in order. The decision is a function of the resolved addresses and the address in use, so a unit test covers the order without a host name that resolves to both families.
The audio callback counts the events that do not fit in the channel and reports them later as EventsDropped. The lost events can hold the StopRecording of the recording that turned typing on, so VRChat kept showing the typing indicator with no recording. Since the typing indicator remembers the last state it sent, a later change of the OSC destination also turned typing on at the new destination. The loop cannot know which events were lost, so on EventsDropped it now turns typing off, also while translation is off, like StopRecording. If the recording goes on, typing stays off until a part of it is processed or the next recording starts.
While translation was off, the loop turned the VRChat typing indicator off at the end of every recording that the noise gate detected and at every EventsDropped, also when BabbleBoop never turned typing on. On main the loop ignored audio events while translation was off; it sent only the off of SetEnabled(false) and the off at shutdown. A user who switches translation off so that another OSC app or the in game keyboard can use the chatbox can see the typing indicator of that app go off during an utterance, if VRChat keeps one typing state for each player (not checked against VRChat). StopRecording while translation is off and EventsDropped now send the typing off only if TypingIndicator::is_typing is true, which is the state of the last call even when its send failed. The two cases that need the off still get it: a recording that started while translation was on and ends after a SetEnabled(false) that did not reach the loop, and lost events that can hold the StopRecording of a recording that turned typing on. EventsDropped uses the same check while translation is on: if typing is off, the lost events cannot have left it on. StopRecording while translation is on is not changed. Two tests relied on the extra off after a StopRecording. They now wait until the loop handled the StopRecording and check that the shutdown sends the first and only message, so they still fail if typing goes on again after a part or at a new output port.
The rules for when the processing loop sends /chatbox/typing false grew one event at a time: StopRecording while translation is off, lost audio events, and a change of the OSC destination each had their own check of the last typing state. A destination change while translation was off still sent an off that BabbleBoop did not cause, which can clear the typing indicator of another app that uses the chatbox. TypingIndicator now remembers the destination where it last sent true. stop_typing sends false only there, and clears the state, so a second off and an off without an on send nothing. stop_typing_elsewhere turns typing off at the remembered destination when the settings send to a new one, and tells the loop to turn it on at the new one (while translation is on, as before). Switching translation off and the end of the loop use force_stop_typing, which sends the off in any case: the remembered state is what BabbleBoop sent, not what VRChat shows. The loop arms for StopRecording and EventsDropped now call stop_typing without their own checks, and osc_destination and is_typing are gone. send_failure is a plain FailureLog, since the calls take &mut self. Changed test expectations: a StopRecording after Pipeline::process turned typing off no longer sends a second off (short recording, utterance, long recording part, new output port, new input port), and a destination change while typing is off sends nothing to the old destination (change back between recordings, change after lost events). The new model test now starts each utterance with StartRecording, since an off without an on sends nothing. The pipeline delivery fixture turns typing on first, as an utterance does.
The TypingIndicator doc said that it always sends to the address in the settings of the caller. Since @5a2ccc4@, stop_typing_elsewhere sends its off to the address of the last on. The EventsDropped and accept_transcription docs said that typing turns off in any case; it turns off only if BabbleBoop turned it on at that address. ARCHITECTURE.md now says that the two forced offs go out whatever the remembered state. The save_total_cost doc now says that a full disk most likely leaves the cost file empty, which the next start detects, and that a partial total is possible but unlikely.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch acts on an architecture review of v0.6.0. It makes the processing loop testable end to end, gives config a single owner, and fixes the bugs the review found. It also adds ARCHITECTURE.md with the rules for code that runs on more than one thread.
Bugs fixed
Structure
run_processing_loopmoved frommain.rsinto the library behind two seams: the audio input channels and aPlaybackOutputtrait.main.rsnow only starts the cpal devices (157 lines, down from 374).OpenAiclient with a typedApiError. The base URL lets tests run against a local server.AppStateholds oneArc<AudioShared>instead of ten separate fields. The GUI reads the recorder status as one snapshot. The audio callback never waits for that lock.process_audioand its ten arguments are replaced by aPipelinestruct;audio_processing.rsis nowpipeline.rs.test_support.rs.src/tests.rsis gone, and items that were public only for tests are private again.AppCommand::Quit,AppState.log_tx, two unused theme colours, and a duplicated helper are removed. The upload duration is computed from the samples. The rate limiter uses the tokio clock.Tests
There are 277 unit tests, up from 207, including end to end tests of the processing loop with fake audio events, a local UDP socket for VRChat, and a local OpenAI server. Every commit passes
cargo fmt --check,cargo clippy --all-targets -- -D warningsandcargo test. CI now also runs clippy on Windows.Not covered