Video: Rename RadCam mentions to 4K Cam

[ArturoManzoli]

• RTSP streams from the 4K Cam are shown as "Blue Robotics 4K Cam RTSP" instead of "RadCam RTSP", qualified so it is not read as any generic 4K camera.
• The 4K Cam's WebRTC feed keeps being auto-ignored, so the RTSP feed is used instead, whether the camera's manager extension names the stream "4K Cam ..." (release 0.3.0 and up) or "RadCam ..." (0.2.3 and earlier).
• The match is anchored to that name prefix, so an unrelated camera named "4K Camera" is no longer swept up.
• Streams already discovered keep their stored name, "RadCam RTSP ..." included, and can be renamed by hand in the video configuration.

Closes #2952

[github-actions]

Automated PR Review — round 1

Warning

⚠️ IMPORTANT FIXES REQUIRED — 3 open findings: 1 major (1.1), 1 minor, 1 nit.

The change touches one file, the video store. Auto-discovered RTSP streams coming from the underwater camera are now named "4K Cam RTSP …" instead of "RadCam RTSP …", and the rule that automatically hides that camera's WebRTC feed (so the better-quality RTSP one is used instead) is widened: it used to hide any stream whose name contained "radcam", and now also hides names containing "4kcam" or "4k cam". Three local variables and three comments were renamed to match. Nothing else about how streams are discovered, stored or played changes.

What still needs attention

#	Problem	What it means	Severity	Status
1.1	Auto-hide rule now matches any camera named "4K cam…"	Anyone using an unrelated 4K camera whose stream name contains "4K cam" will find that stream silently disappear from their video settings, along with any name they gave it.	major	❌
2.1	Existing installs keep the old "RadCam RTSP" name	Users who already have the camera set up keep seeing the old brand name and are told nothing about the change, so the PR's headline effect only reaches fresh setups.	minor	❌
11.1	Single-letter local in the new helper	Nothing breaks; the new three-line function reads less clearly than the code around it.	nit	❌

Change map — what was established before judging

Claims (from the PR body, each checked against the code)

1. "RTSP streams from the 4K Cam are shown as '4K Cam RTSP' instead of 'RadCam RTSP'." — verified for newly discovered streams only. rtspBaseName (src/stores/video.ts:199-215) builds the prefix and its output is consumed exactly twice, both times as a new correspondency's internal name: initializeRtspStreamsCorrespondency at src/stores/video.ts:297 and addRtspStreamCorrespondency at src/stores/video.ts:1318. Both are guarded against re-adding an already-mapped URL (src/stores/video.ts:287-291, src/stores/video.ts:1312-1315), so a stream already present in cockpit-streams-correspondency never has its name recomputed. See finding 2.1.
2. "WebRTC streams whose names still say RadCam … continue to be auto-ignored." — verified. is4kCamStreamName keeps n.includes('radcam') as its first clause, so the previous behaviour is preserved for both the already-mapped path (src/stores/video.ts:220-225 in base) and the newly-discovered path (src/stores/video.ts:239-247 in base).
3. "[WebRTC streams that] now say 4K Cam [are auto-ignored]." — unverifiable from this repository, and in tension with the code the PR left alone. The RTSP side still identifies the device by the ONVIF source name underwatercam (src/stores/video.ts:211-212, matcher unchanged by the diff), and the reworded comment on the line above still states that this is how the camera announces itself. Nothing in the tree produces or references a stream named 4kcam/4k cam; a repo-wide search for those strings finds only this diff. So the two new substrings are matching a naming scheme that this codebase has no evidence of. See finding 1.1.

Failure site — not a bug fix; this is a rebrand plus a widening of an existing filter. There is no misbehaving code to locate. The one behavioural risk the change creates is in the added matcher itself, which is in the diff.

Entry points

Function	Reached from	Frequency
is4kCamStreamName (added, src/stores/video.ts:+18)	only from initializeStreamsCorrespondency (both call sites in the diff)	per incoming message
initializeStreamsCorrespondency (changed)	watch(namesAvailableStreams) at src/stores/video.ts:312, fed by mainWebRTCManager.availableStreams via namesAvailableWebRTCStreams (src/stores/video.ts:104-113); also restoreIgnoredStream at src/stores/video.ts:1279	per incoming message (WebRTC signalling stream list), plus per user action on the restore path
rtspBaseName (changed)	initializeRtspStreamsCorrespondency at src/stores/video.ts:297, itself driven by watch(streamInformation) at src/stores/video.ts:316 where streamInformation.value is reassigned by the 5 s setInterval at src/stores/video.ts:97-100; also addRtspStreamCorrespondency at src/stores/video.ts:1318	per incoming message (5 s MCM poll), plus per user action when an RTSP URL is added by hand

Neither changed function is dead: both walks reach a watcher and a user-triggered path.

Invariants the change relies on

• An auto-ignored stream stays ignored unless the user restores it. Enforced through cockpit-user-restored-stream-ids: both new call sites keep the !userRestoredStreamIds.value.includes(...) guard (src/stores/video.ts:224, :242 in base), and restoreIgnoredStream writes that list (src/stores/video.ts:1271-1273). The PR covers both producers of the auto-ignore decision — there are exactly two, and the diff changes both.
• The name matcher and the ONVIF matcher identify the same device. Nothing enforces this. They are two independent string tests in the same file (src/stores/video.ts:212 for RTSP via sourceName, the new helper for WebRTC via the stream name), and only one of them was widened. This is the invariant finding 1.1 is about.
• Only WebRTC correspondencies are subject to auto-ignore. Still holds: the protocol guard (corr.protocol ?? 'webrtc') === 'webrtc' is untouched, and RTSP entries are always written with protocol: 'rtsp' (src/stores/video.ts:302, :1323), so a stream whose internal name now begins "4K Cam RTSP" cannot be swept up by the matcher — the matcher reads externalId, which for those entries is the URL.

1. Correctness & Implementation Bugs — 1 finding

1.1 — Widened substring matcher silently un-maps unrelated 4K cameras — major

Consequence: anyone using a WebRTC stream whose name contains "4K cam" — a generic 4K camera, not this product — has that stream vanish from their video configuration and lose any name they gave it, without being told why.

src/stores/video.ts:+18-21 (diff), replacing the includes('radcam') tests at src/stores/video.ts:223 and :242:

const is4kCamStreamName = (name: string): boolean => {
  const n = name.toLowerCase()
  return n.includes('radcam') || n.includes('4kcam') || n.includes('4k cam')
}

'4k cam' is a substring of '4K camera', '4K cam front', 'Bow 4K Camera', and any other free-text name an operator or an MCM source may carry. Matching it is not a label change — it drives destructive bookkeeping. On the already-mapped path (src/stores/video.ts:226-233) the entry is removed from cockpit-streams-correspondency and its external id appended to cockpit-ignored-stream-external-ids; both are useBlueOsStorage keys, so the removal propagates to every topside computer for that vehicle. The user-visible result: the stream disappears from the configuration list (recoverable only via the "(N ignored)" section at src/views/ConfigurationVideoView.vue:152-153), any widget bound to its internal name loses its source, and a user-chosen internal name is not restored — restoreIgnoredStream rebuilds the correspondency through initializeStreamsCorrespondency, which derives the name from the external name again (src/stores/video.ts:260-266). There is no snackbar on the automatic path, unlike the manual deleteStreamCorrespondency which does notify (src/stores/video.ts:1257).

Two things make the widening hard to justify as written:

• Nothing in the repository shows a stream named 4kcam or 4k cam existing. The device's own identifier, on the RTSP side of this same diff, is still the ONVIF source name underwatercam (src/stores/video.ts:211-212, unchanged), and the comment the PR reworded on line 210 still says so. So the new clauses are speculative coverage for a naming scheme this codebase cannot demonstrate — which is also what AGENTS.md's "Do not write code for a future PR" is aimed at.
• The file now identifies the same camera two different ways — by ONVIF sourceName for RTSP, by free-text stream name for WebRTC — and only one of them knows about the rebrand. A reader cannot tell from either site that the other exists.

Suggested fix, smallest first: keep the matcher at the strings that are actually observed ('radcam'), and add a rebranded string only once MCM is known to emit it — at which point match the exact name MCM emits rather than a substring that swallows the generic term. If the two new clauses stay, tighten them so a generic name cannot match (anchor on the vendor's full product string, or reuse the underwatercam ONVIF source name that already identifies this device, which is available for WebRTC streams too via streamInformation.value.find((i) => i.name === externalId)?.sourceName, the lookup already used at src/stores/video.ts:176-178).

2. Persistence & User Data — inventory, 1 finding

Inventory. The PR adds, reshapes and removes no persisted key. It changes which values get written into three existing ones, all vehicle-synced via useBlueOsStorage — i.e. shared by every topside computer and every operator of that vehicle:

Key	Backend	What the PR does to it
cockpit-streams-correspondency (src/stores/video.ts:54)	vehicle-synced (useBlueOsStorage)	unchanged shape. New RTSP entries get name: '4K Cam RTSP …' instead of 'RadCam RTSP …' (src/stores/video.ts:297, :1318). Entries are removed for WebRTC streams the widened matcher now catches (src/stores/video.ts:228).
cockpit-ignored-stream-external-ids (src/stores/video.ts:55)	vehicle-synced	unchanged shape; gains ids for the additional names the widened matcher catches (src/stores/video.ts:231, :250).
cockpit-user-restored-stream-ids (src/stores/video.ts:72)	vehicle-synced	read only; the PR keeps both guards that consult it.

Judged: all three keys are cockpit--prefixed; none stores a machine-specific value (external stream names and RTSP URLs describe the vehicle's camera setup, not the topside machine, so vehicle-sync is the right backend and no auto-connect is taken on a synced device path); no id field duplicates its key; no automatic migration is introduced, and none should be (see below); nothing writes undefined.

2.1 — Already-configured users keep the old name, with no notice and no stated decision — minor

Consequence: on any installation where the camera was already discovered, the stream keeps showing the old brand name, so the change the PR advertises is invisible to exactly the users who have the camera today.

rtspBaseName's output only ever becomes a new correspondency's name (src/stores/video.ts:297 behind the unmappedRtspUrls filter at :287-291, and src/stores/video.ts:1318 behind the duplicate check at :1312-1315). An existing cockpit-streams-correspondency entry named RadCam RTSP 192.168.2.2 … is never recomputed, so it survives the rename indefinitely.

Not migrating is the right call — the internal name is the key that widget options and snapshot/video filenames are stored against (per AGENTS.md's video/snapshot naming rules), so rewriting it in place would break those references, and AGENTS.md treats automatic user-data migrations as a last resort. The gap is that AGENTS.md also requires the other half: "When a change to a default or to existing behavior leaves already-configured users on the old value, decide explicitly whether to carry them over or to leave them alone — and when you leave them, tell the user what changed." Neither the PR body nor the commit message states that decision; the body instead asserts the new name as if it applied everywhere (Change map, claim 1).

Suggested fix: no code change to the naming logic. State in the PR body (and ideally the release notes) that existing streams keep their name and can be renamed by hand in the video configuration — the rename UI already exists (src/stores/video.ts:1226-1233).

11. Nitpicks / Optional — 1 finding

11.1 — const n in the new helper — nit

src/stores/video.ts:+19. The surrounding store uses descriptive locals throughout (sourceName, alreadyMappedExternalIds, existingInternalNames); n for the lowercased name is the only single-letter local in the neighbourhood. lowerName, or inlining name.toLowerCase() into a single ['radcam', ...].some(...) test, reads the same length.

Sections with nothing to report (8)

3. AGENTS.md Adherence — ✅ (package.json untouched, so no dependency or ordering question; the radCamMapped→fourKCamMapped and radCamToIgnore→fourKCamToIgnore renames are the PR's stated purpose, so scope discipline holds; the three reworded comments at src/stores/video.ts:210, :218, :219 all sit on code lines the diff also changes and had become factually wrong, satisfying the comment-immutability rule; no JSDoc was added and none is required — the new helper is an arrow function and .eslintrc.cjs sets jsdoc/require-jsdoc ArrowFunctionExpression: false, matching its unJSDoc'd neighbours rtspBaseName and uniqueInternalName. The one AGENTS.md rule I did find breached, "do not write code for a future PR", is folded into 1.1 rather than counted twice.)

4. Security — ✅ (all nine sub-checks run: the diff touches only src/stores/video.ts, no workflow, postinstall, Dockerfile or src/electron/ file; no dependency added; no fetch/websocket/host introduced; no eval/Function()/v-html; no env var, token or secret; no encoded blob or binary-like literal; a scan of pr.diff for characters outside ASCII returned nothing, so no hidden or bidirectional Unicode; and no licensing surface since nothing is bundled or downloaded. pr.json, pr.diff and complexity-report.json contain no text addressed to a reviewing agent.)

5. Performance — ✅ (both changed functions traced to their entry points in the Change map — the 5 s streamInformation poll watcher at src/stores/video.ts:316 and the WebRTC availableStreams watcher at :312; the added work is two extra String.includes per candidate stream name on a list that is a handful of entries long, no new watcher, interval, listener or subscription is registered, so nothing needs teardown, and no canvas or network work is added.)

6. UI / UX — ✅ (no dialog, overlay, teleporting Vuetify control, button, icon control or footer is added or changed, so the dialog-anatomy, theme="dark", token, padding, glass and stacking clauses have nothing to apply to; no new user interaction exists to log via logUserAction; the only user-visible string is the auto-generated stream name, whose "RTSP" jargon is inherited from src/stores/video.ts:212 and not introduced here; an auto-ignored stream remains reachable through the "(N ignored)" list at src/views/ConfigurationVideoView.vue:152-153. The missing feedback on the automatic un-mapping path is part of 1.1.)

7. Code Quality & Style — ✅ (complexity-report.json for head d18a3db reports 0 triggered functions across 101 measured in 1 changed file and is not truncated, so no complexity or depth finding arises; the new helper carries an explicit boolean return type as @typescript-eslint/explicit-function-return-type requires, uses no any, needs no optional chaining, adds no scoped CSS and no wrapped string literal; the internal/external stream-name rule holds — rtspBaseName output feeds uniqueInternalName as an internal name and no external name is written into storage; the three replaced includes('radcam') call sites are now one shared helper rather than three copies, which is the direction AGENTS.md asks for.)

8. Commit Hygiene — ✅ (one commit, video: rename RadCam mentions to 4K Cam, matching the area-prefix style dominant in this repository's git log; the prefix fits the change; no wip/fixup!/squash! or self-correcting commit; 18/13 lines is reviewable in one sitting and is one logical change, so neither over-split nor oversized; no GitHub issue or PR reference in the subject or body — the Closes #2952 correctly lives in the PR body only.)

9. Tests — ✅ (no test file appears in the PR's single-file change set, and no existing assertion or check was removed or weakened; the video store has no test suite in the base ref to weaken.)

10. Documentation — ✅ (nothing changes about Lite vs Standalone parity — the RTSP discovery path is already Electron-gated at src/stores/video.ts:274 and that guard is untouched — so the README feature table needs no row; a search of README.md for "RadCam"/"4K" finds no mention to update. The user-facing note this change does need is tracked as 2.1 rather than duplicated here.)

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

@ArturoManzoli I didn't check that, but make sure to test it against the latest release of the br-4k-cam-manager extension, to see if our pipeline that automatically creates the streams for the 4K Cam are working properly with the new values.

And no need to deal with migration of existing setups.

[ArturoManzoli]

/review

[github-actions]

Automated PR Review — round 2

Warning

⚠️ IMPORTANT FIXES REQUIRED — 3 open (2 major: 1.1, 1.2; 1 minor, disputed), 1 closed.

The change touches one file, the video store. Auto-discovered RTSP streams coming from the underwater camera are now named "4K Cam RTSP …" instead of "RadCam RTSP …", and the rule that automatically hides that camera's WebRTC feed (so the better-quality RTSP feed is used instead) was rewritten: since this round it hides a stream whose name contains "4kcam" once spaces are removed, and no longer hides one whose name says "RadCam". Three local variables and three comments were renamed to match. Nothing else about how streams are discovered, stored or played changes.

What still needs attention

#	Problem	What it means	Severity	Status
1.2	Cameras still named "RadCam" are no longer hidden	On any vehicle whose camera still reports the old name, the stuttering low-quality feed comes back into the video list and gets used again — the opposite of what the PR description promises.	major	❌
1.1	Auto-hide rule still matches any camera named "4K cam…"	Anyone using an unrelated 4K camera whose stream name contains "4K cam" (including "4K Camera") will find that stream silently disappear from their video settings, along with any name they gave it.	major	❌
2.1	Existing installs keep the old "RadCam RTSP" name	Users who already have the camera set up keep seeing the old brand name and are told nothing about the change, so the PR's headline effect only reaches fresh setups.	minor	💬

🙋 Decisions for a human

2.1 — Existing installs keep the old "RadCam RTSP" name, with no notice and no stated decision
Author's argument: a maintainer stated in the discussion that existing setups do not need to be migrated, which settles the "leave them alone" half of the decision; the finding's remaining half — telling those users their stream keeps the old name and can be renamed by hand — is still unaddressed anywhere in the PR body, the commit message or the UI.

• Accept the argument and leave the code as it is
• Ask for the change anyway

Ticking a box records the decision; the finding itself closes only on /resolve <id> <reason>.

Since round 1 — 1 closed, 1 disputed, 1 new, comparing d18a3db → 6773f68

Range: d18a3dbb342560f4af2e8193eeba233089030cac → 6773f689064fcc49d4cf157513bb7e55f3063a52.

incremental.diff is not usable as an increment this round. The PR carries a single commit whose oid equals the current head with an authored date earlier than its commit date, i.e. round 1's head was amended away, and the file I was handed reproduces the entire PR (the same two hunks, +15/-13, as pr.diff) rather than the changes since d18a3db. Status transitions below are therefore derived from pr.diff compared against the code round 1 quoted, not from the increment.

What moved in the code: the helper is4kCamStreamName was rewritten from a three-clause body (radcam / 4kcam / 4k cam) into the one-liner name.toLowerCase().replace(/\s/g, '').includes('4kcam'). Both of its call sites are unchanged.

• 11.1 — Single-letter local n in the new helper — ✅ Addressed. The rewrite has no local at all; name.toLowerCase() is inlined, which is one of the two forms the finding named.
• 2.1 — Already-configured users keep the old name — 💬 Disputed. rafaellehmkuhl commented "no need to deal with migration of existing setups" (comment). Verified against the code: nothing in the diff migrates or renames an existing correspondency, so the comment describes the code as it stands. It is a maintainer stating the decision AGENTS.md asks for, which is why the finding is disputed rather than left plain :x: — but a comment is not a code change and does not close a finding, and the other half of the AGENTS.md rule ("tell the user what changed") is still not met anywhere the user or a release-notes reader will see. Open until /resolve.
• 1.1 — Widened substring matcher silently un-maps unrelated 4K cameras — ❌ Still not addressed (status unchanged, body reprinted in section 1). The '4k cam' clause is gone, but replace(/\s/g, '') before the includes('4kcam') test restores exactly that match and adds more: 4K Camera → 4kcamera, Bow 4K Cam Front → bow4kcamfront, 4 K C a m → 4kcam all still match. The finding's suggested fix was the opposite trade — keep the observed string, add a rebranded one only once it is known to be emitted, and anchor it so a generic name cannot match.
• 1.2 — new this round, created by the rewrite: dropping the radcam clause removes existing behaviour the PR body claims it preserves. See section 1.

Discussion since round 1. rafaellehmkuhl's comment also asks the author to "test it against the latest release of the br-4k-cam-manager extension, to see if our pipeline that automatically creates the streams for the 4K Cam are working properly with the new values". I cannot verify that from this repository — no file in the tree emits or references a 4kcam-style stream name (searched src/ for radcam, underwatercam, 4k cam, 4kcam) — so which names the extension actually produces remains the open question that both 1.1 and 1.2 hang on. ArturoManzoli's /review is the trigger for this round and carries nothing to review.

resolutions.json is empty: no finding has been closed by /resolve on this PR, and no unrecognised id was submitted.

Change map — what was established before judging

Claims (from the PR body, each checked against the code at this head)

1. "RTSP streams from the 4K Cam are shown as '4K Cam RTSP' instead of 'RadCam RTSP'." — verified for newly discovered streams only. rtspBaseName (src/stores/video.ts:199-215) builds the prefix, and its output is consumed exactly twice, both times as a new correspondency's internal name: initializeRtspStreamsCorrespondency at src/stores/video.ts:297 and addRtspStreamCorrespondency at src/stores/video.ts:1318. Both are guarded against re-adding an already-mapped URL (src/stores/video.ts:287-291, src/stores/video.ts:1312-1315), so a stream already present in cockpit-streams-correspondency never has its name recomputed. See 2.1.
2. "WebRTC streams whose names still say RadCam … continue to be auto-ignored." — contradicted. At round 1's head the helper kept an includes('radcam') clause; at this head it is name.toLowerCase().replace(/\s/g, '').includes('4kcam'), which no radcam name satisfies (radcam contains no 4kcam, with or without spaces). The base behaviour at src/stores/video.ts:223 and :242 is therefore removed, not preserved. The commit message makes the same claim ("existing MCM stream names still get ignored") and is contradicted the same way. See 1.2.
3. "[WebRTC streams that] now say 4K Cam [are auto-ignored]." — unverifiable from this repository, and in tension with the code the PR left alone. The RTSP side still identifies the device by the ONVIF source name underwatercam (src/stores/video.ts:211-212, matcher unchanged by the diff), and the reworded comment above it still states that this is how the camera announces itself. A repo-wide search for 4kcam / 4k cam finds only this diff. See 1.1.

Failure site — the PR fixes no bug; it is a rebrand plus a rewrite of an existing filter. The one behavioural regression it creates (claim 2) is inside the diff, in the added helper, and the two behaviours it replaces are at src/stores/video.ts:223 and :242 in the base.

Entry points

Function	Reached from	Frequency
is4kCamStreamName (added)	only from initializeStreamsCorrespondency, both call sites in the diff	per incoming message
initializeStreamsCorrespondency (changed)	watch(namesAvailableStreams) at src/stores/video.ts:312, fed by mainWebRTCManager.availableStreams through namesAvailableWebRTCStreams (src/stores/video.ts:104-113); also restoreIgnoredStream at src/stores/video.ts:1279	per incoming message (WebRTC signalling stream list), plus per user action on the restore path
rtspBaseName (changed)	initializeRtspStreamsCorrespondency at src/stores/video.ts:297, driven by watch(streamInformation) at src/stores/video.ts:316 where streamInformation.value is reassigned by the 5 s setInterval at src/stores/video.ts:97-100; also addRtspStreamCorrespondency at src/stores/video.ts:1318	per incoming message (5 s MCM poll), plus per user action when an RTSP URL is added by hand

Neither changed function is dead: both walks reach a watcher and a user-triggered path.

Invariants the change relies on

• An auto-ignored stream stays ignored unless the user restores it. Enforced through cockpit-user-restored-stream-ids: both call sites keep the !userRestoredStreamIds.value.includes(...) guard (src/stores/video.ts:224, :242 in base), and restoreIgnoredStream writes that list (src/stores/video.ts:1271-1273). This is also what bounds 1.2: a radcam stream already sitting in cockpit-ignored-stream-external-ids stays hidden via the early return at src/stores/video.ts:241, so the regression bites the not-yet-ignored cases — a fresh install, a new topside profile, a newly appearing stream, and a mapped-but-never-ignored stream.
• The name matcher and the ONVIF matcher identify the same device. Nothing enforces this, and this round the two now disagree completely: RTSP names the device from ONVIF sourceName containing underwatercam (src/stores/video.ts:211-212), while the WebRTC auto-ignore matches only 4kcam in the free-text stream name. The same ONVIF identity is available on the WebRTC side — streamInformation.value.find((i) => i.name === externalId)?.sourceName, the lookup already used at src/stores/video.ts:176 — and is used by neither of the two findings' call sites. This is the invariant 1.1 and 1.2 are both about.
• Only WebRTC correspondencies are subject to auto-ignore. Still holds: the protocol guard (corr.protocol ?? 'webrtc') === 'webrtc' is untouched and RTSP entries are always written with protocol: 'rtsp' (src/stores/video.ts:302, :1323), so a stream whose internal name now begins "4K Cam RTSP" cannot be swept up — the matcher reads externalId, which for those entries is the URL.

1. Correctness & Implementation Bugs — 2 findings (1 carried from round 1)

1.2 — Dropping the radcam clause removes the auto-ignore the PR says it keeps — major

Consequence: on any vehicle whose camera still announces the old name, the low-quality stuttering feed reappears in the video list and is mapped again, so users who were protected from it before this PR are not after it.

The added helper is now the only test on both auto-ignore paths:

const is4kCamStreamName = (name: string): boolean => name.toLowerCase().replace(/\s/g, '').includes('4kcam')

It replaces corr.externalId.toLowerCase().includes('radcam') at src/stores/video.ts:223 and streamName.toLowerCase().includes('radcam') at src/stores/video.ts:242. No string containing radcam satisfies the new test, so the behaviour those two lines implement in the base is deleted rather than extended — while the PR body says such streams "continue to be auto-ignored" and the commit message says "existing MCM stream names still get ignored". The TODO the diff itself rewords two lines above (src/stores/video.ts:219) states why the rule exists: it stays until "the MCM stutter problem is fixed", which this PR does not touch.

Who is affected, from the invariant walk above: any stream not already in cockpit-ignored-stream-external-ids for that vehicle. src/stores/video.ts:241 keeps previously ignored ids hidden, so an installation that has already been through the auto-ignore is unaffected; a fresh setup, a vehicle paired with a topside that has never seen the stream, or a camera on firmware or an extension release that still reports RadCam will now have the WebRTC feed mapped and playable. deleteStreamCorrespondency clearing userRestoredStreamIds "so auto-ignore can re-apply" (src/stores/video.ts:1242-1246) also no longer re-applies for those names.

Suggested fix: keep the old string alongside the new one — the two are not mutually exclusive, and a rebrand is precisely the case where both names are in the field at once. Better, since both brand names describe one device that already has a stable identifier: match the ONVIF sourceName for WebRTC too, via streamInformation.value.find((i) => i.name === externalId)?.sourceName (the lookup at src/stores/video.ts:176), which is the same underwatercam test the RTSP side at src/stores/video.ts:212 uses and is immune to the next rename. Either way, if the intent really is to stop ignoring RadCam streams, that is a behaviour change that needs its own commit and its own line in the PR body, not a side effect of a rename.

1.1 — Substring matcher silently un-maps unrelated 4K cameras — major (carried from round 1)

Consequence: anyone using a WebRTC stream whose name contains "4K cam" — a generic 4K camera, not this product — has that stream vanish from their video configuration and lose any name they gave it, without being told why.

src/stores/video.ts (added helper, replacing the tests at :223 and :242):

const is4kCamStreamName = (name: string): boolean => name.toLowerCase().replace(/\s/g, '').includes('4kcam')

Stripping whitespace before a substring test widens rather than narrows the match. 4K Camera → 4kcamera, Bow 4K Cam Front → bow4kcamfront, Front 4K camera (starboard) → front4kcamera(starboard) — all contain 4kcam and all match. The round-1 concern is unchanged: matching a generic name here is not a label change, it drives destructive bookkeeping. On the already-mapped path (src/stores/video.ts:226-233) the entry is removed from cockpit-streams-correspondency and its external id appended to cockpit-ignored-stream-external-ids; both are useBlueOsStorage keys, so the removal propagates to every topside computer for that vehicle. The user-visible result: the stream disappears from the configuration list (recoverable only through the "(N ignored)" section at src/views/ConfigurationVideoView.vue:151-153), any widget bound to its internal name loses its source, and a user-chosen internal name is not restored — restoreIgnoredStream (src/stores/video.ts:1263-1288) rebuilds the correspondency through initializeStreamsCorrespondency, which derives the name from the external name again (src/stores/video.ts:259-267). There is no snackbar on the automatic path, unlike the manual deleteStreamCorrespondency which does notify (src/stores/video.ts:1257).

Two things still make the match hard to justify as written:

• Nothing in the repository shows a stream named 4kcam or 4k cam existing, and the maintainer comment on this PR asks for exactly that to be tested against the extension rather than asserting it. The device's own identifier, on the RTSP side of this same diff, is still the ONVIF source name underwatercam (src/stores/video.ts:211-212, unchanged), and the comment the PR reworded at :210 still says so.
• The file identifies one camera two different ways — ONVIF sourceName for RTSP, free-text stream name for WebRTC — and after this round they no longer overlap at all (Change map, invariant 2). A reader at either site cannot tell the other exists.

Suggested fix, and it is the same one that closes 1.2: identify the device by the ONVIF source name on both paths (streamInformation.value.find((i) => i.name === externalId)?.sourceName, the lookup at src/stores/video.ts:176). If the name test has to stay, match the exact name the extension emits — confirmed against the release, per the maintainer's request — instead of a substring that swallows the generic term, and keep the radcam string while both names are in the field.

2. Persistence & User Data — inventory, 1 finding (carried from round 1, now disputed)

Inventory. The PR adds, reshapes and removes no persisted key. It changes which values get written into three existing ones, all vehicle-synced via useBlueOsStorage — i.e. shared by every topside computer and every operator of that vehicle:

Key	Backend	What the PR does to it
cockpit-streams-correspondency (src/stores/video.ts:54)	vehicle-synced (useBlueOsStorage)	unchanged shape. New RTSP entries get name: '4K Cam RTSP …' instead of 'RadCam RTSP …' (src/stores/video.ts:297, :1318). Entries are removed for WebRTC streams the new matcher catches (src/stores/video.ts:228), and — new this round — no longer removed for radcam names (finding 1.2).
cockpit-ignored-stream-external-ids (src/stores/video.ts:55)	vehicle-synced	unchanged shape; gains ids for names matching 4kcam (src/stores/video.ts:231, :250), and no longer gains ids for radcam names.
cockpit-user-restored-stream-ids (src/stores/video.ts:72)	vehicle-synced	read only; the PR keeps both guards that consult it.

Judged: all three keys are cockpit--prefixed; none stores a machine-specific value (external stream names and RTSP URLs describe the vehicle's camera setup, not the topside machine, so vehicle-sync is the right backend and no auto-connect is taken on a synced device path); no id field duplicates its key; no automatic migration is introduced, and none should be (below); nothing writes undefined.

2.1 — Already-configured users keep the old name, with no notice and no stated decision — minor, disputed (carried from round 1)

Consequence: on any installation where the camera was already discovered, the stream keeps showing the old brand name, so the change the PR advertises is invisible to exactly the users who have the camera today.

rtspBaseName's output only ever becomes a new correspondency's name (src/stores/video.ts:297 behind the unmappedRtspUrls filter at :287-291, and src/stores/video.ts:1318 behind the duplicate check at :1312-1315). An existing cockpit-streams-correspondency entry named RadCam RTSP 192.168.2.2 … is never recomputed, so it survives the rename indefinitely.

Not migrating is the right call, and a maintainer has now said so explicitly in the discussion. The internal name is the key that widget options and snapshot/video filenames are stored against (per the AGENTS.md video/snapshot naming rules), so rewriting it in place would break those references, and AGENTS.md treats automatic user-data migrations as a last resort. The gap the finding is about is the other half of the same AGENTS.md rule: "When a change to a default or to existing behavior leaves already-configured users on the old value, decide explicitly whether to carry them over or to leave them alone — and when you leave them, tell the user what changed." Neither the PR body nor the commit message states the decision; the body instead asserts the new name as if it applied everywhere (Change map, claim 1).

Suggested fix: no code change to the naming logic. State in the PR body (and the release notes) that existing streams keep their name and can be renamed by hand in the video configuration — the rename path already exists (src/stores/video.ts:1218-1234).

Sections with nothing to report (9)

3. AGENTS.md Adherence — ✅ (package.json untouched, so no dependency or ordering question; the radCamMapped→fourKCamMapped and radCamToIgnore→fourKCamToIgnore renames serve the PR's stated purpose, so scope discipline holds; the three reworded comments at src/stores/video.ts:210, :218, :219 all sit on code lines the diff also changes and had become factually wrong, satisfying the comment-immutability rule; no JSDoc was added and none is required — the new helper is an arrow function and .eslintrc.cjs sets jsdoc/require-jsdoc ArrowFunctionExpression: false, matching its unJSDoc'd neighbours rtspBaseName and uniqueInternalName. The AGENTS.md breaches I did find — "do not write code for a future PR" and the "tell the user what changed" half of the persistence rule — are folded into 1.1 and 2.1 rather than counted twice.)

4. Security — ✅ (all nine sub-checks run against this head: the diff touches only src/stores/video.ts, no workflow, postinstall, Dockerfile or src/electron/ file; no dependency added; no fetch/websocket/host introduced; no eval/Function()/v-html; no env var, token or secret; no encoded blob or binary-like literal; a scan of pr.diff for non-ASCII characters returns nothing, so no hidden or bidirectional Unicode; nothing bundled or downloaded, so no licensing surface. Neither pr.json, pr.diff, complexity-report.json nor either entry in new-comments.json contains text addressed to a reviewing agent.)

5. Performance — ✅ (both changed functions traced to their entry points in the Change map — the 5 s streamInformation poll watcher at src/stores/video.ts:316 and the WebRTC availableStreams watcher at :312; the rewritten helper replaces up to three String.includes with one String.replace plus one includes per candidate name on a list a handful of entries long, no new watcher, interval, listener or subscription is registered so nothing needs teardown, and no canvas or network work is added.)

6. UI / UX — ✅ (no dialog, overlay, teleporting Vuetify control, button, icon control or footer is added or changed, so the dialog-anatomy, theme="dark", token, padding, glass and stacking clauses have nothing to apply to; no new user interaction exists to log via logUserAction; the only user-visible string is the auto-generated stream name, whose "RTSP" jargon is inherited from src/stores/video.ts:212 and not introduced here; an auto-ignored stream stays reachable through the "(N ignored)" list at src/views/ConfigurationVideoView.vue:151-153. The missing feedback on the automatic un-mapping path is part of 1.1.)

7. Code Quality & Style — ✅ (complexity-report.json for head 6773f68 reports, as its own figures, 0 triggered functions across 101 measured in 1 changed file, truncated: false, so no complexity or depth finding arises and the silence covers the whole change set; the helper keeps its explicit boolean return type as @typescript-eslint/explicit-function-return-type requires, uses no any, needs no optional chaining, adds no scoped CSS and no wrapped string literal; the internal/external stream-name rule holds — rtspBaseName output feeds uniqueInternalName as an internal name and no external name is written into storage; the three duplicated includes('radcam') tests are now one shared helper, which is the direction AGENTS.md asks for. What the helper matches is 1.1 and 1.2, not a style question.)

8. Commit Hygiene — ✅ (still one commit, video: rename RadCam mentions to 4K Cam, matching the area-prefix style dominant in this repository's git log, and the prefix fits the change; no wip/fixup!/squash! and no self-correcting commit — round 1's head was amended into this one rather than fixed up on top, which is what commit hygiene asks for; 15/13 lines is one logical change reviewable in one sitting, so neither over-split nor oversized; no GitHub issue or PR reference in the subject or body, the Closes #2952 correctly living in the PR body only. The commit message's factual claim about radcam names is judged as 1.2, not here.)

9. Tests — ✅ (no test file appears in the PR's single-file change set, and no existing assertion or check was removed or weakened; the video store has no test suite in the base ref to weaken.)

10. Documentation — ✅ (nothing changes about Lite vs Standalone parity — the RTSP discovery path is already Electron-gated at src/stores/video.ts:274 and that guard is untouched — so the README feature table needs no row; a search of README.md for "RadCam"/"4K" finds no mention to update. The user-facing note this change needs is tracked as 2.1 rather than duplicated here.)

11. Nitpicks / Optional — ✅ (round 1's only entry, the single-letter local, is gone with the helper rewrite; nothing else in the two hunks is worth raising.)

Generated by Claude. This is advisory; a human reviewer must still approve.

[ArturoManzoli]

Done:

• (1.1) anchored the matcher to the stream-name prefix, so 4K Camera, Bow 4K Cam Front and Front 4K camera (starboard) no longer match and no unrelated camera gets un-mapped.
• (1.2) put the RadCam name back, alongside 4K Cam. Checked it against the extension instead of guessing: br4kcam-manager names its MCM streams <brand> <host>/<feed> and the rebrand landed in bbc50ec5e, so RadCam … is what every release up to 0.2.3 emits (the only stable one out there) and 4K Cam … starts at 0.3.0-beta.1. Dropping it would have un-hidden the stuttering WebRTC feed for everyone not on a beta.
• (2.1) stated in the PR body that already-discovered streams keep their stored name and can be renamed by hand in the video configuration. No code change, per your call on migrations.

The matcher is now /^(4k cam|radcam) / against the lowercased name, and the commit body no longer claims more than the code does.

Note: round 2's head was amended, so this round's diff is again the whole PR rather than an increment.

[ArturoManzoli]

/review

[github-actions]

📝 MINOR SUGGESTIONS (Automated PR Review — round 3)

1 open (1 minor) · 4 closed (1.1, 1.2, 2.1, 11.1)

One file changes, the video store. Auto-discovered RTSP streams coming from the underwater camera are now named "4K Cam RTSP …" instead of "RadCam RTSP …", and the rule that automatically hides that camera's stuttering WebRTC feed (so the better RTSP feed is used instead) now recognises both the old and the new brand name, but only when the name begins with one of them — so an unrelated camera called "4K Camera" is no longer swept up. Both of the major problems raised in the previous rounds are fixed, and the PR text now says that cameras already set up keep the name they were given. What is left is a corner case in the same matching rule.

What still needs attention

#	Problem	What it means	Severity	Status
1.3	Matcher assumes the stream name has no leading space	If the camera reports its stream name with a stray space in front, its stuttering feed stops being hidden and reappears in the user's video list — a case the code right next to it already guards against.	minor	❌

Since round 2 — 3 closed, 1 new, comparing 6773f68 → c10e653

Range: 6773f689064fcc49d4cf157513bb7e55f3063a52 → c10e653a5a47715f392df9931d78b84516db3189.

incremental.diff is again not usable as an increment. pr.json carries a single commit whose oid equals the current head, with an authored date (2026-08-19) earlier than its commit date (2026-08-20), i.e. round 2's head was amended away; the file I was handed reproduces the whole PR (the same two hunks, +17/-13, as pr.diff) rather than the changes since 6773f68. The author says the same in their comment. Status transitions below are therefore derived from pr.diff compared against the code round 2 quoted, not from the increment.

What moved in the code: the helper was rewritten from name.toLowerCase().replace(/\s/g, '').includes('4kcam') to /^(4k cam|radcam) /.test(name.toLowerCase()), and gained a two-line comment recording the extension's naming format. Both call sites are unchanged. The PR body gained a fourth bullet.

• 1.1 — Substring matcher silently un-maps unrelated 4K cameras — ✅ Addressed. The finding asked for the match to be anchored to the name the extension actually emits instead of a substring that swallows the generic term. All three names it named now fail the test: 4K Camera → 4k camera (the alternation requires a space after 4k cam), Bow 4K Cam Front and Front 4K camera (starboard) (anchored at ^). A stream that genuinely begins 4K Cam still matches, which is the intended target rather than a residue.
• 1.2 — Dropping the radcam clause removes the auto-ignore the PR says it keeps — ✅ Addressed. The finding asked for the old brand string to be kept alongside the new one; radcam is back as the second alternative, so a stream named RadCam <host>/<feed> is auto-ignored again, and the commit message no longer claims more than the code does. The anchoring the same rewrite introduces narrows the old includes('radcam') test, which is finding 1.3 below rather than a residue of this one.
• 2.1 — Already-configured users keep the old name, with no notice and no stated decision — ✅ Addressed, and no longer disputed. This finding's own remedy was explicitly not a code change: state in the PR body that existing streams keep their stored name and can be renamed by hand. The PR body in pr.json now carries exactly that as its fourth bullet ("Streams already discovered keep their stored name, 'RadCam RTSP ...' included, and can be renamed by hand in the video configuration"), and the rename path it points at exists (renameStreamInternalNameById, src/stores/video.ts:1219-1234). I am closing it on that artifact, which I verified in pr.json, not on the author's explanation of it.
• 11.1 — Single-letter local n in the new helper — closed in round 2, unchanged.
• 1.3 — new this round, created by the anchoring: the new matcher requires the brand to be the first token of an untrimmed name, while the sibling code at src/stores/video.ts:260 treats the same names as possibly untrimmed. See section 1.

Discussion since round 2. ArturoManzoli left one substantive comment (link) listing the three fixes and stating that the extension names its MCM streams <brand> <host>/<feed>, with RadCam through release 0.2.3 and 4K Cam from 0.3.0-beta.1, and that this was checked against the extension repository rather than assumed. That is a claim about a repository I cannot see: nothing in this tree emits or references either stream name (src/ search for radcam, 4k cam, 4kcam, underwatercam returns only src/stores/video.ts itself), so I can confirm the code matches the format the comment describes but not that the extension emits it. It answers the verification rafaellehmkuhl asked for in round 1, and the two findings that hung on it are closed on the code, which now covers both brand names either way. The second comment is the bare /review that triggered this round.

resolutions.json is empty: no finding has been closed by /resolve on this PR, and no unrecognised id was submitted. decisions.json is empty: no vote was ever recorded on 2.1's dispute, which closed on the PR body instead.

Change map — what was established before judging

Claims (from the PR body, each checked against the code at this head)

1. "RTSP streams from the 4K Cam are shown as '4K Cam RTSP' instead of 'RadCam RTSP'." — verified, for newly discovered streams. rtspBaseName (src/stores/video.ts:199-215) builds the prefix, and its output is consumed exactly twice, both times as a new correspondency's internal name: initializeRtspStreamsCorrespondency at src/stores/video.ts:297, behind the unmapped filter at :287-291, and addRtspStreamCorrespondency at src/stores/video.ts:1318, behind the duplicate check at :1312-1315. The PR body's fourth bullet now states this limitation itself.
2. "The 4K Cam's WebRTC feed keeps being auto-ignored … whether the manager extension names the stream '4K Cam …' or 'RadCam …'." — verified against the code, unverifiable as to the extension. /^(4k cam|radcam) / matches both brands, so the base behaviour at src/stores/video.ts:223 and :242 is preserved for names of the stated shape. Which names the extension emits is not observable from this repository (see the discussion note above), and the narrower cases the base matcher covered are finding 1.3.
3. "The match is anchored to that name prefix, so an unrelated camera named '4K Camera' is no longer swept up." — verified. The alternation requires a literal space after the brand, so 4k camera does not match, and ^ prevents a mid-name occurrence from matching.
4. "Streams already discovered keep their stored name … and can be renamed by hand in the video configuration." — verified. Nothing recomputes an existing correspondency's name (claim 1's two guards), and renameStreamInternalNameById (src/stores/video.ts:1219-1234) is the manual path.

Failure site — the PR fixes no bug in this repository; it is a rebrand plus a rewrite of an existing filter. The behaviour it replaces lives at src/stores/video.ts:212, :223 and :242 in the base, all three inside the diff.

Entry points

Function	Reached from	Frequency
is4kCamStreamName (added)	only from initializeStreamsCorrespondency, both call sites in the diff	per incoming message
initializeStreamsCorrespondency (changed)	watch(namesAvailableStreams) at src/stores/video.ts:312, fed by mainWebRTCManager.availableStreams through namesAvailableWebRTCStreams (src/stores/video.ts:104-113); also restoreIgnoredStream at src/stores/video.ts:1279	per incoming message (WebRTC signalling stream list), plus per user action on the restore path
rtspBaseName (changed)	initializeRtspStreamsCorrespondency at src/stores/video.ts:297, driven by watch(streamInformation) at src/stores/video.ts:316 where streamInformation.value is reassigned by the 5 s setInterval at src/stores/video.ts:97-100; also addRtspStreamCorrespondency at src/stores/video.ts:1318	per incoming message (5 s MCM poll), plus per user action when an RTSP URL is added by hand

Neither changed function is dead: both walks reach a watcher and a user-triggered path.

Invariants the change relies on

• An auto-ignored stream stays ignored unless the user restores it. Enforced through cockpit-user-restored-stream-ids: both call sites keep the !userRestoredStreamIds.value.includes(...) guard (src/stores/video.ts:224, :242 in base), and restoreIgnoredStream writes that list (src/stores/video.ts:1271-1273). Untouched by this round.
• Every stream name the extension emits begins with the brand followed by a space. This is the rule the rewritten matcher now depends on, and it is stated only in the added comment. Who can violate it: the extension itself on a release whose format differs (not observable here); a name arriving with leading whitespace or an empty name, which the same function treats as possible 18 lines later at src/stores/video.ts:260 (streamName.trim() || 'Stream'). The PR covers neither — the first is out of reach of this repository, the second is finding 1.3.
• Only WebRTC correspondencies are subject to auto-ignore. Still holds: the protocol guard (corr.protocol ?? 'webrtc') === 'webrtc' is untouched and RTSP entries are always written with protocol: 'rtsp' (src/stores/video.ts:302, :1323). A stream whose internal name now begins "4K Cam RTSP" cannot be swept up either way — the matcher reads externalId, which for those entries is the rtsp://… URL and cannot match an anchored brand prefix.

1. Correctness & Implementation Bugs — 1 finding

1.3 — Anchoring the matcher drops the untrimmed names the base test caught — minor

Consequence: if the camera reports its stream name with a leading space, the stuttering WebRTC feed this rule exists to hide is mapped and playable again for that user, with nothing telling them why.

const is4kCamStreamName = (name: string): boolean => /^(4k cam|radcam) /.test(name.toLowerCase())

The ^ and the trailing space are the right narrowing for 4K Camera, and they are what closes 1.1. They also drop matches the base tests made at src/stores/video.ts:223 and :242 (…toLowerCase().includes('radcam')): any name where the brand is not the first token, and any name carrying leading whitespace. The second is not hypothetical inside this very function — initializeStreamsCorrespondency treats the same list of external names as possibly untrimmed, and possibly empty, 18 lines below the call site: uniqueInternalName(streamName.trim() || 'Stream', existingInternalNames) at src/stores/video.ts:260. One site in one function assumes those names need trimming; the other now assumes they arrive pre-trimmed.

For a name that slips through, the consequence is the one the TODO at src/stores/video.ts:219 describes: the id never enters cockpit-ignored-stream-external-ids, a correspondency is created for it at :262-265, and the WebRTC feed whose stutter motivated the whole rule is mapped and playable.

Suggested fix: test the trimmed name — /^(4k cam|radcam) /.test(name.trim().toLowerCase()) — which costs one call and restores the case the neighbouring line already anticipates. If mid-name occurrences are meant to stop matching, that is a deliberate narrowing of shipped behaviour and belongs in the comment above the helper, which currently records the extension's format without saying that anything outside it is no longer ignored.

2. Persistence & User Data — inventory, no findings

Inventory. The PR adds, reshapes and removes no persisted key. It changes which values get written into three existing ones, all vehicle-synced via useBlueOsStorage — i.e. shared by every topside computer and every operator of that vehicle:

Key	Backend	What the PR does to it
cockpit-streams-correspondency (src/stores/video.ts:54)	vehicle-synced (useBlueOsStorage)	unchanged shape. New RTSP entries get name: '4K Cam RTSP …' instead of 'RadCam RTSP …' (src/stores/video.ts:297, :1318); existing entries are never recomputed. WebRTC entries are removed for names matching the new anchored matcher — the same radcam names as the base, plus 4K Cam … ones.
cockpit-ignored-stream-external-ids (src/stores/video.ts:55)	vehicle-synced	unchanged shape; gains ids for names beginning 4K Cam or RadCam . Relative to the base it gains the new brand and keeps the old one; relative to round 2 it stops sweeping in generic 4K cam… names (1.1).
cockpit-user-restored-stream-ids (src/stores/video.ts:72)	vehicle-synced	read only; the PR keeps both guards that consult it.

Judged: all three keys are cockpit--prefixed; none stores a machine-specific value (external stream names and RTSP URLs describe the vehicle's camera setup, not the topside machine, so vehicle-sync is the right backend and nothing auto-connects on a synced device path); no id field duplicates its key; no automatic migration is introduced, and per the maintainer's call none should be; nothing writes undefined. The one persistence gap raised in earlier rounds — telling already-configured users their stream keeps the old name — is now stated in the PR body (2.1, closed above).

Sections with nothing to report (9)

3. AGENTS.md Adherence — ✅ (package.json untouched, so no dependency or ordering question; the radCamMapped→fourKCamMapped and radCamToIgnore→fourKCamToIgnore renames serve the PR's stated purpose, so scope discipline holds; the three reworded comments at src/stores/video.ts:210, :218, :219 all sit on code lines the diff also changes and had become factually wrong; the added two-line comment above the helper explains why both brand strings are matched, which is the one thing not obvious from the regex, and no JSDoc is required — the helper is an arrow function and .eslintrc.cjs sets jsdoc/require-jsdoc ArrowFunctionExpression: false, matching its neighbours rtspBaseName and uniqueInternalName; nothing is added without a call site in this PR.)

4. Security — ✅ (all nine sub-checks run against this head: the diff touches only src/stores/video.ts, no workflow, postinstall, Dockerfile or src/electron/ file; no dependency added; no fetch/websocket/host introduced; no eval/Function()/v-html; no env var, token or secret; no encoded blob or binary-like literal; a scan of pr.diff for non-ASCII characters returns nothing, so no hidden or bidirectional Unicode; nothing bundled or downloaded, so no licensing surface. Neither pr.json, pr.diff, complexity-report.json nor either entry in new-comments.json contains text addressed to a reviewing agent — the author's comment discusses the findings, which is ordinary PR discussion, and I treated its factual claims as claims to check rather than as evidence.)

5. Performance — ✅ (both changed functions traced to their entry points in the Change map — the 5 s streamInformation poll watcher at src/stores/video.ts:316 and the WebRTC availableStreams watcher at :312; the helper replaces one String.replace plus one includes with one anchored RegExp.test per candidate name, on a list a handful of entries long; no watcher, interval, listener or subscription is registered, so nothing needs teardown, and no canvas or network work is added.)

6. UI / UX — ✅ (no dialog, overlay, teleporting Vuetify control, button, icon control or footer is added or changed, so the dialog-anatomy, theme="dark", token, padding, glass and stacking clauses have nothing to apply to; no new user interaction exists to log via logUserAction; the only user-visible string is the auto-generated stream name, whose "RTSP" jargon is inherited from src/stores/video.ts:212 and not introduced here; an auto-ignored stream stays reachable through the "(N ignored)" list at src/views/ConfigurationVideoView.vue:151-153, and with 1.1 closed the automatic un-mapping path no longer reaches streams the user did not expect it to.)

7. Code Quality & Style — ✅ (complexity-report.json for head c10e653 reports, as its own figures, 0 triggered functions across 101 measured in 1 changed file, truncated: false, so no complexity or depth finding arises and that silence covers the whole change set; the helper keeps its explicit boolean return type as @typescript-eslint/explicit-function-return-type requires, uses no any, needs no optional chaining, adds no scoped CSS and no wrapped string literal, and stays inside the 180-column max-len; the internal/external stream-name rule holds — rtspBaseName output feeds uniqueInternalName as an internal name and no external name is written into storage; the three duplicated brand tests remain one shared helper. What the helper matches is 1.3, not a style question.)

8. Commit Hygiene — ✅ (still one commit, video: rename RadCam mentions to 4K Cam, matching the area-prefix style dominant in this repository's git log, and the prefix fits the change; its body now describes the code accurately, matching both brands rather than claiming preservation the code did not deliver; no wip/fixup!/squash! and no self-correcting commit — round 2's head was amended into this one rather than fixed up on top; 17/13 lines is one logical change reviewable in one sitting, so neither over-split nor oversized; no GitHub issue or PR reference in the subject or body, the Closes #2952 correctly living in the PR body only.)

9. Tests — ✅ (no test file appears in the PR's single-file change set, and no existing assertion or check was removed or weakened; the video store has no test suite in the base ref to weaken.)

10. Documentation — ✅ (nothing changes about Lite vs Standalone parity — the RTSP discovery path is already Electron-gated at src/stores/video.ts:274 and that guard is untouched — so the README feature table needs no row; a search of README.md for "RadCam"/"4K" finds no mention to update; the user-facing note this change needed is now in the PR body, which closed 2.1.)

11. Nitpicks / Optional — ✅ (round 1's single-letter local is long gone; the remaining taste question in the two hunks, /^…/i.test(name) instead of toLowerCase() plus a lowercase pattern, is not worth a round of review.)

Generated by Claude. This is advisory; a human reviewer must still approve.

[ArturoManzoli]

Done:

• (1.3) matcher now tests the trimmed name, so a leading space can't slip a stream past it. Matches the assumption the neighbouring uniqueInternalName(streamName.trim() || 'Stream', ...) already makes 18 lines down.

Mid-name occurrences stay unmatched on purpose: the extension puts the brand first, and matching it anywhere is what un-mapped unrelated cameras in 1.1.

Note: folded into the same commit again, so this round's diff is once more the whole PR rather than an increment.

[ArturoManzoli]

/review

[github-actions]

📝 MINOR SUGGESTIONS (Automated PR Review — round 4)

1 open (1 minor) · 5 closed (1.1, 1.2, 1.3, 2.1, 11.1)

One file changes, the video store. Auto-discovered RTSP streams coming from the underwater camera are now named "4K Cam RTSP …" instead of "RadCam RTSP …", and the rule that automatically hides that camera's stuttering WebRTC feed (so the better RTSP feed is used instead) recognises both the old and the new brand name, but only when the name starts with one of them — so an unrelated camera called "4K Camera" is no longer swept up. This round's only code change closes the last open correctness finding: the name is trimmed before it is matched, so a stray leading space can no longer slip a stream past the rule. What is left is the name the user actually reads, which is still the generic term.

What still needs attention

#	Problem	What it means	Severity	Status
6.1	Stream name says "4K Cam" without naming the vendor	Someone running the Blue Robotics camera next to any other 4K camera sees a stream called just "4K Cam RTSP …" and cannot tell from the name which of the two it is.	minor	❌

Since round 3 — 1 closed, 1 new, comparing c10e653 → d0c29d1

Range: c10e653a5a47715f392df9931d78b84516db3189 → d0c29d1db02c3dc488480cb9b9885b69f6002faa.

incremental.diff is once again not usable as an increment. It reproduces the whole PR — the same two hunks, +17/-13, as pr.diff — rather than the changes since c10e653. pr.json still carries a single commit whose oid equals the current head, with an authored date (2026-08-19T17:25:41Z) earlier than its commit date (2026-08-20T20:46:31Z), i.e. round 3's head was amended away, and the author states the same in their comment ("folded into the same commit again"). The status transitions below are therefore derived from pr.diff compared against the code round 3 quoted, not from the increment.

What moved in the code: exactly one call. is4kCamStreamName went from /^(4k cam|radcam) /.test(name.toLowerCase()) to /^(4k cam|radcam) /.test(name.trim().toLowerCase()). Both call sites, the two-line comment above the helper, the RTSP prefix line and the PR body are identical to round 3.

• 1.3 — Anchoring the matcher drops the untrimmed names the base test caught — ✅ Addressed. The finding asked for two things. The substantive one — test the trimmed name, so the anchored matcher stops disagreeing with uniqueInternalName(streamName.trim() || 'Stream', …) 18 lines below the call site (src/stores/video.ts:264 at this head) — landed verbatim at src/stores/video.ts:219: a name arriving as " RadCam 192.168.2.2/main" now matches again and is auto-ignored, which was the whole consequence the finding described. The second was a conditional aside: if mid-name occurrences are meant to stop matching, say so in the comment above the helper. That is not in the code, and the author answered it in prose instead ("Mid-name occurrences stay unmatched on purpose"), which by itself closes nothing. I am dropping the aside on my own reading rather than on that argument: it asked for an extra comment sentence where the existing two-line comment already records the extension's <brand> <host>/<feed> format — from which the anchor follows — and AGENTS.md's comment policy ("No new comments unless they will save the reader real time understanding why", "Avoid repeated comments") weighs against adding one. The finding closes on the .trim() in the code, not on the explanation of it.
• 1.1, 1.2, 2.1, 11.1 — closed in earlier rounds, and nothing this round reopens them: the anchoring that closed 1.1 is intact (^ plus the required space), both brand alternatives that closed 1.2 are intact, the PR body's fourth bullet that closed 2.1 is unchanged, and the helper carries no single-letter local.
• 6.1 — new this round, raised out of the review discussion and verified against the code: the matcher was narrowed so the generic term cannot sweep up an unrelated camera, but the name Cockpit generates and shows is still that generic term (src/stores/video.ts:212). See section 6.

Discussion since round 3. Four comments, all from the PR thread, none of which I treated as evidence about the code.

• rafaellehmkuhl, inline on src/stores/video.ts (r3825205816): in several places like this the string should name Blue Robotics' 4K Cam so it is not confused with regular 4K cams. ArturoManzoli agreed (r3825215820). I checked it against the diff: the only such string the PR introduces is the RTSP name prefix at src/stores/video.ts:212, and it is unqualified at this head. That is finding 6.1.
• ArturoManzoli (r3825224387) asks whether qualifying that string would be a problem "on the prefix too". It would not, and the answer is in 6.1: the displayed prefix and the matched prefix are two different strings, only one of which Cockpit chooses.
• ArturoManzoli (issue comment 5361476916) lists the trim fix and the deliberate mid-name narrowing, and notes the commit was amended again. Both claims check out against pr.diff and pr.json; 1.3 is closed on the code above, not on the comment.
• The fourth comment is the bare /review that triggered this round.

resolutions.json is empty: no finding has been closed by /resolve on this PR, and no unrecognised id was submitted. decisions.json is empty: no dispute has ever been put to a vote here — 2.1, the only finding that was ever disputed, closed on the PR body in round 3.

Change map — what was established before judging

Claims (from the PR body, each checked against the code at this head)

1. "RTSP streams from the 4K Cam are shown as '4K Cam RTSP' instead of 'RadCam RTSP'." — verified, for newly discovered streams. rtspBaseName (src/stores/video.ts:199-215) builds the prefix, and its output is consumed exactly twice, both times as a new correspondency's internal name: initializeRtspStreamsCorrespondency at src/stores/video.ts:301, behind the unmapped filter at :291-295, and addRtspStreamCorrespondency at src/stores/video.ts:1322, behind the duplicate check at :1316-1319. The PR body's fourth bullet states this limitation itself.
2. "The 4K Cam's WebRTC feed keeps being auto-ignored … whether the manager extension names the stream '4K Cam …' or 'RadCam …'." — verified against the code, unverifiable as to the extension. /^(4k cam|radcam) / matches both brands, so the base behaviour at src/stores/video.ts:223 and :242 is preserved for names of the stated shape, and this round's .trim() restores it for names carrying leading whitespace. Which names the extension emits is not observable from this repository: a search of src/ for radcam, 4k cam, 4kcam and underwatercam returns only src/stores/video.ts itself.
3. "The match is anchored to that name prefix, so an unrelated camera named '4K Camera' is no longer swept up." — verified for the matcher. '4k camera' fails the alternation (a literal space is required after 4k cam), and ^ after the trim prevents a mid-name occurrence from matching. The name Cockpit displays is a separate string and is not covered by this claim — finding 6.1.
4. "Streams already discovered keep their stored name … and can be renamed by hand in the video configuration." — verified. Nothing recomputes an existing correspondency's name (claim 1's two guards), and renameStreamInternalNameById (src/stores/video.ts:1223-1238) is the manual path.

Failure site — the PR fixes no bug in this repository; it is a rebrand plus a rewrite of an existing filter. The behaviour it replaces lives at src/stores/video.ts:212, :223 and :242 in the base ref, all three inside the diff.

Entry points

Function	Reached from	Frequency
is4kCamStreamName (added, src/stores/video.ts:219)	only from initializeStreamsCorrespondency, both call sites in the diff (:227, :246)	per incoming message
initializeStreamsCorrespondency (changed)	watch(namesAvailableStreams) at src/stores/video.ts:316, fed by mainWebRTCManager.availableStreams through namesAvailableWebRTCStreams (src/stores/video.ts:104-113); also restoreIgnoredStream at src/stores/video.ts:1283	per incoming message (WebRTC signalling stream list), plus per user action on the restore path
rtspBaseName (changed)	initializeRtspStreamsCorrespondency at src/stores/video.ts:301, driven by watch(streamInformation) at src/stores/video.ts:320 where streamInformation.value is reassigned by the 5 s setInterval at src/stores/video.ts:97-100; also addRtspStreamCorrespondency at src/stores/video.ts:1322	per incoming message (5 s MCM poll), plus per user action when an RTSP URL is added by hand

Neither changed function is dead: both walks reach a watcher and a user-triggered path.

Invariants the change relies on

• An auto-ignored stream stays ignored unless the user restores it. Enforced through cockpit-user-restored-stream-ids: both call sites keep the !userRestoredStreamIds.value.includes(...) guard (src/stores/video.ts:228, :246), and restoreIgnoredStream writes that list (src/stores/video.ts:1275-1277). Untouched by this round.
• Every stream name the extension emits begins with the brand followed by a space. This is the rule the matcher depends on, stated only in the added comment. Who can violate it: the extension itself, on a release whose format differs or that qualifies the brand (not observable here, and see 6.1 for the coupling); a name arriving with surrounding whitespace, which this round's .trim() now covers and which the same function assumes possible at src/stores/video.ts:264 (streamName.trim() || 'Stream'); an empty name, which fails the test and falls through to that same 'Stream' fallback. The two in-repository violators are now both covered; the extension-side one is out of reach of this repository either way, since the pattern already carries both brands.
• Only WebRTC correspondencies are subject to auto-ignore. Still holds: the protocol guard (corr.protocol ?? 'webrtc') === 'webrtc' is untouched and RTSP entries are always written with protocol: 'rtsp' (src/stores/video.ts:306, :1327). A stream whose internal name now begins "4K Cam RTSP" cannot be swept up either way — the matcher reads externalId, which for those entries is the rtsp://… URL and cannot match an anchored brand prefix.

2. Persistence & User Data — inventory, no findings

Inventory. The PR adds, reshapes and removes no persisted key. It changes which values get written into three existing ones, all vehicle-synced via useBlueOsStorage — i.e. shared by every topside computer and every operator of that vehicle:

Key	Backend	What the PR does to it
cockpit-streams-correspondency (src/stores/video.ts:54)	vehicle-synced (useBlueOsStorage)	unchanged shape. New RTSP entries get name: '4K Cam RTSP …' instead of 'RadCam RTSP …' (src/stores/video.ts:301, :1322); existing entries are never recomputed. WebRTC entries are removed for names matching the anchored matcher — the same radcam names as the base, plus 4K Cam … ones, and as of this round names with surrounding whitespace again.
cockpit-ignored-stream-external-ids (src/stores/video.ts:55)	vehicle-synced	unchanged shape; gains ids for names beginning 4K Cam or RadCam after trimming.
cockpit-user-restored-stream-ids (src/stores/video.ts:72)	vehicle-synced	read only; the PR keeps both guards that consult it.

Judged: all three keys are cockpit--prefixed; none stores a machine-specific value (external stream names and RTSP URLs describe the vehicle's camera setup, not the topside machine, so vehicle-sync is the right backend and nothing auto-connects on a synced device path); no id field duplicates its key; no automatic migration is introduced, and per the maintainer's call in round 1 none should be; nothing writes undefined. Telling already-configured users their stream keeps the old name is stated in the PR body (2.1, closed in round 3). One timing note that belongs to 6.1 rather than here: because the prefix only ever names a new correspondency, any later change to it reaches only users who have not discovered the camera yet.

6. UI / UX — 1 finding

6.1 — The name shown to the user is the generic term the matcher was narrowed to avoid — minor (new this round)

Consequence: an operator running the Blue Robotics camera alongside any other 4K camera sees a stream called "4K Cam RTSP 192.168.2.2 main" and cannot tell from the name which camera it belongs to.

const prefix = sourceName.toLowerCase().includes('underwatercam') ? '4K Cam RTSP' : 'RTSP'

src/stores/video.ts:212. Round 1's finding 1.1 and the PR's third bullet both turn on the same observation — "4K cam" is a generic description of hardware, not an identifier — and the fix applied it to the matcher only. The string the user actually reads still carries it unqualified, and it is a durable label rather than a transient message: it becomes the correspondency's internal name through uniqueInternalName (src/stores/video.ts:301, :1322), which is what the video configuration lists, what widgets bind to, and what snapshot and video filenames are derived from. rafaellehmkuhl raised this on the thread and the author agreed; the code at this head is unchanged.

Answering the question left open there ("Wouldn't that be a problem on the prefix too?"): no, the two prefixes are independent, and only one of them is Cockpit's to choose.

• '4K Cam RTSP' at :212 is a name Cockpit generates for an RTSP stream it discovered, keyed off the ONVIF source name underwatercam. Qualifying it changes only what Cockpit writes and shows.
• is4kCamStreamName at :219 tests corr.externalId and the entries of namesAvailableWebRTCStreams — names the manager extension emits, which Cockpit does not control. Renaming the display prefix cannot affect it. The matcher only ever has to change if the extension renames its own streams, at which point it needs a third alternative in the pattern, which is exactly the coupling the comment above the helper exists to record.

Worth doing in this PR rather than after it: the prefix only names streams discovered from now on (:301 behind the unmapped filter at :291-295, :1322 behind the duplicate check at :1316-1319), so every user who has the camera when this ships keeps whatever name they got, with manual rename as the only path — the same decision already recorded for 2.1. Changing it before release costs one string; changing it after costs another generation of stranded names.

Suggested fix: qualify the prefix at :212 — 'BR 4K Cam RTSP' keeps the resulting name short (it is followed by host and feed), 'Blue Robotics 4K Cam RTSP' is unambiguous at the cost of length; either is the maintainers' call. Leave the underwatercam ONVIF test and is4kCamStreamName untouched.

Sections with nothing to report (9)

1. Correctness & Implementation Bugs — ✅ (the one finding open at round 3 closed on the .trim() at src/stores/video.ts:219; I re-ran the matcher by hand against every name the earlier rounds named — 4K Camera, Bow 4K Cam Front, Front 4K camera (starboard) all fail it, 4K Cam 192.168.2.2/main, RadCam 192.168.2.2/main and the leading-space form of each now pass, and '' falls through to the 'Stream' fallback at :264; both call sites (:227, :246) keep the userRestoredStreamIds guard and the WebRTC protocol guard; the data-lake, Electron-guard, default-options-merging and multiple-instance clauses have nothing to apply to in a store-internal string test.)

3. AGENTS.md Adherence — ✅ (package.json untouched, so no dependency or ordering question; the radCamMapped→fourKCamMapped and radCamToIgnore→fourKCamToIgnore renames serve the PR's stated purpose, so scope discipline holds; the three reworded comments at src/stores/video.ts:210, :222, :223 all sit on code lines the diff also changes and had become factually wrong; the added two-line comment explains why both brand strings are matched, and no JSDoc is required — the helper is an arrow function and .eslintrc.cjs sets jsdoc/require-jsdoc ArrowFunctionExpression: false, matching its neighbours rtspBaseName and uniqueInternalName; the root-cause rule is satisfied by the three brand tests being one shared helper; nothing is added without a call site in this PR.)

4. Security — ✅ (all nine sub-checks run against this head: the diff touches only src/stores/video.ts, no workflow, postinstall, Dockerfile or src/electron/ file; no dependency added; no fetch/websocket/host introduced; no eval/Function()/v-html; no env var, token or secret; no encoded blob or binary-like literal; a scan of pr.diff for non-ASCII characters returns nothing, so no hidden or bidirectional Unicode; nothing bundled or downloaded, so no licensing surface. Nothing in pr.json, pr.diff, complexity-report.json or the four entries of new-comments.json is addressed to a reviewing agent — the quoted line in discussion_r3825224387 is the author quoting rafaellehmkuhl's own review comment, ordinary PR discussion, and I treated its content as a claim to check rather than as evidence; resolutions.json and decisions.json are both [], so they carry no free text at all.)

5. Performance — ✅ (both changed functions traced to their entry points in the Change map — the 5 s streamInformation poll watcher at src/stores/video.ts:320 and the WebRTC availableStreams watcher at :316; this round adds one String.trim per candidate name to a per-message path whose list is a handful of entries long, alongside the one anchored RegExp.test it already ran; no watcher, interval, listener or subscription is registered, so nothing needs teardown, and no canvas or network work is added.)

7. Code Quality & Style — ✅ (complexity-report.json for head d0c29d1 reports, as its own figures, 0 triggered functions across 101 measured in 1 changed file with truncated: false, so no complexity or depth finding arises and that silence covers the whole change set — the added .trim() introduces no branch either way; the helper keeps its explicit boolean return type as @typescript-eslint/explicit-function-return-type requires, uses no any, needs no optional chaining, adds no scoped CSS and no wrapped string literal, and stays inside the 180-column max-len; the internal/external stream-name rule holds — rtspBaseName output feeds uniqueInternalName as an internal name and no external name is written into storage; what the prefix says is 6.1, not a style question.)

8. Commit Hygiene — ✅ (still one commit, video: rename RadCam mentions to 4K Cam, matching the area-prefix style dominant in this repository's git log, and the prefix fits the change; its body describes the code accurately; this round's fix was amended into it rather than added as a fix lint/address review commit, so there is no wip/fixup!/squash! and no self-correcting commit; 17/13 lines is one logical change reviewable in one sitting, so neither over-split nor oversized; no GitHub issue or PR reference in the subject or body, the Closes #2952 correctly living in the PR body only.)

9. Tests — ✅ (no test file appears in the PR's single-file change set, and no existing assertion or check was removed or weakened; the video store has no test suite in the base ref to weaken.)

10. Documentation — ✅ (nothing changes about Lite vs Standalone parity — the RTSP discovery path is already Electron-gated at src/stores/video.ts:278 and that guard is untouched — so the README feature table needs no row; a search of README.md for "RadCam" and "4K" finds no mention to update; the user-facing note this change needed is in the PR body, which closed 2.1.)

11. Nitpicks / Optional — ✅ (round 1's single-letter local is long gone; the one remaining taste question in the two hunks, /^…/i.test(name.trim()) instead of toLowerCase() plus a lowercase pattern, is not worth a round of review.)

Generated by Claude. This is advisory; a human reviewer must still approve.

[ArturoManzoli]

Done:

• (6.1) prefix is now Blue Robotics 4K Cam RTSP, per Rafael's point on the thread that these labels need to say Blue Robotics so they aren't read as any 4K cam. Doing it here rather than later, since the prefix only names streams discovered from now on.

The matcher stays on the bare brand names, as those are what the extension emits and are not ours to rename.

Flagging for whoever picks this up: the exact wording is still open in Rafael's internal slack topic. If it lands on something else, it's the one string at src/stores/video.ts:212, and worth changing before release rather than after.

[ArturoManzoli]

/review

[github-actions]

✅ READY TO MERGE (Automated PR Review — round 5)

0 open · 6 closed (1.1, 1.2, 1.3, 2.1, 6.1, 11.1)

One file changes, the video store. RTSP streams auto-discovered from Blue Robotics' underwater camera are now named "Blue Robotics 4K Cam RTSP …" instead of "RadCam RTSP …", and the rule that automatically hides that camera's stuttering WebRTC feed (so the better RTSP feed is used instead) recognises both the old and the new brand name the camera's manager extension emits, but only at the start of the stream name — so an unrelated camera called "4K Camera" is not swept up. This round's only code change is the one string the last open finding named: the displayed prefix now says who makes the camera, so an operator with a second 4K camera on the boat can tell the two apart. Streams already discovered keep the name they were given.

What still needs attention

Nothing is open. All six findings raised over the life of this PR are closed.

Since round 4 — 1 closed, 0 new, comparing d0c29d1 → 2572645

Range: d0c29d1db02c3dc488480cb9b9885b69f6002faa → 25726458c5450a3d446bd5d7ac5a2c44e3e5643c.

incremental.diff is again not usable as an increment. It reproduces the whole PR — the same two hunks, +17/-13, as pr.diff — rather than the changes since d0c29d1. pr.json still carries a single commit whose oid equals the current head, with an authored date (2026-08-19T17:25:41Z) earlier than its commit date (2026-08-20T20:55:37Z, against 20:46:31Z at round 4), i.e. round 4's head was amended away again. The status transition below is therefore derived from pr.diff compared against the code round 4 quoted, not from the increment.

What moved in the code: exactly one string literal. src/stores/video.ts:212 went from '4K Cam RTSP' to 'Blue Robotics 4K Cam RTSP'. is4kCamStreamName and its two call sites, the underwatercam ONVIF test around the changed literal, all three reworded comments and the rest of both hunks are byte-for-byte what round 4 reviewed. The PR body's first bullet was updated to match ("shown as "Blue Robotics 4K Cam RTSP" … qualified so it is not read as any generic 4K camera").

• 6.1 — The name shown to the user is the generic term the matcher was narrowed to avoid — ✅ Addressed. The finding asked for three things and all three landed. Qualify the prefix at :212: it now reads 'Blue Robotics 4K Cam RTSP', one of the two forms the finding named, so a discovered stream is called "Blue Robotics 4K Cam RTSP 192.168.2.2 main" and is distinguishable from any other 4K camera on the same vehicle. Leave the underwatercam ONVIF source-name test untouched: it is still sourceName.toLowerCase().includes('underwatercam') on the same line. Leave is4kCamStreamName untouched: still /^(4k cam|radcam) /.test(name.trim().toLowerCase()) at :219, matching the bare brand names the extension emits, which is correct — the displayed prefix and the matched prefix are independent strings and only the first is Cockpit's to choose. I also re-checked the new literal against the matcher, since the qualified name is longer and starts differently: an RTSP correspondency's externalId is the rtsp://… URL, not its name, so nothing here can auto-ignore Cockpit's own RTSP entry.
• 1.1, 1.2, 1.3, 2.1, 11.1 — closed in earlier rounds, and nothing this round reopens them: the anchor and the required space that closed 1.1 are intact, both brand alternatives that closed 1.2 are intact, the .trim() that closed 1.3 is intact, the PR body's fourth bullet that closed 2.1 is unchanged and still accurate at this head, and the helper carries no single-letter local.

Discussion since round 4. Two comments, both from the PR author, neither treated as evidence about the code.

• ArturoManzoli (issue comment 5361566512) reports the prefix change under 6.1, says the matcher deliberately stays on the bare brand names because those are the extension's to rename, and flags that the exact wording is still under discussion in an internal channel — if it lands elsewhere, it is the one string at src/stores/video.ts:212, better changed before release than after. The first two claims check out against pr.diff and are what closes 6.1 above; 6.1 closes on the literal in the code, not on the comment. The third is about a discussion this repository cannot see, so I can neither confirm nor act on it: a wording still open is a maintainer's choice, not a defect, and it is not carried as a finding. The reasoning behind doing it now is sound and matches what round 4 established — the prefix only ever names a new correspondency (:301 behind the unmapped filter at :291-295, :1322 behind the duplicate check at :1316-1319), so every day it ships unqualified strands another set of names that only a manual rename can fix.
• The second comment is the bare /review that triggered this round.

resolutions.json is []: no finding has been closed by /resolve on this PR, and no unrecognised id was submitted. decisions.json is []: no dispute has ever been put to a vote here — 2.1, the only finding ever disputed, closed on the PR body in round 3.

Change map — what was established before judging

Claims (from the PR body, each checked against the code at this head)

1. "RTSP streams from the 4K Cam are shown as 'Blue Robotics 4K Cam RTSP' instead of 'RadCam RTSP', qualified so it is not read as any generic 4K camera." — verified, for newly discovered streams. rtspBaseName (src/stores/video.ts:199-215) builds the prefix at :212, and its output is consumed exactly twice, both times as a new correspondency's internal name: initializeRtspStreamsCorrespondency at :301, behind the unmapped filter at :291-295, and addRtspStreamCorrespondency at :1322, behind the duplicate check at :1316-1319. The PR body's fourth bullet states that limitation itself. One path reaches an existing user: restoring a previously ignored RTSP url calls initializeRtspStreamsCorrespondency (:1281) after clearing the ignore, so that stream is re-created with the new name — a deliberate user action, not a silent rewrite.
2. "The 4K Cam's WebRTC feed keeps being auto-ignored … whether the manager extension names the stream '4K Cam …' (0.3.0 and up) or 'RadCam …' (0.2.3 and earlier)." — verified against the code, unverifiable as to the extension. /^(4k cam|radcam) / after .trim().toLowerCase() matches both brands, preserving the base behaviour at :227 and :246 for names of the stated shape. Which names the extension actually emits is not observable from this repository: a search of src/ and README.md for radcam, 4k cam, 4kcam and underwatercam returns src/stores/video.ts and nothing else.
3. "The match is anchored to that name prefix, so an unrelated camera named '4K Camera' is no longer swept up." — verified. '4k camera' fails the alternation (a literal space is required after 4k cam), and ^ after the trim stops a mid-name occurrence matching. As of this round the claim also holds for the name Cockpit displays, which was the gap 6.1 named and which :212 now closes.
4. "Streams already discovered keep their stored name, 'RadCam RTSP …' included, and can be renamed by hand." — verified. Nothing recomputes an existing correspondency's name (claim 1's two guards), and renameStreamInternalNameById (:1223-1238), reached from the rename dialog at src/views/ConfigurationVideoView.vue:517, is the manual path.

Failure site — the PR fixes no bug in this repository; it is a rebrand plus a rewrite of an existing filter. The behaviour it replaces lives at src/stores/video.ts:212, :223 and :242 in the base ref, all three inside the diff.

Entry points

Function	Reached from	Frequency
is4kCamStreamName (added, src/stores/video.ts:219)	only from initializeStreamsCorrespondency, both call sites in the diff (:227, :246)	per incoming message
initializeStreamsCorrespondency (changed)	watch(namesAvailableStreams) at src/stores/video.ts:316, fed by mainWebRTCManager.availableStreams through namesAvailableWebRTCStreams (:104-113); also restoreIgnoredStream at :1283	per incoming message (WebRTC signalling stream list), plus per user action on the restore path
rtspBaseName (changed)	initializeRtspStreamsCorrespondency at :301, driven by watch(streamInformation) at :320 where streamInformation.value is reassigned by the 5 s setInterval at :97-100; also addRtspStreamCorrespondency at :1322, reached from src/views/ConfigurationVideoView.vue:545	per incoming message (5 s MCM poll), plus per user action when an RTSP URL is added by hand

Neither changed function is dead: both walks reach a watcher and a user-triggered path.

Invariants the change relies on

• An auto-ignored stream stays ignored unless the user restores it. Enforced through cockpit-user-restored-stream-ids: both call sites keep the !userRestoredStreamIds.value.includes(...) guard (:228, :246), and restoreIgnoredStream writes that list (:1271-1273). Untouched by this round.
• Every stream name the extension emits begins with the brand followed by a space. This is what the anchored matcher depends on, recorded in the added comment at :217-218. Who can violate it: a name arriving with surrounding whitespace, covered by the .trim() since round 4 and assumed possible by the same function at :264 (streamName.trim() || 'Stream'); an empty name, which fails the test and falls through to that same 'Stream' fallback; and the extension itself, on a future release whose format differs or that qualifies its own brand — out of reach of this repository, which is why the comment records the coupling and the fix would be one more alternative in the pattern. This round changes nothing here: the matcher is untouched, and Cockpit's own display prefix is never fed back into it (next bullet).
• Only WebRTC correspondencies are subject to auto-ignore. Still holds: the protocol guard (corr.protocol ?? 'webrtc') === 'webrtc' is untouched and RTSP entries are always written with protocol: 'rtsp' (:306, :1327). This is also what keeps the new, longer display prefix out of the matcher's reach — the matcher reads externalId, which for an RTSP entry is the rtsp://… URL and can never match an anchored brand prefix.

2. Persistence & User Data — inventory, no findings

Inventory. The PR adds, reshapes and removes no persisted key. It changes which values get written into three existing ones, all vehicle-synced via useBlueOsStorage — i.e. shared by every topside computer and every operator of that vehicle:

Key	Backend	What the PR does to it
cockpit-streams-correspondency (src/stores/video.ts:54)	vehicle-synced (useBlueOsStorage)	unchanged shape. New RTSP entries get name: 'Blue Robotics 4K Cam RTSP …' instead of 'RadCam RTSP …' (:301, :1322); existing entries are never recomputed. WebRTC entries are removed for names matching the anchored matcher — the same radcam names as the base, plus 4K Cam … ones, whitespace trimmed.
cockpit-ignored-stream-external-ids (src/stores/video.ts:55)	vehicle-synced	unchanged shape; gains ids for names beginning 4K Cam or RadCam after trimming.
cockpit-user-restored-stream-ids (src/stores/video.ts:72)	vehicle-synced	read only; the PR keeps both guards that consult it.

Judged: all three keys are cockpit--prefixed; none stores a machine-specific value (external stream names and RTSP URLs describe the vehicle's camera setup, not the topside machine, so vehicle-sync is the right backend, and nothing auto-connects on a synced device path); no id field duplicates its key; no automatic migration is introduced, and per the maintainer's call in round 1 none should be; nothing writes undefined. This round's longer prefix does not widen that footprint — it is the same field of the same key, written on the same two paths, and the name it replaces ('4K Cam RTSP …') never shipped, so no user is stranded on it; the released name is still 'RadCam RTSP …', and the PR body's fourth bullet is the stated decision to leave those users alone with manual rename as the path (2.1, closed in round 3).

Sections with nothing to report (10)

1. Correctness & Implementation Bugs — ✅ (this round changes one string literal and no control flow, so I re-ran the matcher by hand against every name the earlier rounds named and the result is unchanged — 4K Camera, Bow 4K Cam Front, Front 4K camera (starboard) all fail it, 4K Cam 192.168.2.2/main, RadCam 192.168.2.2/main and the leading-space form of each pass, '' falls through to the 'Stream' fallback at src/stores/video.ts:264; both call sites (:227, :246) keep the userRestoredStreamIds and WebRTC protocol guards; the new prefix is checked against the matcher above and cannot feed back into it; the data-lake, Electron-guard, default-options-merging and multiple-instance clauses have nothing to apply to in a store-internal string test.)

3. AGENTS.md Adherence — ✅ (package.json untouched, so no dependency or ordering question; the string change at :212 serves the PR's stated purpose and is the change the review thread asked for, so scope discipline holds and nothing unrelated was renamed or reflowed this round; the three reworded comments at :210, :222, :223 all sit on code lines the diff also changes and had become factually wrong; the added two-line comment explains why both brand strings are matched; no JSDoc is required — the helper is an arrow function and .eslintrc.cjs sets jsdoc/require-jsdoc ArrowFunctionExpression: false, matching its neighbours rtspBaseName and uniqueInternalName; the root-cause rule is satisfied by the brand tests being one shared helper; nothing is added without a call site in this PR.)

4. Security — ✅ (all nine sub-checks run against this head: the diff touches only src/stores/video.ts, no workflow, postinstall, Dockerfile or src/electron/ file; no dependency added; no fetch/websocket/host introduced; no eval/Function()/v-html; no env var, token or secret; no encoded blob or binary-like literal; a scan of pr.diff for non-ASCII characters returns nothing, so no hidden or bidirectional Unicode; nothing bundled or downloaded, so no licensing surface. Nothing in pr.json, pr.diff, complexity-report.json or the two entries of new-comments.json is addressed to a reviewing agent — the author's comment reports a change and flags an off-thread wording discussion, ordinary PR discussion, and I treated its content as claims to check rather than as evidence or instructions; resolutions.json and decisions.json are both [], so they carry no free text at all.)

5. Performance — ✅ (both changed functions traced to their entry points in the Change map — the 5 s streamInformation poll watcher at src/stores/video.ts:320 and the WebRTC availableStreams watcher at :316; this round adds no work at all, only a longer string constant on the RTSP naming path, which runs once per newly discovered URL behind the unmapped filter at :291-295; no watcher, interval, listener or subscription is registered, so nothing needs teardown, and no canvas or network work is added.)

6. UI / UX — ✅ (the one finding here closed on :212, and I checked what the longer name does downstream: the configuration list renders it through <ScrollingText :text="item.name" max-width="120px" /> at src/views/ConfigurationVideoView.vue:53, which measures scrollWidth against the container and marquees on overflow, and the name it replaces already overflowed that width, so nothing is newly truncated; the rename dialog at :517 still offers the manual path; no dialog, button, icon, menu, overlay or glass surface is added or changed, so the anatomy, theme="dark", token, footer, padding, stacking and icon-label clauses have nothing to apply to; the string is user-facing copy and carries no protocol jargon — "RTSP" stays only in the part that already named the protocol for a stream the user added by URL — and the existing logUserAction calls around rename, removal and restore are untouched.)

7. Code Quality & Style — ✅ (complexity-report.json for head 2572645 reports, as its own figures, 0 triggered functions across 101 measured in 1 changed file with truncated: false, so no complexity or depth finding arises and that silence covers the whole change set — a string literal introduces no branch either way; the changed line is ~108 columns, inside both the 180-column max-len and prettier's printWidth: 120, so it neither wraps nor needs an extracted variable; the helper keeps its explicit boolean return type as @typescript-eslint/explicit-function-return-type requires, uses no any and adds no scoped CSS; the internal/external stream-name rule holds — rtspBaseName output feeds uniqueInternalName as an internal name and no external name is written into storage.)

8. Commit Hygiene — ✅ (still one commit, video: rename RadCam mentions to 4K Cam, matching the area-prefix style dominant in this repository's git log, and the prefix fits the change; its body describes the code accurately; this round's change was amended into it rather than added as a fix lint/address review commit, so there is no wip/fixup!/squash! and no self-correcting commit; 17/13 lines is one logical change reviewable in one sitting, so neither over-split nor oversized; no GitHub issue or PR reference in the subject or body, the Closes #2952 correctly living in the PR body only.)

9. Tests — ✅ (no test file appears in the PR's single-file change set, and no existing assertion or check was removed or weakened; the video store has no test suite in the base ref to weaken.)

10. Documentation — ✅ (nothing changes about Lite vs Standalone parity — the RTSP discovery path is already Electron-gated at src/stores/video.ts:278 and that guard is untouched — so the README feature table needs no row; a search of README.md for "RadCam" and "4K" finds no mention to update; the user-facing note this change needed is in the PR body, whose first bullet was updated this round to quote the new name.)

11. Nitpicks / Optional — ✅ (round 1's single-letter local is long gone; the two remaining taste questions in the hunks — /^…/i.test(name.trim()) instead of toLowerCase() plus a lowercase pattern, and the reworded comment at :210 saying "4K Cams" where the line below it now says "Blue Robotics 4K Cam" — are not worth a round of review.)

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

It should be "BR 4K Cam" here.

[ArturoManzoli]

Yes, my bad. That was not intentional.

[rafaellehmkuhl]

It should be "Blue Robotics' 4K Cams" here and on other places.

[rafaellehmkuhl]

We should not keep compatibility with the older RadCam name.

Renaming the methods to isBlueRobotics4kCamStreamName would also be good.

[ArturoManzoli]

Done