fix: apply pass 1 review verdicts to schema library v2 - #80
Draft
lancamat1 wants to merge 56 commits into
Draft
Conversation
…ces keep an MTU Packet: DCIM-06 (reject, redesign), keeps DCIM-07. Moving mtu to InterfacePhysical left InterfaceLag and InterfaceVirtual with no MTU at all, which is where a non-default MTU is most often needed (bundles, SVIs, tunnels, loopbacks). The fix for a bad default (1514 was frame size) is the default, so mtu returns to the DcimInterface generic with default_value 1500. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ufacturer Packet: ORG-N1 (change). CC-2 wants plural names on cardinality-many relationships, and both declarations are PR-touched. The rename also removes the clash with DcimGenericDevice.device_type and .platform, which are cardinality one and mean something else. Identifiers stay singular on the object part, so the forward sides in base/dcim.yml are untouched. Checklist: CC-2 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet: LOC-02 (change). The human_friendly_id was silently deriving the uniqueness constraint, which CC-4 forbids; unique: true states what Infrahub already enforces, so there is no behaviour change and no migration. optional and order_weight are written on the same three lines, and display_label plus order_by give the generic parity with LocationGeneric (it is a relationship peer in seven places and had a null display label). Checklist: CC-4, CC-6, CC-9 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…order Packet: IPAM-05 (change, substance accepted). The mandatory plus default_value plus base palette work is exactly what CC-5 and CC-6 ask for; the one defect is that this node listed reserved after deprecated while DcimDevice, GenericVLAN, LocationRack and IpamIPAddress all put reserved first (4:1). Two nodes in the same file ordered the same dropdown differently. Checklist: CC-5 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet: IPAM-04 (color half only, the rest is a question to the author). backbone and management both used #AEC6CF, so the two roles were indistinguishable in the UI and the color carried no information. #9678B6 is freed by dropping the v1 technical role. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packets: AGG-01, VLAN-01 (both change). CC-4 forbids relying on the constraint Infrahub derives from the human_friendly_id. IpamAggregate resolved to a derived [[prefix__value]] (namespace-less is correct here) and IpamVLANGroup.name to a derived [[name__value]]; both are now stated in the file. Checklist: CC-4 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packets: AGG-02, PSU-03, DCIM-CC2 (all change; DCIM-CC2 confirmed by Matej as a rename). CC-2 says Booleans are bare snake_case with no is_ prefix (is_private -> private, 7:1 in the tree) and no run-together words (hotswappable -> hot_swappable, named by the checklist). DCIM-CC2 follows the CC-2 majority count, serial_number 3 against serial 2, so base DcimPhysicalDevice and DcimGenericSFP move to serial_number. The matching object files are renamed in the same commit so the data still loads. Checklist: CC-2 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packets: IPAM-07 (change), base_ipam consistency note 3, ruling R5 (IpamCVLAN part deferred to the QinQ scoping change). The namespace-first human_friendly_id flip is accepted, but the declared constraints still read value-first, so Infrahub no longer recognises them as the same constraint and adds its derived one on top: the live schema carried two functionally identical constraints on IpamPrefix, IpamIPAddress and DcimCrossConnect. Reordering restores one explicit constraint and no implicit one. Checklist: CC-4 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…unique label Packet: CABLE-02 (change). order_by and display_label both point at label, which was optional and non-unique, so unlabelled cables rendered an empty display label and sorted arbitrarily. Making label the node's declared identity fixes both and covers CC-4's HFID-less-node list. Checklist: CC-4 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet: CLU-01 (change). ClusterGeneric.location is cardinality many with a singular name, and CC-2 makes plural the rule. No identifier is declared on either side and the reverse LocationGeneric.clusters is named independently, so the rename does not have to be mirrored. Checklist: CC-2 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e ComputeGenericUnit
Packets: COMP-03, HOST-03 (both escalated, decision Q6-B).
CC-3 says menu placement follows the node's own domain: a Compute server filed under
DcimDevice ("Network Device") and a Cluster filed under VirtualizationVirtualMachine both
broke that, the latter also inverting containment. ComputeGenericUnit becomes the visible
anchor, which is the library's own idiom and yields a real "all compute units" list.
CLU-01 stands as the author wrote it: ClusterGeneric stays hidden.
Checklist: CC-3, CC-9
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tte convention Packet: XCON-CC6 (change, no packet owns the file). CC-6 names cross_connect as one of the status deviants: status is mandatory with a default_value everywhere else. planned is the safe default, since a cross-connect has to be ordered from a carrier before it is connected. The three colors were still the v1 palette. Checklist: CC-5, CC-6 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet: MOD-04 (change).
Text is the wrong kind: the description documented NetBox's {module} token substitution,
which nothing here implements, the sibling DcimPhysicalDevice.position in the same
namespace is a Number, and the author's own first version (98a730f) was Number with
min_value 1. No uniqueness constraint restored: position is optional.
Checklist: CC-2
Note: objects/extensions/device_module/device_module.yml still quotes the three values;
unquoting them is the paired objects-file fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nja2 template Packet: MOD-06 (change). The generic declared computed_name as read_only + mandatory with no computed_attribute, so it could be neither set by a client nor computed by the server, and any node inheriting the generic without overriding it could not be created. The generic can compute from module_bay, which it owns; concrete kinds keep their own override. The stale "Overriten at node level" comment goes with it. Checklist: CC-4, CC-8 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet: PSU-02 (change).
CC-2 requires units in parentheses in the label, which base does uniformly ("Height (U)",
"Weight (kg)"), and wattage carried no label at all. Commit 8eae840 in this PR set the
min_value precedent; without it a wattage accepts 0 and negatives.
Checklist: CC-2
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… stale 4096-char FIXME Packets: CONN-01 (change), CONN-02 (ruling R6). The FIXME's premise is false on 1.11.0: the full 28-choice list loads at roughly 3.3 KB against a 4096-char limit. CONN-02 keeps the two lists duplicated, so each now carries a sync comment pointing at the other; a shared generic in a common ancestor is the real deduplication and is a base-tier change for a separate release. The three restored choice names keep the surrounding UPPERCASE form; the case sweep owns all 28 values at once. Checklist: CC-5, CC-8 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…re-type enum in the domain Packet: DWDM-03 (change). The flip to optional was unstated and left one identical 25-choice list with two optionalities inside one domain. DcimGenericSFP.sfp_type, form_factor and DcimGenericPatchPanelInterface.connector_type are all mandatory; CC-6's "no sensible default" exception covers a mandatory connector type, and CONN-01 removed the one argument for optional by restoring the missing choices. Checklist: CC-6 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…precated vocabulary Packets: LMIN-03, RACK-02 (both change). CC-2 resolves split vocabulary by majority. Counted across base/ and extensions/, the choice name deprecated wins 5:2 and the label Deprecated 4:3, so both Sites rename. The Rack already used name: deprecated with label: Retired, exactly the name/label split CC-2 exists to close. Checklist: CC-2 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntifiers Packet: PP-01 (identifier half; the dependency and description halves belong to the metadata commit). CC-7 wants identifiers in subject__object form. DWDM-01 fixed the dwdm half of the v1 collision and left these two bare; v2.0 is the last release that may rename a stored relationship identifier. Checklist: CC-7 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ent/Component Packet: RACK-01 (change). Two defects rode in on the split. CC-4: name was globally unique, so two sites could not each have a rack called R01, and rack names repeat across sites in every real estate; identity is now scoped by site with the uniqueness declared explicitly. CC-7: LocationRack.site was Attribute while its reverse LocationSite.racks is Component, and Parent is what pairs with Component. Deletion behaviour is unchanged, the site side stays Component and site stays mandatory. Checklist: CC-4, CC-7 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet: RACK-04 (change). CC-2's explicit call: facility beats facility_id 2:1, and the rack either aligns or documents that it is a different concept. The attribute carries no description at all, so nothing in the file distinguishes a rack's local identifier from a site's. The objects file moves with it or the load breaks. Checklist: CC-2 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet: SNMP-03 (change). Adding the link is right, the peer was not: concrete DcimDevice silently excludes ComputePhysicalServer, VirtualizationVirtualMachine and DcimPatchPanel, and servers and VMs are the most SNMP-polled things in an estate. TEN-02 in this same diff peers the base generic, and DcimGenericDevice is base, so the change costs no dependency edge. CC-4's explicit uniqueness behind the HFID name folds in here. Checklist: CC-1, CC-4, CC-7 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet: SNMP-04 (change). CC-2 makes plural the rule on cardinality-many relationships and routes untouched singulars to their own packet; SnmpClient had two, the new ip_address and the pre-existing community. CC-4 lists SnmpClient among the nodes whose HFID name is not explicitly unique. community also had no kind, declared here as Attribute rather than left to the kind sweep. Checklist: CC-2, CC-4 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ce it under Organization Packets: TEN-01, TEN-04 (both change). CC-3 names this node explicitly: a single-node Tenancy namespace whose only node inherits OrganizationGeneric violates one-namespace-per-domain, and the folder keeps the feature name. The missing label and menu_placement ride along, matching the two namespace siblings OrganizationManufacturer and OrganizationProvider. TEN-04: tenant__ipaddress runs two words together where every sibling identifier in the file separates them. Checklist: CC-3, CC-7 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet: IPAM-03 (change). The bare stub in base is forced: base loads standalone and first, so it cannot declare vlan_groups without dangling on IpamVLANGroup. What was wrong is the mechanism the extension used to fill it: re-declaring the whole generic under generics: replaces base's definition, duplicates a kind across two files, and hides who fills the stub from a reader of base. extensions: nodes: - kind: <GenericKind> is the documented and repo-established mechanism, already used on LocationHosting by three extensions in this tree. The stub also gains the label and description CC-2/CC-8 require. Checklist: CC-1, CC-2, CC-8 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ss per kind Packets: VLAN-03, QINQ-02 (both change; decision Q4-A). CC-2 rejects a literal Generic namespace: the namespace is prepended to form the kind and "Generic" is not a domain, so the convention is <Ns>Generic<Thing>, exactly the DcimGenericDevice shape. Separately, [vlan_id__value, vlan_group] on the generic made IpamVLAN, IpamSVLAN and IpamCVLAN share one flat VLAN-ID space per group, so two C-VLANs with the same C-tag under different S-VLANs were rejected at create time. That is the central Q-in-Q case. The constraint moves down to IpamVLAN and IpamSVLAN; IpamCVLAN keeps only its own [vlan_id__value, svlan]. Checklist: CC-2, CC-4 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packets: IPAM-07 cross-reference, base_ipam consistency note 3, ruling R5. Third and last of the duplicate-constraint fixes; IpamPrefix, IpamIPAddress and DcimCrossConnect landed earlier. IpamCVLAN's HFID reads [svlan__name__value, vlan_id__value] while the declared constraint read [vlan_id__value, svlan], so Infrahub no longer recognised them as the same constraint and added its derived one on top. Sequenced after the QinQ scoping change so the reorder applies to the final constraint set. Checklist: CC-4 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet: VRRP-02 (change, admitted under ruling R3). Measured: deleting a single virtual IP address deleted the VRRP group and the group's other virtual IP. The cause is kind: Component on IpamIPAddress.vrrp, which implies on_delete: cascade and declares that an IP address owns a VRRP group. Every other Component relationship in the library points container to children. The group to IPs cascade stays: a VIP exists only for its group. A kind change does not touch the derived identifier, so no data migration. Checklist: CC-7 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…onnection Packet: IXP-02 (change). Live-verified: PeeringIXPConnection.bgp_sessions resolves to Component, which defaults to on_delete: cascade, while RoutingBGPSession.ixp_connection is optional. So deleting an IXP connection silently deleted its BGP sessions with their policies and AS links, even though the schema says a session need not have a connection. This is the only Component pair in the repo whose child side is optional by necessity. kind stays Component for the separate-tab UI; on_delete, not the kind, drives deletion. Checklist: CC-7 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packets: SNMP-03, SNMP-04 (objects half). The SNMP-04 verdict states the objects file populates neither renamed relationship; it does populate community on SnmpClient three times, so the load would break on communities. The header comment also still named DcimDevice and ip_address. Split out rather than folded because both schema commits had already landed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packets: EXP-02 (STRUCT-01) The three IPAM peers name an Infra* namespace that does not exist anywhere in base or extensions, so the schema cannot load. This PR deletes extensions/topology and makes this file the library's only topology model, so the pre-existing breakage is now v2.0's to carry. The missing extensions/vlan dependency edge lands with the merged .metadata.yml dependency commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packets: EXP-03 CircuitEndpoint is not a kind anywhere in the library; the declared dependency extensions/circuit defines DcimCircuitEndpoint. Two tokens stand between this extension and loading, and the declared dependency list is already correct. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packets: EXP-04 Checklist: CC-4 base/location.yml drops shortname from LocationGeneric in this PR and switches the human_friendly_id to name, but LocationRack here still resolves its HFID through shortname__value and defines no shortname of its own, so the schema no longer loads. The break arrives entirely from base, so this PR owns it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ne hop Packets: EXP-05 (STRUCT-05) Re-pointing these three nodes from the removed DeviceGenericModule to DcimGenericModule left the templates traversing module_bay -> device, and Jinja2 computed attributes only resolve direct relationships, so the schema is rejected. Switch to the one-hop idiom the rest of the library uses, module_bay__computed_name__value, and correct the three description examples: the name now keys off device name plus bay, not the device serial number. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packets: lead reviewer ruling, 2026-08-31 (new) Checklist: CC-4 Disk names repeat across devices, so a bare name__value human_friendly_id cannot address a disk; the node already declares the matching [device, name__value] uniqueness constraint. Resolving device__name__value needs a name on the DcimDeviceWithPhysicalDisks generic, so add one with allow_override: any, following the precedent base/dcim.yml sets on DcimPhysicalDevice.name for DcimModuleBay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packets: RTG-CC1, BGPC-CC1, IXP-03, STRUCT-11, STRUCT-12, CIRC-05, PP-01, RACK-05, EXP-02 Checklist: CC-1 Every declared edge is now referenced by the extension's YAML and every kind an extension references is owned inside its transitive closure. Twelve entries change: - extensions/circuit: drop extensions/location_minimal (DcimCircuit.location is gone, only LocationHosting from base remains) (CIRC-05) - extensions/patch_panel: add extensions/device_module, and stop promising the removed DcimPatchPanelModule in the description (PP-01) - extensions/rack: re-point extensions/location_minimal to extensions/location_site, the smallest provider of Location.Site, with the either-package note in the description since a flat list cannot say "one of" (RACK-05, ruling 1) - extensions/routing: drop extensions/vlan, add extensions/vrf (RTG-CC1) - extensions/routing_bgp_community: drop extensions/routing, add extensions/routing_policies (BGPC-CC1) - extensions/routing_bgp_rr: drop extensions/routing (RTG-CC1) - extensions/routing_policies_aggregate: drop extensions/routing and extensions/routing_aggregate (RTG-CC1) - extensions/routing_policies_bgp: drop extensions/routing (RTG-CC1) - extensions/routing_policies_ospf: drop extensions/routing (RTG-CC1) - extensions/routing_policies_pim: drop extensions/routing (RTG-CC1) - extensions/peering_ixp: drop extensions/routing and extensions/routing_policies, both unused and both reached transitively (IXP-03, STRUCT-11) - experimental/topology: add extensions/vlan, now that the VLAN peer resolves to IpamVLAN (EXP-02) routing_policies_aggregate.yml gains one comment line so the commented-out extensions block records that uncommenting it needs the dropped extensions/routing_aggregate edge back (RPOL-CC9 bucket b rider). Resolver check after the edit: 57 nodes ordered, no cycle, no dangling edge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ions and honour it in the loader Packets: STRUCT-09, FR-004b, LSITE-01, EXP-06 (STRUCT-08) Checklist: CC-1, CC-8 Two pairs of packages define the same kind and cannot be loaded together: extensions/location_minimal and extensions/location_site both define Location.Site, and experimental/location_extended and extensions/rack both define Location.Rack. Until now co-loading was silent: the hierarchical Site simply won in either order, so a user who picked location_site for a flat model got the hierarchy with no warning, and the rack pair fails outright with "Hierarchy must be provided if either parent or children is defined". - .metadata.yml gains a machine-readable exclusive_with key on all four entries, declared in both directions, and reciprocal prose in every description, which CC-1 requires on both sides. - location_minimal's description also records the constraint that stays invisible until data is loaded: LocationGeneric.name is unique across every location kind, so a single-country deployment enters the hierarchy at Country rather than duplicating a country under several regions (FR-004b field test; generic-level uniqueness itself stays as it is). - Both location YAML files, plus rack and location_extended, carry the same note above their nodes block, and the three READMEs say it too, including the "LocationSite comes from either location package" line rack was missing. - load_all_schemas honours the key through _select_extensions: a member is skipped once its partner is selected, so location_minimal, the superset, is what CI loads. Because the skip only fires after the partner is loaded, the partner always comes first and a dependency on the skipped member is satisfied by it, which is what makes rack's re-point at location_site safe for hierarchy users. An experimental package always yields to the extension it collides with, whatever the load order says, so rack survives a TEST_EXPERIMENTAL run and location_extended is the one skipped. Resolver check: 57 nodes ordered, no cycle. Selection drops location_site without the flag, and location_site plus location_extended with it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packets: STRUCT-13 Checklist: CC-1 CC-1 makes .metadata.yml the normative dependency graph, and this task hard-codes a second, hand-ordered copy of it that already contradicts it: extensions/circuit sat six positions before its own declared dependency extensions/location_minimal, and only survived because circuit.yml happens to reference no location kind. Route the list through the resolver the file already contains and drop the TEMPORARY marker; the [DEMO] docstring says what the task is. Order produced: base, aggregate, cable, circuit, compute, lag, location_minimal, vlan, vrf, cluster, rack, qinq, hosting_cluster. Every declared edge respected, and the closure of the 13 entries adds nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CC-8 (descriptions and documentation): stale or copy-pasted text is a defect, and every node and generic carries a description. - PIM interface no longer described as an OSPF interface (routing_pim/pim.yml) - RoutingAggregateRoute.destination no longer called a static route - "A routing policiers for X" corrected in the four routing_policies_* files - IXP-level import/export policies and BGP communities described at IXP scope, not session scope (peering_ixp) - "Fully operational and currently in connected" fixed in cable, cross_connect - "Type of WDM (e.g CWDM. DWDM)" fixed three times in dwdm - "Redifine" fixed in lag - DcimModuleBay.device comment now states the real peer, DcimPhysicalDevice - DcimPhysicalDevice.name comment explains why the attribute is duplicated - DcimConnector description records the cost of kind Attribute on many - vrrp.yml computed-name note points at IpamCVLAN.name instead of the old group + ip field names - bucket (b) markers reworded as guidance: base/dcim.yml sub-interface hint, vrf.yml namespace block, the four "TODO: extend if need" markers - IpamPrefixScope drops VRF from its example scopes and gains a label; IpamVLANGroupScope drops Rack - IpamVRF.enforce_unique documented as declarative intent - DcimCircuitEndpoint description no longer points at the removed meet_me_room - grammar on IpamVLAN, IpamVRF, OrganizationGeneric, LocationGeneric - .metadata.yml: firewall_policer copy-paste, qinq and snmp stale text - 13 missing node and generic descriptions added (dwdm x3, lag, mlag, location_minimal x3, location_site, patch_panel x3, vlan) - em dash removed from line 5 of the disclaimer header in all 56 schema files - orphan docs/docs/reference/modules.mdx deleted (STRUCT-04) 58 files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CC-5 (dropdown and choice hygiene): choice names are lowercase snake_case,
every choice carries a label, and status choices reuse the base palette.
- connector_type: 28 UPPERCASE values lowercased in dwdm and patch_panel
(URM_P4, URM_P8 and Splice included)
- cable_type: mmf-om1..om5, smf-os1/os2, dac-passive, dac-active, mrj21-trunk
- cable color: dark-red, dark-purple, light-blue, dark-green, light-green,
dark-orange, light-grey, dark-grey
- routing_policies policy_type: import-policy, export-policy,
import-export-policy
- users permissions: read-only
- peering_ixp status: pre-maintenance, post-maintenance
- snmp security_level: noAuthNoPriv, authNoPriv, authPriv
- labels added to the two choice sets that had none: the virtual machine role
in compute, permissions in users
- status colors aligned to the base palette (#00d25b active): mlag interface,
routing protocol, both peering_ixp status sets, physical disk. Non-status
colors, including the routing lavender, are untouched.
The matching object files are updated in the same commit so the demo data
still loads: dwdm, patch_panel, routing_policies_{aggregate,bgp,ospf,pim},
snmp, users.
19 files.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CC-7 asks for kind and cardinality on every relationship. Twelve relationships still relied on the implicit default; each now states the kind it already resolves to at runtime, so nothing changes behaviour. - base/ipam.yml IpamIPAddress.interface gets kind Attribute, matching InterfaceLayer3.ip_addresses on the other end of ip_address__interface (DCIM-15 and IPAM-12 reached the same ruling) - kind Generic written out on: OrganizationProvider.circuits and LocationHosting.circuit_endpoints (circuit), DcimCircuit.contract (circuit_contract), ComputeGenericUnit.worker_in_cluster (cluster), RoutingBGPSession.device and OrganizationGeneric.asn (routing_bgp), InterfacePhysical.plugged_sfp, LocationHosting.spare_sfps and OrganizationManufacturer.sfps (sfp), IpamRouteTarget.import_vrf and .export_vrf (vrf) No Attribute/Generic flips beyond the ip_address__interface pair. SnmpClient.community already gained its kind earlier in the review. 7 files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CC-9 asks that every attribute and relationship carry an order_weight on the base scheme: Parent 900, name 1000, description 1100, status 1200, role 1300-1400, tags 2000. After this commit no attribute or relationship in base/ or extensions/ is missing a weight, and no two fields on a resolved node share one (checked with inheritance and extensions: blocks applied, state: absent honoured). - 106 missing weights filled, on attributes and on relationships, including the ones the review commits added: DcimInterface.mtu keeps a slot of its own, DcimDeviceWithPhysicalDisks.name, DcimPhysicalDevice.name, IpamIPAddress.interface, and the four routing_policies_* reverse relationships - scheme slots restored where they had drifted: description 2000 to 1100 in base/dcim.yml, dwdm and patch_panel; description 1200 to 1100 in base/location.yml, base/organization.yml, DcimPlatform, DcimGenericModule and vrf; tags 3000 to 2000 in organization, rack, peering_ixp (twice) and bgp_community; LocationGeneric.name 900 to 1000; status to 1200 on DcimDevice, IpamPrefix, IpamIPAddress, DcimCable, LocationSite (both files), ComputePhysicalServer, ClusterHosting, DcimGenericSFP and RoutingProtocol - Parent relationships moved to 900: DcimInterface.device, MlagInterface, DcimPhysicalDisk.device, RoutingProtocol.device, RoutingOSPFInterface.ospf, and the three new ones on RoutingPIMInterface, PeeringIXPConnection and UserAccount - sibling ties broken: VirtualizationVirtualMachine vcpu/memory/disk (all 1900), DcimPhysicalDevice position/serial_number/location (all 1500, with rack_face at 1515), GenericMlagDomain peer_links/mlag_interfaces, PeeringIXPConnection ipv6_address/router, RoutingOSPFInterface area/interface - the IpamGenericVLAN 900/910/920/930 ladder rebased onto the scheme - IPAM-13 applied to base/ipam.yml: IpamPrefix description 1100, status 1200, role 1300, scope 1500; IpamIPAddress description 1100, status 1200, role 1300, fqdn 1400, primary_of 1500, interface 1600. IpamIPAddress.vrrp moves to 1700 so it clears the pinned interface slot, and tenancy's IpamIPAddress.tenant to 1350 so it clears role Weights only. No attribute, relationship, choice or flag changes. 40 files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CC-6 asks for `optional` in writing everywhere, because the default is not the same on both sides: live 1.11.0 treats an omitted `optional` as false on an attribute and true on a relationship, which is easy to read the wrong way in a file. Each of the 98 places that relied on the default now states the value it already resolves to. - 69 attributes gain `optional: false`, 29 relationships gain `optional: true` - the value written is the current effective one, verified field by field: all 406 attributes and relationships in base/ and extensions/ resolve to exactly the same `optional` before and after this commit - `optional: false` stays next to `default_value` rather than being dropped (decision Q1a), and base/dcim.yml and base/ipam.yml each gain one note at their first status definition recording why: Infrahub 1.11 treats an attribute with a default_value as optional at runtime, so `optional: false` is there to document intent - patch_panel connector_type gets its explicit `optional: false`, the second half of DWDM-03 - `state: absent` entries are left alone, and experimental/ is out of scope per the tier rule 31 files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CC-9 has two halves: the menu intent of every node and generic is written in the file, and every concrete node is reachable from the menu. Both land here, because the routing fix is made of the same keys. include_in_menu written out (50 nodes and generics) Every definition in base/ and extensions/ now states include_in_menu. The 50 that relied on the default gain `include_in_menu: true`, the value they already resolve to, so users can read the menu shape out of the files and later move it to a menu.yml. experimental/ is out of scope per the tier rule. Menu placement (CC-3, both live-tested against 1.11.0 on :55000 earlier) - IpamAggregate moves from menu_placement OrganizationRIR to BuiltinIPAM, where an IPAM object belongs - OrganizationRIR gains menu_placement OrganizationGeneric, matching Manufacturer and Provider Routing menu path (RPOL-CC9) Ten concrete routing nodes were hidden with no menu_placement and no visible generic to reach them. Three of them inherit neither routing generic, so making the generics visible is not enough on its own. The fix is this repo's own dominant shape, used by SFP, location, organization, SNMP, patch panel and OADM: a visible generic root plus explicit menu_placement on every concrete node. - RoutingProtocol becomes visible and is relabelled from "Protocol" to "Routing Protocol", which is what a sidebar root has to say - RoutingPolicy becomes visible and gains the label "Routing Policy" it never had - menu_placement RoutingProtocol on RoutingAggregateRoute, RoutingOSPF and RoutingPIM; RoutingOSPFInterface under RoutingOSPF and RoutingPIMInterface under RoutingPIM; RoutingBGPRRCluster under the existing RoutingBGPSession root; the four RoutingPolicy flavours under RoutingPolicy The sidebar grows by two collapsed top-level entries, not by ten. Every placement target sits in the node's own declared dependency closure, so no extension can produce a dangling menu_placement. 35 files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet BGP-* / checklist CC-6: role is optional with no default value, and RoutingBGPSession.role is the attribute the rule was written against. The explicit-optional sweep only recorded the current effective value, so the semantic flip is made here on its own. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packet MOD-04 (parked objects finding, review brief item a): the schema commit turned DcimModuleBay.position into a Number with min_value 1, but the mock data still carried "1"/"2"/"3" as quoted strings, which load as Text. The PSU bays already used unquoted 4/5 and are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Parked objects finding (review brief item b): on infrahub-sdk 1.17.0 a bare string in a cardinality-many relationship list is sent as a node ID, not as a human_friendly_id, so every one of these references failed to resolve. An HFID is always a list of its parts, so single-part ones are wrapped in a list too. The comments that asserted flat strings were enough are corrected with them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Parked objects finding (review brief item c): the circuit mock data re-declared the Americas -> United States -> NYC1/SJC1 hierarchy that objects/extensions/location_minimal already owns, and re-upserting a hierarchical root fails. The endpoints now reference the existing sites by LocationHosting's name__value human_friendly_id instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Output of `uv run invoke docs.generate` after the 56 schema and metadata commits on this branch. Covers the renames (device_types/platforms, OrganizationTenant, serial_number, private, facility, IpamGenericVLAN), the new human_friendly_id and dependency declarations, and the .metadata.yml description rewrites, plus the sweeps' order_weight/optional/include_in_menu. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ider descriptions M-TEXT leftovers: the device_psu_module header announced three PSU bays while the file creates two (PSU-01); circuit.yml re-upserts Lumen and Colt with descriptions that drifted from objects/base/01_organization.yml, so whichever file loaded last won. Checklist: CC-8. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Infrahub caps `description` at 128 characters. Three descriptions added by review commit 351f655 were over the cap, and the first one aborted the whole base load on a fresh 1.11.0 instance before any node was created: base/dcim.yml generic DcimConnector 212 chars extensions/circuit node DcimCircuitEndpoint 208 chars extensions/vrf attribute enforce_unique 146 chars Each description keeps its factual core; the UI-behaviour half moves to a YAML `#` comment above the field, which has no length limit and stays next to the definition it explains. A scan of every `description` and `label` in base/, extensions/ and experimental/ (recursive walk of the parsed YAML) reports zero remaining values over 128 characters, so these three were the only offenders. Verified on a fresh infrahub-testcontainers 1.11.0 instance: `infrahubctl schema load base` now reports "4 schemas processed in 7.106 seconds" where it previously failed with "Unable to load the schema". Reference pages regenerated with `invoke docs.generate`. The three hand-written mutual-exclusion notes in extensions/*/README.md are restored afterwards because the generator overwrites them with the pointer template. Checklist: CC-8. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review commit 0ffcd60 gave DcimPhysicalDisk a human_friendly_id of [device__name__value, name__value] to match its uniqueness_constraints, but left the peer generic DcimDeviceWithPhysicalDisks with a plain `name`. Infrahub only accepts an HFID that walks a peer when the referenced attribute combination on that peer is unique, so a fresh 1.11.0 instance rejected the extension outright: HFID of DcimPhysicalDisk refers to peer DcimDeviceWithPhysicalDisks with a non-unique combination of attributes ['name__value'] `unique: true` on that attribute closes the gap. The library ships no implementer of this generic (it is documented in .metadata.yml as one you apply to your own device kind), and every device kind here inherits DcimGenericDevice, which already declares name unique, so no shipped node changes behaviour. The existing `allow_override: any` still lets an implementer restate the attribute. This mirrors DcimPhysicalDevice.name in base/dcim.yml, which carries the same unique + allow_override pair and the same explanatory comment. Verified on a fresh infrahub-testcontainers 1.11.0 instance: `infrahubctl schema load extensions/physical_disk` reports "1 schema processed in 4.891 seconds"; it previously aborted on the HFID error above. Checklist: CC-4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
objects/extensions/circuit loads at schema-dependency position 2 but referenced
LocationSite NYC1 and SJC1, which objects/extensions/location_minimal only
creates at position 7. On a fresh 1.11.0 instance the file aborted with:
Unable to find the node NYC1 / LocationHosting
DcimCircuitEndpoint.location is mandatory and its peer LocationHosting is a base
generic that base itself never implements, so the fixture has to bring its own
sites. They are created with name and description only, without a parent:
extensions/location_site has no tier above Site and extensions/location_minimal
puts sites under Americas -> United States, so restating either tree would tie
this file to one of the two mutually exclusive location extensions. Restating
location_minimal's is not possible in any case, re-upserting the hierarchy root
fails with "Not supported to assign a value to parent for LocationRegion".
Declaring extensions/location_site as a dependency in .metadata.yml (the way
extensions/rack does, which would move circuit behind the location fixture) was
rejected: CC-1 dropped circuit -> location_minimal as a declared-but-unused
edge, and circuit.yml still references no LocationSite kind, only the base
LocationHosting generic.
On the parent question the brief raised: a partial LocationSite upsert does
clear `parent`, confirmed directly (load location_minimal, upsert
{name: NYC1, description: ...}, parent goes from "United States" to null). It is
not a consequence of this change. objects/extensions/sfp, cross_connect, vlan,
compute, dwdm and hosting_cluster already carry fifteen nested
{kind: LocationSite, data: {name: ...}} blocks that do exactly the same thing,
several of them after location_minimal in the load order, so NYC1 and SJC1 end
a full ordered load without a parent with or without this commit. This file
adds no new exposure: at position 2 it runs before every other file that
touches a site. Cleaning up the fifteen pre-existing blocks (a plain HFID
scalar resolves against LocationHosting and does not re-save the node) is a
separate sweep.
Verified on a fresh infrahub-testcontainers 1.11.0 instance: objects/base plus
all 34 objects/extensions directories load in resolver order with zero errors,
35/35, where objects/extensions/circuit previously failed. GraphQL then reports
DcimCircuit 3 and DcimCircuitEndpoint 4.
Checklist: CC-1.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mechanism, read out of infrahub-sdk 1.17.0 and then confirmed on a live
instance. `infrahubctl object load` builds every record from scratch and saves
it with allow_upsert. In InfrahubNodeBase._generate_input_data
(node/node.py:213) a relationship that the document leaves out is treated three
different ways: cardinality-many is skipped, mandatory cardinality-one is
skipped, and OPTIONAL cardinality-one is written into the payload as an
explicit None. A partial re-upsert therefore clears exactly the optional
cardinality-one links, and `create()` sends that payload unfiltered because
allow_upsert never passes exclude_unmodified.
DcimDevice.mlag_domain is optional cardinality one and shares the identifier
device__mlag_domain with MlagDomain.peers, which is min_count/max_count 2.
objects/extensions/mlag wired nyc1-rtr01 and nyc1-rtr02 into NYC1-MLAG-01 at
resolver position 16; objects/extensions/rack (17), device_psu_module (18),
dwdm (22), routing_ospf (28) and routing_pim (29) each re-upsert nyc1-rtr01
afterwards without it, and all five failed with:
['DcimDeviceUpsert'] Node <id> has 0 peers for device__mlag_domain,
no fewer than 2 allowed
Restating mlag_domain in those five files was rejected: objects/extensions/rack
and four others would then fail whenever extensions/mlag is not loaded, which
inverts the layering and contradicts the self-sufficiency rule applied to the
circuit fixture. Referencing the device instead of re-declaring it is not
available either, DcimModuleBay.device peers with DcimPhysicalDevice, which
declares no human_friendly_id, so the nested {kind, data} block that re-saves
the device is the only way to express that link.
So the fixture owns its own endpoints. The domain is now built on nyc1-rtr02
and a new nyc1-rtr03, and this file is the only one anywhere under objects/
that ever saves either of them; nyc1-rtr01 is left to the files that own it.
objects/extensions/routing_bgp and peering_ixp are unaffected, they reference
nyc1-rtr02 by human_friendly_id, which does not re-save it. Both peers moved
into a single document so the LocationRack block can be shared through a YAML
anchor, which does not cross a `---` boundary.
The upsert rule is written out once, in this file's header, with the failing
error text, so a future file re-declaring a linked node has somewhere to look.
objects/extensions/circuit and objects/extensions/vrrp point at it.
Verified on a fresh infrahub-testcontainers 1.11.0 instance: all 35 object
directories load in resolver order with zero errors, and GraphQL reports
NYC1-MLAG-01 with peers nyc1-rtr02 and nyc1-rtr03, two peer_links and
MlagInterface 2.
Checklist: CC-1, CC-7.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same defect family as the MLAG one, uncovered once that fix let objects/extensions/rack get past its device upsert. rack then failed on the nested interface instead: ['InterfacePhysicalUpsert'] Node <id> has -1 peers for interfacelayer3__networkvrrpinterface, no fewer than 1 allowed extensions/vrrp adds InterfaceLayer3.vrrp as an optional cardinality-one relationship, and its counterpart NetworkVRRPInterface.interface is mandatory. objects/extensions/vrrp (resolver position 14) put NYC1-CORE-VRRP on GigabitEthernet0/0/0/0, which objects/extensions/rack (17) owns and re-declares afterwards; the re-declaration nulls the optional side and would orphan the NetworkVRRPInterface, so the server refuses the mutation. The group moves to GigabitEthernet0/0/0/3, which nothing else creates or touches, so all three VRRP groups now sit on interfaces this file creates itself (/1, /2, /3). The header records why /0 is off limits and points at the upsert rule in objects/extensions/mlag/mlag.yml. The stale claim that GigabitEthernet0/0/0/0 "already exists (from extensions/rack)" goes with it: rack loads after this file, sfp is what creates that interface earlier. A sweep of the live 1.11.0 schema for the same shape (an optional cardinality-one relationship whose counterpart is mandatory cardinality-one) found only two others: InterfaceLag.vrrp, which no fixture uses, and DcimModuleBay.installed_module, where objects/extensions/device_psu_module creates fresh bays instead of reusing the occupied ones. Verified on a fresh infrahub-testcontainers 1.11.0 instance: objects/base plus all 34 objects/extensions directories load in resolver order with zero errors, 35/35. GraphQL reports three NetworkVRRPInterface records on GigabitEthernet0/0/0/1, /2 and /3. Checklist: CC-7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying schema-library with
|
| Latest commit: |
af2ced0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a5c1e871.schema-library.pages.dev |
| Branch Preview URL: | https://review-schema-library-v2.schema-library.pages.dev |
The markdown lint CI job failed with a single MD047 violation on docs/docs/home.mdx, which is generated from docs/_templates/home_page.j2 by _generate_home_page_documentation(). The template ends with a newline, but the renderer built its environment as jinja2.Environment(trim_blocks=True), and keep_trailing_newline defaults to False, so Jinja stripped the template's final newline from the rendered output. Regenerating the docs therefore reproducibly emits a home page that markdownlint rejects. Set keep_trailing_newline=True and regenerate. The flag only suppresses the lexer's end-of-source newline strip, so the output is the previous output plus one newline, and nothing else changes. Co-Authored-By: Claude Fable 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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Review output for #75. I went through the v2 diff decision by decision (142 decisions extracted from the net diff, each judged against a checklist agreed before judging) and this branch carries every "change" verdict as one commit with the reasoning in the commit message. 55 commits.
Key changes
Review context
Blind pass: rationale was taken only from the PR body, commit messages and YAML comments, not from existing review threads. The 76 accept and 11 question verdicts follow as inline threads on #75.
Test plan
uv run invoke schemas.load-all-schemason a fresh 1.11.0: zero errors, location_site skipped as mutually exclusive with location_minimalTEST_EXPERIMENTAL=1pass: everything loads except experimental/security, which still carries the Infra* kinds already fixed on main in d5ae01b, so it resolves once main is merged back🤖 Generated with Claude Code