Shared switch ports (KAN-199) and overrides generate (KAN-120) - #30
Merged
Conversation
Several wired clients reporting the same switch and port usually means an unmanaged switch or a virtualisation host the controller cannot see, and needs no cooperation from the hidden device to detect, unlike LLDP. Topology.shared_ports() is the one source of truth, restricted to direct CLIENT_UPLINK reports and computed against the final topology so a [[hosted]] override that reparents a client already resolves the flag. Surfaced four ways from that one method: a `*` on the port label plus a legend note in DOT/SVG/PDF/PNG, the label marker alone in draw.io (no legend there), a named SHARED SWITCH PORTS section in --report, and a console warning from cmd_render that is obfuscation-aware like _report_displacements. Never drawn as a synthetic node: which of the two causes it is can't be told apart from here, so no node is invented either way. The has_unknown_switch controller flag stays unread; it's a separate, unverified signal and TODO.md/CLAUDE.md now say so explicitly rather than letting it read as done alongside this. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The `*` port-label marker from the KAN-199 shared-port change rode on Style.show_port_labels, which --layout unifi (the default) turns off entirely because ortho routing can't place edge labels without them drifting onto unrelated nodes. So on the render most people actually produce, the marker and its legend row were both gone: detection, --report and the console warning all still worked, but the diagram itself showed nothing. Give the edge its own layout-independent channel instead, the same way TOPOLOGY_GRAPH's arrowhead=odot already is: arrowhead=diamond, applied unconditionally rather than gated on show_port_labels. The two markers never collide, since shared_ports() only ever counts direct CLIENT_UPLINK edges. The legend row stays gated on show_legend, matching every other marker's treatment in --layout unifi. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-120) Prints a candidate overrides.toml to stdout, seeded from the same three signals --report already names: clients with no reported uplink ([[link]], with `to` always left blank), switch ports shared by several wired clients ([[device]] + [[hosted]], KAN-199), and artwork matches refused as ambiguous ([[node]]). Every block is commented out, so the file is a no-op until a human edits and uncommments it; the one thing it never fills in is a client's real uplink. A new `generate` action on the existing `overrides` subcommand rather than a flag on render: this produces a config skeleton, not a diagram, and being its own subcommand makes it opt-in for free rather than something a render flag could leave on by habit. Artwork ambiguity is resolved offline and best-effort, the same way `unifi-map shape` already does, so it never touches the network. --report now points at `overrides generate` from each of the three sections it can help with. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CHANGELOG.md had no Unreleased entry for either. docs/usage.md's "Reading the diagram" table was missing the new diamond-arrowhead row -- the same gap the code itself had until the previous commit -- and its --report summary sentence didn't mention the new SHARED SWITCH PORTS section. Neither usage.md's command list nor overrides.md mentioned `overrides generate` at all; overrides.md gets its own "Generating a starting point" section, sibling to the existing one for `check`. TODO.md's KAN-199 entry still described the label-only marker from before the arrowhead fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
overrides.generate_candidates() interpolated node/edge labels and ids straight into the skeleton TOML and into the `#` comment lines around them. A switch named Core "A" produced name = "Unmanaged switch (port 7 on Core "A")" -- invalid TOML the moment the block was uncommented. Worse, a raw newline in a label would have ended the `#` line it sat on and let whatever followed be read as real TOML, unreviewed: a genuine break of "inert until edited", the one guarantee this command makes, not just a cosmetic one. Two helpers fix both hazards: _comment_safe() collapses whitespace so nothing can end the line it sits on (same technique already used by render_mermaid._flatten()), and _toml_value() additionally escapes `\` and `"` for values placed inside quotes. Applied everywhere a label or an id reaches the output, not just the field the repro happened to hit -- _norm_mac() never validates a MAC's shape, and this project's own threat model treats all of it as attacker-controlled. Six tests pin it, including one that parses the entire generated file with tomllib and asserts it comes back empty, which is what a newline actually escaping its comment would have broken. Mutation-tested: reverted, confirmed all six fail red, restored. Found by external review of cfcd2fe. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Five commits, building on each other:
Topology.shared_ports(): a marker on the diagram, a namedSHARED SWITCH PORTSsection in--report, a console warning (obfuscation-aware), and never as a synthetic node — which of the two causes it is can't be told apart from here, so nothing is invented either way.*port-label marker rode onStyle.show_port_labels, which--layout unifi(the default) turns off entirely. Gave the edge its own layout-independent channel (arrowhead=diamond, unconditional, the same approachTOPOLOGY_GRAPH'sarrowhead=odotalready used) so the signal survives the render most people actually produce.unifi-map overrides generate(KAN-120). Prints a commentedoverrides.tomlskeleton to stdout, seeded from the same three signals--reportnames: unplaced clients, shared switch ports, and ambiguous artwork matches. Every block is commented out, so the file is a no-op until edited; the one thing it never fills in is a client's real uplink.CHANGELOG.mdUnreleased entries,docs/usage.md's diagram-reading table and--reportwalkthrough, a new "Generating a starting point" section indocs/overrides.md, andTODO.md/CLAUDE.mdreconciled against what actually shipped."produced invalid TOML the moment a block was uncommented, and a label containing a newline would have broken out of its#comment entirely — a real break of "inert until edited", not a cosmetic one. Fixed with two small encoders, applied to every label and id reaching the output.Checklist
make checkpasses (ruff format --check,ruff check,pytest), and Ichecked its exit code rather than eyeballing piped output
or device addresses
cache/orout/contents committedit accurately (
docs/usage.mdfor layouts and reading the diagram,docs/output.mdfor formats)Anything you are unsure about
KAN-199's other signal — reading the controller's own
has_unknown_switchboolean from the v2 topology payload — is still unbuilt and still unverified (per-site behaviour, whether it clears, whether an all-UniFi network ever sets it). Documented as open inTODO.md/CLAUDE.mdrather than attempted here, since it would be guessing.Every guard in this PR was mutation-tested: reverted, confirmed the new/changed tests fail red, restored.