Attribute the ESPHome firmware YAML; drop internal references - #6
Closed
jayzuccarelli wants to merge 2 commits into
Closed
Attribute the ESPHome firmware YAML; drop internal references#6jayzuccarelli wants to merge 2 commits into
jayzuccarelli wants to merge 2 commits into
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.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughDocumentation and metadata were revised across the repository, broker, tooling, and firmware areas. Changes cover wording, punctuation, attribution, ignore rules, displayed messages, ownership metadata, and explanatory comments; runtime logic and configuration behavior remain unchanged. ChangesDocumentation and metadata refresh
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 53 minutes. |
…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.
Owner
Author
|
Superseded by #11: same commits rebased onto current main, plus the TV-test comment cleanup. |
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.
Pre-announce verification pass.
1. Unattributed copy of upstream ESPHome firmware (the real one)
firmware/voice_pe_dual.yamlis a near-verbatim derivative ofesphome/home-assistant-voice-pe'shome-assistant-voice.yaml. The fingerprint is upstream's own typo, present at line 17 in both:No ESPHome copyright appeared anywhere in the repo, and
NOTICE.mdnever mentioned the YAML at all. ESPHome's license applies MIT terms to non-C++ files (the GPLv3 half covers.c/.cpp/.h/.hpp/.tcc/.ino), and MIT requires the copyright notice be preserved.Added a header naming upstream and
Copyright (c) 2019 ESPHome, plus aNOTICE.mdentry. Fixed the inherited typo while I was there.This is the single finding most likely to become a public "you copied this without credit" thread, and it cost ten minutes.
Good news on the GPL question: there is no contamination. The GPLv3
voice_kitC++ is pulled at compile time viaexternal_components, never vendored, and*.binis gitignored. MIT-at-root is correct. The problem was attribution, not compatibility.2. A flagged issue that turned out to be a non-issue
The vendored component's
LICENSEreadsCopyright (c) 2024-2025with no holder name, which looks like the name was stripped. It wasn't: upstreamfjfricke/ha-openai-realtime's own LICENSE reads exactly the same. Our copy is faithful andNOTICE.md's claim that the copyright is preserved is accurate. Left alone.3. Implied OpenAI endorsement
.esphome_component.ymldeclaredauthor: OpenAI Realtime Voice Agentand__init__.pyhadCODEOWNERS = ["@openai-realtime-voice-agent"], a handle that doesn't exist. Both read as OpenAI having authored a project that merely calls their API. Nowjayzuccarelli.4. Internal references
.gitignorecarried a comment advertising a withheld internal document. Verified it was never committed, so nothing leaked, but the comment invites the question. Removed.CLAUDE.mdwas 11 lines of internal issue-tracker workflow with zero repo-technical content. Untracked and gitignored so the local copy survives.5. Em-dashes, and two they broke
104 removed. Two needed repair afterwards, both caught by auditing rather than assuming:
| `OPENAI_API_KEY` |, | required |. Nownone.,for scenarios with no first-audio measurement. Nown/a.Verification: 10/10 against a live broker
Ran the real harness against an isolated broker on
:8766, which by design leaves the live puck on:8765untouched (confirmed: it stayed up throughout).This matters beyond a green check: the em-dash sweep touched two model-facing prompt strings in the tuned follow-up-bias instructions.
challenge_follow_upis the scenario that exercises exactly that behavior, and it passed. The README's "10 scenarios" and reliability claims are independently confirmed.Clean
No secrets in the tree or across all 57 commits. No employer references anywhere, so no conflict surface despite this being a voice-assistant project. No audio or transcript fixture was ever committed.
Summary by CodeRabbit
Documentation
Chores