docs: two pages that state the opposite of what the code does - #360
Merged
Conversation
Both found by the pre-tag sweep, both verified against the source, and both are the kind that survive because they are stated confidently. ## PLATFORMS.md said a scope is not requested. It is. "`moderation:ban` is deliberately not requested: nothing in polyemesis bans or times out a viewer." internal/oauth/kick.go:102 requests it, and internal/automod's action matrix carries ActionTimeout and ActionBan. The doc was wrong twice over: it omitted the scope from the list AND explicitly denied it. This one is not drift, it is a decision the page never caught up with. The code comment at kick.go:88-94 records the reversal in full -- the scope was omitted on the grounds that asking a restreamer's audience for the power to ban read as overreach, and that was reversed when moderation shipped. It deliberately keeps the old argument rather than deleting it, "because it is the argument to re-read if the decision is ever revisited". The page kept only the old half. A false statement about what a consent screen will ask for is the worst class of error on this page: an operator reads it, connects an account, and is shown a permission the documentation told them would not be requested. Also carried across the reason the list is settled in one go, which is the practical fact an operator needs: adding a scope later does not upgrade an existing token, it forces every operator to reconnect. ## HARDWARE.md had the default encoder exactly inverted "`libx264` stays the default even on a machine with a working GPU. ... Hardware is an opt-in you make deliberately." Tools.DefaultVideoEncoder (internal/ffmpeg/detect.go:415) walks encoderPreference -- videotoolbox, nvenc, qsv, vaapi, amf, x264 -- and returns the FIRST that passed the probe. Hardware wins, and the function's own comment says why: "a machine with a usable GPU that silently software-encodes cannot serve the feature it was bought for". RENDITIONS.md:295 has always had this right, so the two pages contradicted each other and a reader had no way to tell which to believe. Rewritten to match the code, including the every-probe-failed fallback, and keeping the true half of the old text -- libx264 IS identical everywhere and is a legitimate choice -- as the override it actually is rather than the default it is not. Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
|
There was a problem hiding this comment.
Pull request overview
Updates two documentation pages to match the current behavior of the codebase: (1) the Kick OAuth scopes actually requested (including moderation:ban) and the operational impact of adding scopes later, and (2) the actual default video encoder selection behavior (hardware-first when probes succeed, with libx264 as fallback).
Changes:
- Corrected
PLATFORMS.mdto listmoderation:banas requested on Kick and documented the historical rationale + reversal, plus the reconnect requirement when scopes change. - Corrected
HARDWARE.mdto reflect the code’s hardware-first default encoder selection order and the “all probes failed” fallback behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| docs/PLATFORMS.md | Fixes incorrect claim about moderation:ban not being requested; documents scope-change reconnect implications. |
| docs/HARDWARE.md | Fixes inverted default encoder behavior; documents hardware preference order and fallback to libx264. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.



Batch B, docs half. The encoder-profile defects (
hevc_vaapi, NVENC capped VBR) are being fixed separately.Both of these are verified against source, and both survived because they are stated confidently.
PLATFORMS.mdsaid a scope is not requested. It is.internal/oauth/kick.go:102requests it.internal/automod/matrix.go:54carriesActionTimeoutandActionBan. The page was wrong twice — it omitted the scope from its list and explicitly denied it.This isn't drift, it's a decision the page never caught up with.
kick.go:88-94records the reversal in full:The page kept only the old half — the half the code explicitly labels as superseded.
A false statement about what a consent screen will ask for is the worst class of error on this page: an operator reads it, connects an account, and is shown a permission the documentation promised would not be requested.
Also carried across the fact an operator actually needs: adding a scope later does not upgrade an existing token, it forces everyone to disconnect and reconnect. That's why the list is settled in one go, and it explains the "reconnect once" notice they may already have hit.
HARDWARE.mdhad the default encoder exactly invertedTools.DefaultVideoEncoder(internal/ffmpeg/detect.go:415) walksencoderPreference— videotoolbox, nvenc, qsv, vaapi, amf, x264 — and returns the first that passed the probe. Hardware wins, and the function's own comment says why:RENDITIONS.md:295has always had this right, so the two pages contradicted each other and a reader had no way to tell which to believe.Rewritten to match the code, including the every-probe-failed fallback (still
libx264, because an empty-c:vis neither usable nor legible). Kept the true half of the old text —libx264is identical everywhere and is a legitimate choice — reframed as the override it is rather than the default it isn't.Verification
grep -c '"moderation:ban"' internal/oauth/kick.go→ 1grep -rn "opt-in you make deliberately" docs/→ 0 remainingHARDWARE.mdandRENDITIONS.mdnow agreeDocs-only, so this also exercises the #357 path-filter fix again.
https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX