fix: read {base} as the raw template name on every module rename - #104
Merged
marcinpsk merged 8 commits intoSep 23, 2026
Merged
Conversation
A module rename took {base} from the interface's current name. A reapply
runs the planner again on rows an earlier apply renamed: a module type
change, a virtual-chassis change, Apply Rules, or a second apply. A rule
whose templates use {base} then renamed the result again. '{base}-x' went
from 3-x to 3-x-x, a channelized '{base}-parent' parent grew on every
apply, and '{base}:{channel}' channels changed once after install.
NetBox keeps no link from an interface to its template, so RawBases
recovers the raw name by a claim. A row belongs to a template when its name
is the template's raw name, a historical virtual-chassis form of it, or the
name the rule gives it, with {vc_position} matched at any position. An
exact raw name wins first, as in the drift guard. A row that no template
claims, or that two templates claim, is blocked and the reason is logged.
Rules without {base} skip the claim and keep their behavior. A module type
without interface templates has no raw names, so there {base} stays the
current name. Device-interface rules keep the current name. Prediction
receives raw names and takes them as they are.
Closes #103
…eviews
Three gaps in the raw-base claim, found in review:
- A raw name that spells the claim marker made the second replacement
rewrite the inserted raw name, and re.compile raised. The pattern is now
built from the pieces between markers.
- A rule with arithmetic over {base} matched only the current
virtual-chassis position, so a renumber left its interfaces unclaimed.
Evaluation now tries each marker combination and keeps a marker for every
variable that is not in arithmetic.
- The Apply Rules preview blocked the members of an installed flat family,
because their names are no template's renamed form. Apply Rules still
renamed a drifted family, so the preview hid it. The preview now plans
installed flat families with the same plan Apply Rules executes.
…not spell
A family built on a drifted interface named its parent from the recovered
raw name when the parent template was blank. A blank parent template keeps
the interface's current name, so channelized_family_names and
intended_family_names now take the current name and the base separately.
The claim matched marker text in a rule template, a raw name or a variable
value as the variable it stands for. RawBases now picks markers that none
of them contains.
A {base} inside arithmetic, on a template that uses the {vc_position}
token, still matches only the current position. The guide states it.
The claim markers were plain text, so a rule could still assemble one
during evaluation ('InrRawBaseMark{0}'), and recovery then read it as a
variable. PostgreSQL text cannot hold a NUL byte, so no stored template,
name or variable value can spell a NUL-delimited marker, and arithmetic
yields digits only. The markers now use NUL, and the free-marker search is
gone.
A forced reapply reaches rows an earlier apply already renamed, so a
rule that renames nothing there is working, not obsolete. The
potentially-deprecated heuristic now runs only without force. Before this,
every virtual-chassis renumber tagged every applied module rule. The test
that pinned that path is rewritten to cover the case the heuristic is for:
a rule whose output is the raw name the module already has.
An exact raw name claimed its interface before any other form was
checked. When one template's renamed name equals another template's raw
name, the interface went to the wrong template. Templates 'port' and
'port.2' with '{base}.{vc_position}' renamed 'port.2' to 'port.2.3' after a
renumber.
A raw name now beats only another template's historical virtual-chassis
form, as in the drift guard. A renamed form that matches it makes the
name ambiguous, so neither template claims it and the interface keeps its
name. A module type whose templates overlap under the rule therefore
renames nothing, on install too.
… marker The Apply Rules preview now builds RawBases and installed flat plans for each module, and both read the module's templates. find_interfaces_for_rule did not pin the template cache, so each module cost a refetch and a template query. It now selects the bay-chain relations and pins the cache for its modules, as the batch path does. _historical_bases recovered drifted flat-family bases through a plain-text sentinel, the shape the raw-base claim left in 1884451. A rule that spells the sentinel built the wrong matcher, and conversion stopped offering the family. It now uses the same NUL marker. install_channelized_family passed the row's current name as {base} and only a test called it, so it is removed. raw_bases reads the vc_position digit pattern from template_names instead of a copy.
Prediction took every given name as its own base, so it predicted renames for templates whose names overlap under the rule, which installation leaves alone. given_raw_names still keeps a given name as its own base, so a stale name is predicted from itself as documented, but it refuses a name the claim over the given names finds ambiguous. A limited preview counted an installed flat family once in the modules it visited and each interface in the modules it skipped, so its total differed from an unlimited preview. A flat family now counts its members.
Prediction ran the raw-base claim over every given name, channels included. A channel name that matches its parent's renamed form made the parent ambiguous, so prediction kept names that installation renames. Installation claims only interfaces outside a channel. Prediction now drops the given names its templates describe as channels, and it still claims over the given names only.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (24)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughModule ChangesModule raw-base resolution
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant plan_module_families
participant module_raw_bases
participant RawBases
participant resolve_template_claims
participant FamilyPlanners
plan_module_families->>module_raw_bases: Build bases for module interfaces
module_raw_bases->>RawBases: Create raw-base resolver
RawBases->>resolve_template_claims: Resolve template claims
resolve_template_claims-->>RawBases: Return claims and ambiguities
RawBases-->>plan_module_families: Provide base lookups
plan_module_families->>FamilyPlanners: Plan installed and prospective families
Merge Risk: ⚪ Minimal · up to Module rules that use 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 64.93% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 134 functions across 21 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks each template name Comment |
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
marcinpsk
merged commit Sep 23, 2026
4e45a89
into
refactor/name-template-language-seam
6 checks passed
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.
Closes #103. Stacked on #101.
Problem
A module rename took
{base}from the interface's current name. A reapply runs the planner again on rows an earlier apply already renamed. It happens on a module type change, a virtual-chassis change, Apply Rules, or a second call toapply_interface_name_rules. A rule whose templates use{base}then renamed its own output:{base}-x3-x3-x-xon a forced reapply,3-x-x-xon Apply Rules{base}-parent7-parent-parenton every apply, forced or not{base}:{channel}8:1..8:4et-0/0/8:1.. on the first reapplyChange
family/raw_bases.pyaddsRawBases. It gives each live interface name the raw template name that a{base}rule reads. NetBox keeps no link from an interface to its template, so the value comes from a claim over the module's templates:{vc_position}matches any position, so a renumber still finds the row. A variable used inside arithmetic cannot take a marker, so it matches only its current value.BLOCKEDwith a logged reason. Two-sided ambiguity goes through the existingresolve_template_claims, with a newraw baselabel kind.Every module-context planner now takes
{base}fromRawBases: plain renames, installed channelized families (breakout and lockstep), creation of flat and channelized families, and the Apply Rules preview of live rows.channelized_family_targetsandlockstep_family_targetstakebase_nameseparately from the parent's current name, because suffix recovery still needs the current name.Scope decisions:
{base}never computes the claim, so its behavior is unchanged.name_template.variable_tokenis the one definition the evaluator andreferences_variableshare.{base}stays the current name, as before.{base}as the current name, which is their documented input.given_raw_nameskeeps each given name as its own base, so a stale name is still predicted from itself as documented. It refuses a name the claim finds ambiguous, because install leaves that name alone.The preview selects the bay-chain relations and pins the template cache, as the batch path does, so one module type's templates load once per scan. The Apply Rules preview now plans installed flat families with
plan_installed_flat_families, the plan that Apply Rules executes. Before this change, it planned their members as new families built on each member name (brk-brk-xe-1/0/3:0:0). The claim would otherwise block those members, which would hide a drifted family that Apply Rules still renames.The claim builds its renamed-form matcher from NUL-delimited markers. PostgreSQL text cannot hold a NUL byte, so no stored template, name or variable value can spell a marker, not even through arithmetic.
Known limits, which fail closed: the interface keeps its name and a warning is logged.
{base}inside arithmetic, on a template that uses the{vc_position}token, matches only the current position. On the parent branch the same rule failed to evaluate, so nothing regresses.portandport.2with{base}.{vc_position}. Such a module type renames nothing, even on install, where the parent branch renamed it. The parent branch also renamed it wrongly after two renumbers (port.2toport.2.3).The potentially-deprecated heuristic now runs only without force. A forced reapply reaches rows that an earlier apply renamed, so a rule that renames nothing there is working. Before this change, every virtual-chassis renumber tagged every applied module rule.
test_no_op_rename_adds_deprecated_tagpinned that path. It now covers the case the heuristic is for: a rule whose output is the raw name the module already has.TemplateNamesmoved frominstalled.pytotemplate_names.py, soRawBasesand the planners share one lazy template load.plan_installed_familieslost its unusedinterfacesargument. The batch path callsplan_installed_families_fromwith the rows and bases it already has.install_channelized_familyis removed: it passed the current name as{base}, and only a test called it._historical_bases, the conversion matcher, uses the same NUL marker.Behavior changes in existing tests
test_module_base_description_stays_true_of_reapplication_behaviorpinned the parent growth. It now asserts that a reapply renames nothing.ChannelizedSuffixRecoveryTestexpected1a-x-xand2-x-x:2after a forced reapply. That was the growth. The stranded child still heals, now to2-x:2.test_two_token_templates_that_do_not_overlap_both_rename:et-{base}after a renumber to position 6 now reads the raw name as it resolves at position 6 (xe-6/0/4). A flat family already uses that base.Tests
test_raw_base.pycovers each case through real installs, realDevice.save()renumbers and the real Apply Rules entry points. The channelized cases run on NetBox 4.7 only. The cases:{base}{base}across a renumberAll fail on the parent branch except the control, a rule without
{base}.The catalogue description of
{base}for module contexts is now "The raw template name of the interface the rule renames." The generated references are regenerated, and "What{base}starts from" indocs/template-variables.mdstates the claim and its exceptions.Summary by CodeRabbit
Bug Fixes
{base}to the raw template name on installation and reapplication, preventing repeated name changes.Documentation
{base}behaves for module and device interface naming rules.