Skip to content

Architecture review: full-codebase baseline #72

Description

@marcinpsk

Architecture review: full-codebase baseline

Scope: first-use baseline across the complete codebase. Recent history and prior heat maps are excluded from scoring. This review proposes deepening opportunities only. It does not propose interfaces yet.

Baseline

  • 4,938 production Python lines
  • 11,078 test Python lines
  • Production scan: rule model, rename engine, Django lifecycle, foreground views, background jobs, REST adapter, GraphQL adapter, forms, filters, tables, navigation, templates, and migrations
  • Supporting scan: tests, documentation, and contributed rule examples
  • No CONTEXT.md or ADRs were found. Domain terms come from the code and project documentation.

Architecture vocabulary:

  • module: solid box
  • seam: dashed connection
  • leakage: red connection
  • deep module: dark box

1. Make the channelized family one deep module

Recommendation: Strong | in-process

Files

  • netbox_interface_name_rules/engine.py:311-669
  • netbox_interface_name_rules/engine.py:965-1059
  • netbox_interface_name_rules/engine.py:1354-1691
  • netbox_interface_name_rules/engine.py:1835-2208
  • netbox_interface_name_rules/engine.py:2211-2565
  • netbox_interface_name_rules/tests/test_channelization.py
  • netbox_interface_name_rules/tests/test_channelized_mode.py
  • netbox_interface_name_rules/tests/test_conversion.py

Problem

Callers repeatedly interpret family identity, topology, names, collisions, and deferred outcomes through incompatible shapes. The implementation represents the same family as ORM rows, parent and child dictionaries, preview dictionaries, template dictionaries, and conversion tuples.

Solution

Concentrate family discovery, planning, mutation, conversion, and reconciliation inside one deep module. Keep NetBox ORM details in its implementation.

flowchart LR
  subgraph Before
    I[Install] --> R1[ORM rows]
    P[Predict] --> R2[Template maps]
    V[Preview] --> R3[Result dictionaries]
    B[Bulk apply] --> R4[Parent maps]
    C[Convert] --> R5[Conversion tuple]
    R1 -. family rules leak .-> R2
    R2 -. family rules leak .-> R3
    R3 -. family rules leak .-> R4
    R4 -. family rules leak .-> R5
  end

  subgraph After
    I2[Install] -. seam .-> F[Deep channelized family module]
    P2[Predict] -. seam .-> F
    V2[Preview] -. seam .-> F
    B2[Bulk apply] -. seam .-> F
    C2[Convert] -. seam .-> F
    F --> K[Identity, topology, names, collisions, mutation, reconciliation]
  end

  classDef leak stroke:#dc2626,stroke-width:2px,color:#991b1b;
  classDef deep fill:#0f172a,color:#ffffff,stroke:#020617,stroke-width:4px;
  class R1,R2,R3,R4,R5 leak;
  class F deep;
Loading

Benefits

  • locality: one family invariant
  • leverage: five paths reuse decisions
  • interface becomes the test surface
  • parity becomes structural
  • deferred ordering stays internal

Deletion test: deleting this module would scatter family detection, naming, collision rules, conversion, and reconciliation back across five callers.

2. Put rule selection behind one deep module

Recommendation: Strong | in-process

Files

  • netbox_interface_name_rules/engine.py:26-258
  • netbox_interface_name_rules/engine.py:778-856
  • netbox_interface_name_rules/engine.py:1090-1212
  • netbox_interface_name_rules/engine.py:1814-1832
  • netbox_interface_name_rules/models.py:306-353
  • netbox_interface_name_rules/tables.py:10-41
  • netbox_interface_name_rules/tests/test_rules.py

Problem

Eligibility and precedence are one domain fact. Automatic apply, device apply, bulk scans, cache invalidation, and displayed priority interpret that fact separately. Tests reach past the current interface to mutate _RULE_CACHE, _pin, _MEMO_MAX, and matching helpers.

Solution

Let one deep module own selection and derive displayed priority. Hide fingerprinting, snapshots, memoization, and concurrency in its implementation.

flowchart LR
  subgraph Before
    A[Automatic apply] --> T[Cache and tiers]
    D[Device apply] --> O[Queryset and Python ordering]
    B[Bulk scan] --> Q[Scope queryset]
    U[Rule table] --> S[Score and label]
    M[Rule field change] -. coordinated edits .-> T
    M -. coordinated edits .-> O
    M -. coordinated edits .-> Q
    M -. coordinated edits .-> S
  end

  subgraph After
    A2[Automatic apply] -. selection seam .-> R[Deep rule selection module]
    D2[Device apply] -. selection seam .-> R
    B2[Bulk scan] -. selection seam .-> R
    U2[Rule table] -. selection seam .-> R
    R --> P[Eligibility, precedence, priority]
    R --> C[Private cache implementation]
  end

  classDef leak stroke:#dc2626,stroke-width:2px,color:#991b1b;
  classDef deep fill:#0f172a,color:#ffffff,stroke:#020617,stroke-width:4px;
  class T,O,Q,S leak;
  class R deep;
Loading

Benefits

  • locality: one precedence policy
  • leverage: every selector agrees
  • cache state stays internal
  • tests stop mutating globals
  • display matches selection

Deletion test: deleting this module would spread selection and cache policy across engine, model, table, and device paths.

3. Deepen the name-template language

Recommendation: Strong | local-substitutable

Files

  • netbox_interface_name_rules/models.py:19-114
  • netbox_interface_name_rules/models.py:250-278
  • netbox_interface_name_rules/forms.py:23-184
  • netbox_interface_name_rules/views.py:41-61
  • netbox_interface_name_rules/views.py:298-379
  • netbox_interface_name_rules/engine.py:1554-1574
  • netbox_interface_name_rules/engine.py:2568-2614
  • netbox_interface_name_rules/api/serializers.py:34-76
  • netbox_interface_name_rules/tests/test_engine.py

Problem

Model, form, view, serializer, and engine must know overlapping syntax, variable, safety, and validation rules. The model states that its AST walk mirrors evaluate_name_template, and the form imports private model validation details. This is an explicit drift risk.

Solution

Concentrate parsing, accepted variables, expression safety, validation, and evaluation in one deep language module used by saved and unsaved rule adapters.

flowchart TB
  subgraph Before
    M[Model validation]
    F[Form validation]
    V[Manual preview]
    E[Engine evaluation]
    S[Serializer validation]
    M -. mirrored syntax .-> E
    F -. private helpers .-> M
    V -. repeats expansion .-> E
    S -. repeats validity .-> M
  end


  subgraph After
    SA[Saved rule adapter] -. language seam .-> L[Deep name-template language module]
    UA[Unsaved preview adapter] -. language seam .-> L
    L --> P[Parsing and accepted variables]
    L --> X[Expression safety and evaluation]
    L --> H[Historical validity rules]
  end

  classDef leak stroke:#dc2626,stroke-width:2px,color:#991b1b;
  classDef deep fill:#0f172a,color:#ffffff,stroke:#020617,stroke-width:4px;
  class M,F,V,E,S leak;
  class L deep;
Loading

Benefits

  • locality: syntax changes once
  • leverage: every caller agrees
  • one production test surface
  • private helpers stop leaking
  • preview matches apply

Deletion test: deleting this module would push parser, validator, and variable knowledge back into model, form, view, serializer, and engine.

4. Deepen the deferred rename lifecycle

Recommendation: Worth exploring | ports & adapters

Files

  • netbox_interface_name_rules/signals.py:14-247
  • netbox_interface_name_rules/engine.py:1397-1451
  • netbox_interface_name_rules/tests/test_signals.py
  • netbox_interface_name_rules/tests/test_channelization.py
  • netbox_interface_name_rules/tests/test_channelized_mode.py

Problem

Correctness depends on previous-state capture and two deferred phases. Their ordering is split between Django adapters and the rename implementation. A failed pre-save read becomes None, which the post-save path interprets as no change. A real change can therefore lose its scheduled work.

Solution

Keep receivers thin. Place state comparison, durable row reload, commit scheduling, callback ordering, reconciliation, and failure policy behind one deep lifecycle module.

flowchart LR
  subgraph Before
    PRE[Django pre-save adapter] --> ATTR[Previous state on model]
    ATTR --> POST[Django post-save adapter]
    POST --> COMMIT[First on-commit phase]
    COMMIT --> NETBOX[NetBox callback]
    NETBOX --> REPAIR[Second deferred phase]
    PRE -. timing knowledge .-> REPAIR
  end

  subgraph After
    MS[Module save adapter] -. lifecycle seam .-> L[Deep deferred rename lifecycle module]
    DS[Device save adapter] -. lifecycle seam .-> L
    NC[NetBox callback adapter] -. lifecycle seam .-> L
    L --> STATE[State, reload, scheduling, ordering, failures]
  end

  classDef leak stroke:#dc2626,stroke-width:2px,color:#991b1b;
  classDef deep fill:#0f172a,color:#ffffff,stroke:#020617,stroke-width:4px;
  class ATTR,POST,COMMIT,NETBOX,REPAIR leak;
  class L deep;
Loading

Benefits

  • locality: timing knowledge concentrates
  • leverage: save paths share rules
  • adapter tests stay behavioral
  • callback ordering becomes testable
  • failures stop meaning no-change

Deletion test: deleting this module would restore scheduling and callback-order knowledge to several receivers and the family implementation.

5. Deepen foreground and background rule operations

Recommendation: Worth exploring | ports & adapters

Files

  • netbox_interface_name_rules/jobs.py:8-73
  • netbox_interface_name_rules/views.py:425-557
  • netbox_interface_name_rules/engine.py:2155-2208
  • netbox_interface_name_rules/engine.py:2522-2565
  • netbox_interface_name_rules/tests/test_misc.py
  • netbox_interface_name_rules/tests/test_views.py

Problem

Foreground and background adapters duplicate execution policy while sharing only counts and a mutable conflict list. The list shape and outcome meaning live in private engine helpers.

Solution

Concentrate selection validation, execution outcomes, and reporting facts in one deep operation module. Let each adapter render those facts as Django messages or JobRunner logs.

flowchart LR
  subgraph Before
    V[Foreground view] --> V1[Parse selection]
    V1 --> V2[Apply limit]
    V2 --> V3[Count and mutable conflicts]
    J[Background job] --> J1[Load rule]
    J1 --> J2[Choose all]
    J2 --> J3[Count and mutable conflicts]
    V3 -. shared meaning leaks .-> J3
  end

  subgraph After
    V4[Foreground adapter] -. operation seam .-> O[Deep rule operation module]
    J4[Background adapter] -. operation seam .-> O
    O --> F[Selection, execution, rename facts, skip facts]
    F --> DM[Django messages]
    F --> JL[JobRunner logs]
  end

  classDef leak stroke:#dc2626,stroke-width:2px,color:#991b1b;
  classDef deep fill:#0f172a,color:#ffffff,stroke:#020617,stroke-width:4px;
  class V3,J3 leak;
  class O deep;
Loading

Benefits

  • locality: outcomes mean one thing
  • leverage: two adapters share policy
  • mutable lists leave the interface
  • real operation tests gain value
  • adapter tests become narrow

Deletion test: deleting this module would spread operation policy back across two real adapters. Two adapters confirm that the seam is not hypothetical.

Top recommendation

Make the channelized family one deep module.

It removes the widest semantic duplication and establishes one test surface before lifecycle or adapter changes.

Next decision: select one candidate for the grilling loop. Exact interfaces remain deliberately undecided.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions