Depersonalize the public repo: drop internal workflow docs, tracker IDs, and session-specific comments - #11
Merged
Merged
Conversation
Pre-announce pass.
Attribution. firmware/voice_pe_dual.yaml is a near-verbatim derivative of
esphome/home-assistant-voice-pe's home-assistant-voice.yaml, including
upstream's "in case you ahve a hidden SSID" typo at line 17, which is the
fingerprint. No ESPHome copyright appeared anywhere in the repo, and
NOTICE.md never disclosed the YAML was copied at all. The ESPHome License
applies MIT terms to non-C++ files, and MIT requires the copyright notice
be preserved. Added a header to the YAML naming upstream and the 2019
ESPHome copyright, plus a NOTICE.md entry. Fixed the inherited typo.
This is the finding most likely to become a public "you copied this
without credit" thread, and it was a ten-minute fix.
Not changed, because the audit that flagged it was wrong: the vendored
component LICENSE reads "Copyright (c) 2024-2025" with no holder, which
looked like a stripped name. Upstream fjfricke/ha-openai-realtime's own
LICENSE reads exactly the same. Our copy is faithful and NOTICE.md's
claim that the copyright is preserved is accurate.
False endorsement. .esphome_component.yml declared "author: OpenAI
Realtime Voice Agent" and CODEOWNERS pointed at @openai-realtime-voice-
agent, a handle that doesn't exist. Both read as OpenAI authorship of a
project that merely calls their API. Now jayzuccarelli.
Internal references. .gitignore carried "# internal ops runbook (not for
public repo)", advertising a withheld document (never committed, verified).
JAY-84 tracker IDs in broker comments now name the behavior. CLAUDE.md was
11 lines of pure Linear workflow naming the team, the project, and me
personally, with no repo-technical content worth keeping: untracked and
gitignored so it stays local.
Softened a comment that called out a bug in fjfricke's project by name,
since NOTICE.md credits it as the original proof-of-concept.
Em-dashes: 104 removed. Repaired three README table cells where the
em-dash was the cell VALUE ("no default") and became a stray comma, and
the harness latency column, which printed a bare "," for scenarios with
no first-audio measurement. Now "n/a".
Verified against a live isolated broker on :8766, which leaves the live
puck on :8765 alone: 10/10 scenarios passed, first-audio p50 155ms,
p95 201ms. challenge_follow_up passed, which exercises the follow-up-bias
prompt string this commit edits.
…ong build The attribution this branch added was half of what MIT asks for. MIT requires the copyright notice AND the permission notice; only the copyright line was here, and no ESPHome license text existed anywhere in the tree. The repo already showed the right pattern one directory over, where the fjfricke component ships a verbatim LICENSE beside the code it covers. ESPHome now gets the same treatment in firmware/LICENSE.esphome, fetched from upstream, with the YAML header and NOTICE.md pointing at it. The header also described firmware that isn't this one. It claimed a dual-mode path streaming to the broker 'alongside the stock Assist pipeline'. The file sets use_wake_word: false and routes its single wake word straight to the broker; its own models block says so, and firmware/README.md agrees v1 is single-wake, with dual mode archived at tag dual-2wake-v0. Reasserting the archived behavior in the one file whose header now carries licensing weight was the wrong place to be wrong. Four comments in server.py had been left as dangling trailing colons where the sweep hit paired em-dashes; lines 661 and 662 were two halves of one sentence and no longer parsed. Restored from the pre-sweep text. Two of the sweep's replacements landed in live model instructions in agent.py, where a comma splice weakened an imperative the em-dash had been holding open. Both are now sentence boundaries. The sweep had also missed nine files, including .env.example, which carries the same sentence as README and config.py and so read differently from both. Repo-wide em-dash count is now zero. Also folded SUNDAY_RUNBOOK.md in with the other local-notes ignores; the filename alone still advertised a withheld document. ruff clean on broker/realtime_broker, all changed Python compiles, the firmware YAML still parses.
The three remaining comments described a specific listening session in a specific room, with wall-clock timestamps. The behavior they document is general: background TV speech false-accepts, and ghost turns after a mid-utterance disconnect. Describe the class, not the session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe pull request removes local runbook guidance, updates repository and firmware documentation, adds the ESPHome license text, and changes firmware component ownership metadata. Runtime behavior and build commands remain unchanged. ChangesDocumentation and attribution cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.gitignore (1)
18-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScope the
CLAUDE.mdignore rule to the intended location.
CLAUDE.mdwithout a slash matches files with that name in every directory. If this change only removes the repository-root runbook, use/CLAUDE.md; otherwise a nested project document can be silently excluded from version control.Proposed fix for a repository-root-only rule
-CLAUDE.md +/CLAUDE.md🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitignore around lines 18 - 19, Scope the CLAUDE.md entry in .gitignore to the repository root by adding the leading slash, so nested CLAUDE.md files remain trackable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@broker/realtime_broker/server.py`:
- Around line 214-216: Remove session-specific experiment evidence from broker
comments: in broker/realtime_broker/server.py lines 214-216, update the
reset_vad docstring to use behavior-based wording without the dated soak result
or quoted response; in lines 722-726, remove the captured “Bye.” and “I'm here
when you're ready” outputs from the disconnect comment while preserving its
general behavioral description.
In `@broker/tools/harness.py`:
- Line 238: Replace the repository-specific “fjfricke `#9` bug” comment beside the
fresh connection in session(url) with a behavior-based reconnect-regression
description. In broker/tools/test_rotation.py lines 3-5, remove the
“fjfricke/ha-openai-realtime#8” reference while retaining the session-expiry
behavior description.
In `@firmware/README.md`:
- Line 39: Update the status description in the firmware README to replace the
comma-spliced room-noise limitation, broker mitigation, and device-side
beamforming roadmap details with separate sentences, preserving all three pieces
of information and their meaning.
---
Nitpick comments:
In @.gitignore:
- Around line 18-19: Scope the CLAUDE.md entry in .gitignore to the repository
root by adding the leading slash, so nested CLAUDE.md files remain trackable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ca1c547-9813-42cd-8e10-76f53084d536
📒 Files selected for processing (27)
.gitignoreCLAUDE.mdMakefileNOTICE.mdREADME.mdbroker/.env.examplebroker/Makefilebroker/realtime_broker/agent.pybroker/realtime_broker/config.pybroker/realtime_broker/server.pybroker/requirements.txtbroker/tools/harness.pybroker/tools/hook_check.pybroker/tools/m2_analyze.pybroker/tools/m2_capture.pybroker/tools/m2_play_ref.pybroker/tools/test_bargein.pybroker/tools/test_client.pybroker/tools/test_rotation.pyfirmware/CRASH_ANALYSIS.mdfirmware/LICENSE.esphomefirmware/README.mdfirmware/components/voice_assistant_websocket/.esphome_component.ymlfirmware/components/voice_assistant_websocket/__init__.pyfirmware/flash.shfirmware/secrets.yaml.examplefirmware/voice_pe_dual.yaml
💤 Files with no reviewable changes (1)
- CLAUDE.md
Comment on lines
214
to
216
| auto-creates a response. The model greets the ghost turn ("I'm here | ||
| when you're ready"; soak 2026-07-02, 5/5 then 2/6 with a delayed | ||
| clear alone). Disabling turn detection makes the server drop the |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove session-specific diagnostic evidence from public broker comments.
Both locations retain run-specific results or captured assistant utterances. Use behavior-based wording and keep experiment evidence in internal test records.
broker/realtime_broker/server.py#L214-L216: Remove the dated soak result and quoted response from thereset_vaddocstring.broker/realtime_broker/server.py#L722-L726: Remove the captured"Bye."and"I'm here when you're ready"output from the disconnect comment.
📍 Affects 1 file
broker/realtime_broker/server.py#L214-L216(this comment)broker/realtime_broker/server.py#L722-L726
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@broker/realtime_broker/server.py` around lines 214 - 216, Remove
session-specific experiment evidence from broker comments: in
broker/realtime_broker/server.py lines 214-216, update the reset_vad docstring
to use behavior-based wording without the dated soak result or quoted response;
in lines 722-726, remove the captured “Bye.” and “I'm here when you're ready”
outputs from the disconnect comment while preserving its general behavioral
description.
| if not r1.got_audio: | ||
| return False, "first session got no audio", r1.first_audio_ms | ||
| async with session(url) as c: # fresh connection — the fjfricke #9 bug | ||
| async with session(url) as c: # fresh connection, the fjfricke #9 bug |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove the remaining author and issue references.
These changed comments still publish repository-specific issue references. Replace them with behavior-based descriptions.
broker/tools/harness.py#L238-L238: replacefjfrickechore: drop internal codename from check.py #9bugwith a reconnect-regression description.broker/tools/test_rotation.py#L3-L5: removefjfricke/ha-openai-realtime#8and retain the session-expiry behavior description.
This conflicts with the PR objective to remove private tracker IDs and session-specific comments.
📍 Affects 2 files
broker/tools/harness.py#L238-L238(this comment)broker/tools/test_rotation.py#L3-L5
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@broker/tools/harness.py` at line 238, Replace the repository-specific
“fjfricke `#9` bug” comment beside the fresh connection in session(url) with a
behavior-based reconnect-regression description. In
broker/tools/test_rotation.py lines 3-5, remove the
“fjfricke/ha-openai-realtime#8” reference while retaining the session-expiry
behavior description.
| ## Status | ||
|
|
||
| Compiles clean (`firmware.factory.bin`) and runs on-device. Known limitation: the far-field mic captures the whole room, so in a loud room the assistant can pick up a TV or another speaker — the broker mitigates this with the `wait_for_user` gate, and device-side beamforming (tapping the XMOS focused channel) is on the roadmap. | ||
| Compiles clean (`firmware.factory.bin`) and runs on-device. Known limitation: the far-field mic captures the whole room, so in a loud room the assistant can pick up a TV or another speaker, the broker mitigates this with the `wait_for_user` gate, and device-side beamforming (tapping the XMOS focused channel) is on the roadmap. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Split the comma-spliced status description.
Line 39 joins the room-noise limitation, broker mitigation, and roadmap item with commas. Use separate sentences so each behavior remains clear.
Proposed wording
-Compiles clean (`firmware.factory.bin`) and runs on-device. Known limitation: the far-field mic captures the whole room, so in a loud room the assistant can pick up a TV or another speaker, the broker mitigates this with the `wait_for_user` gate, and device-side beamforming (tapping the XMOS focused channel) is on the roadmap.
+Compiles clean (`firmware.factory.bin`) and runs on-device. Known limitation: the far-field mic captures the whole room. In a loud room, the assistant can pick up a TV or another speaker. The broker mitigates this with the `wait_for_user` gate. Device-side beamforming (tapping the XMOS focused channel) is on the roadmap.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Compiles clean (`firmware.factory.bin`) and runs on-device. Known limitation: the far-field mic captures the whole room, so in a loud room the assistant can pick up a TV or another speaker, the broker mitigates this with the `wait_for_user` gate, and device-side beamforming (tapping the XMOS focused channel) is on the roadmap. | |
| Compiles clean (`firmware.factory.bin`) and runs on-device. Known limitation: the far-field mic captures the whole room. In a loud room, the assistant can pick up a TV or another speaker. The broker mitigates this with the `wait_for_user` gate. Device-side beamforming (tapping the XMOS focused channel) is on the roadmap. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@firmware/README.md` at line 39, Update the status description in the firmware
README to replace the comma-spliced room-noise limitation, broker mitigation,
and device-side beamforming roadmap details with separate sentences, preserving
all three pieces of information and their meaning.
jayzuccarelli
changed the title
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
1 participant
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.
Supersedes #6 (same two commits, rebased onto current main, plus one more).
Why
The repo is public but read as a personal work log: it carried an agent-instructions file written in the third person about the owner, private tracker IDs, and code comments describing one specific listening session at home.
No secrets were exposed. Checked the full tree and history for credentials, API keys, LAN IPs, Tailscale hostnames, and personal paths: none present. Every key is read from the environment, and
secrets.yamlis gitignored.Changes
CLAUDE.mddeleted from the tree and gitignored. It was 11 lines of pure issue-tracker workflow naming the team, the project, and the owner personally, with zero repo-technical content. The local copy survives for agent use.firmware/voice_pe_dual.yamlderives fromesphome/home-assistant-voice-pe, so the ESPHome license text is vendored and NOTICE.md scoped accordingly.Verification
LICENSE(intentional) and two codename strings inbroker/tools/check.py, which PR chore: drop internal codename from check.py #9 removes. Depersonalize the public repo: drop internal workflow docs, tracker IDs, and session-specific comments #11 and chore: drop internal codename from check.py #9 together finish the job.SUNDAY_RUNBOOK.md(the old home-LAN-IP file) confirmed absent from all oforigin/mainhistory, verified locally rather than via the API.py_compilepasses. The tworufffindings on this branch are pre-existing onmainand untouched here.🤖 Generated with Claude Code
Summary by CodeRabbit