Add model-0.2.37 cortex for the bank identifier migrations (SYN-10665) - #33
Merged
Conversation
…665) The Synapse 0.2.37 model revision anchors the end of the ``econ:bank:swift:bic`` regex. The previous pattern was anchored only at the start, so ``Str.norm`` accepted trailing characters after a valid BIC, and those values no longer re-norm. The cortex contains three invalid BICs and one valid 11 character BIC which the migration must leave alone. One of the invalid values lives in a forked view so the migration is exercised across more than one layer, and one carries properties, a tag, nodedata and an edge so the quarantined queue record can be checked in full. Generated with Synapse 2.250.0 from before the model change. The generator guards on both the Synapse version and ``s_modelrev.maxvers``, since a 2.250.0 checkout which already carries the model change would pass a version check alone and silently produce a cortex with no invalid data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
mikemoritz
added a commit
to vertexproject/synapse
that referenced
this pull request
Aug 28, 2026
…-10665) Anchoring the end of the ``econ:bank:swift:bic`` regex narrows a released type, so values normed under the previous pattern may no longer re-norm. The previous pattern was anchored only at the start, because ``Str.norm`` calls ``self.regex.match()``, so it accepted trailing characters after a valid BIC. A trailing character BIC has no derivable correct value, so the migration removes the node rather than moving it. Each removed node is recorded in the ``model_0_2_37:nodes`` queue with its properties, tags, nodedata and edges, so an operator can rebuild it with the correct value. Unlike the 0.2.31 and 0.2.35 migrations, no node's primary value changes here: the new pattern accepts every valid BIC the old one did. No property in the model is typed ``econ:bank:swift:bic`` either, so there is no re-keying and no inbound reference to rewrite. No operator stormlib helper, following 0.2.35 rather than 0.2.31. Repairing a BIC requires a human supplying the correct code, and the queue is reachable through ``$lib.queue.get()``. Requires the ``model-0.2.37`` cortex from vertexproject/synapse-regression#33. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
…YN-10665) The 0.2.37 model revision now anchors the end of the ``econ:bank:aba:rtn`` and ``econ:bank:iban`` patterns as well, matching Synapse 3.x. Unlike the BIC change these are pure narrowings, with no widening half. Regenerated with the additional data: * an ``econ:bank:account`` referencing an invalid ABA RTN and an invalid IBAN. Neither referring property is read-only, so the account must survive the migration with those properties deleted rather than being removed along with them. This is the first cortex to exercise that path, since no property in the model is typed ``econ:bank:swift:bic``. * a standalone invalid ABA RTN carrying properties and no referrer * valid values of both types which the migration must leave alone Layer and view idens change because the cortex is rebuilt from scratch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
The 0.2.37 migration removes a node by calling ``ModelMigrationBase.removeNode()``, which recursively removes any node referencing it through a read-only property. That arm was unexercised: the ``econ:bank:account`` referrers added previously are not read-only, so only the property-clearing path ran. Added a ``meta:seen`` referencing the invalid IBAN. ``meta:seen:node`` is a read-only ndef, so the migration must remove the ``meta:seen`` node along with the IBAN and queue it under its own record. The ``meta:source`` it points at must survive, since the cascade follows references inbound to the removed node rather than outbound from it. ``meta:seen`` is deprecated, but read-only ndef properties in 2.x live almost entirely on legacy forms since the model moved to light edges, and ``gen-cpe-migration.py`` uses it for the same purpose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
mikemoritz
added a commit
to vertexproject/synapse
that referenced
this pull request
Aug 29, 2026
The 0.2.37 migration removes nodes as its only outcome, so its queue is expected to hold entries. That matches the 0.2.31 migration, which has a repair library, and not 0.2.35, whose ``removeNode()`` calls are both defensive branches marked ``# pragma: no cover``. Repair matters more here than the node contents alone suggest. The ``econ:bank:account`` properties which pointed at a removed identifier are cleared by the migration, and ``repairNode()`` re-points them at the corrected value. Rebuilding the node by hand would restore the node but silently leave the account without its identifier. Extracted ``LibModelMigrationsQueue`` so both libraries share the implementation and differ only by ``queuename``. The three ``_meth`` entry points are declared on each subclass rather than inherited because ``registry.registerLib()`` stamps ``_storm_funcpath`` onto the function object: with inherited methods both libraries resolve to the same object, and the second registration relabels the first. That would have renamed the released 0.2.31 library in its own error messages. Also covers the read-only referrer cascade added to the regression cortex in vertexproject/synapse-regression#33, and switches the Storm boolean literals in this file from ``$lib.true`` to ``(true)``. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
The migration collects sodes per layer and quarantines all of them, but every invalid node in the cortex had data in exactly one layer, so ``delNode()`` never iterated a second sode. Added a tag and nodedata on the base layer's invalid BIC from inside the forked view. The fork's own layer therefore holds a sode for that node with no primary value, which is the case the migration's missing valu guard exists for: ``getStorNodesByForm()`` yields any node with props, tags, tagprops, edges or nodedata in the layer, not only nodes rooted there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
mikemoritz
added a commit
to vertexproject/synapse
that referenced
this pull request
Aug 31, 2026
Repairing a node which had a read-only referrer takes two ``repairNode()`` calls: ``_repairNode()`` cannot re-point a read-only property, so the referring node was removed and queued under its own entry. Nothing told the operator the second call existed, and the referring node's queued ``formvalu`` embeds the old broken value, so it cannot simply be replayed. ``printNode()`` already prints the refs list and had ``isro`` available in ``refinfo`` but discarded it. Marking the read-only entries surfaces the follow-up where someone reading a record before repairing it is already looking. This applies to the 0.2.31 library as well, which has the same two step repair. Also covers the two layer node added to the regression cortex in vertexproject/synapse-regression#33, and reverts the ``$lib.true`` changes on the two 0.2.31 test lines this PR does not otherwise touch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs
vEpiphyte
approved these changes
Sep 1, 2026
vEpiphyte
pushed a commit
to vertexproject/synapse
that referenced
this pull request
Sep 2, 2026
… (SYN-10665) (#5008) ## Summary Model updates to support ingesting bank fraud and scam reporting from a third party feed, plus the model revision the type changes require. These bring the 2.x model closer to 3.x for threat clustering and bank identifiers. The `model-0.2.37` regression cortex that `test_modelrev_0_2_37` loads landed in vertexproject/synapse-regression#33. ### `risk:threat:ext:ids` `:ext:id` gains `'alts': ('ext:ids',)` plus the array property, mirroring `meta:cluster:id`/`:ids` verbatim. The vendor mints one report id per report and a scam operator spans several, so the cluster collapses onto one `risk:threat` and must resolve from any member id. ### `risk:attack:actor` Typed `entity:actor`, the existing 2.x ndef over `ou:org`, `ps:person`, `ps:contact` and `risk:threat` which `entity:relationship:source` and `:target` already use. This matches 3.x, where `risk:attack` picks up `:actor` from the `entity:action` interface typed `entity:actor`. 2.x `risk:attack` previously had no route to `risk:threat` at all -- `:attacker` is a `ps:contact`. Note it sits next to the deprecated `:actor:org` and `:actor:person`. Those are unrelated and stay deprecated in favour of `:attacker`. ### `econ:bank:account:swift:bic` The `econ:bank:swift:bic` form existed but had nowhere to hang off an account, while `:aba:rtn` did. That asymmetry does not reflect real reporting, where the BIC is at least as common an account identifier as an ABA routing number. Placed beside `:aba:rtn`, since both identify the bank which issued the account, where `:number` and `:iban` identify the account itself. This is not a property-to-property copy for a future 3.x migration -- 3.x has no `:swift:bic` anywhere, because routing lives in the primary value of the `econ:bank:account` comp and `econ:bank:swift:bic` is one of three forms implementing `econ:bank:routing:code`. The gain is that the account-to-BIC pairing becomes machine-readable rather than only inferable through `:issuer` and the bank's `:business` property, which is ambiguous once a bank has more than one BIC. It makes the BIC exactly as portable as `:aba:rtn` already is. ### Bank identifier types All three now match 3.x exactly: | type | before | after | |------|--------|-------| | `econ:bank:aba:rtn` | `[0-9]{9}` | `^[0-9]{9}$` | | `econ:bank:iban` | `[A-Z]{2}[0-9]{2}[a-zA-Z0-9]{1,30}` | `^[A-Z]{2}[0-9]{2}[a-zA-Z0-9]{1,30}$` | | `econ:bank:swift:bic` | `[A-Z]{6}[A-Z0-9]{5}` | `^[A-Z]{6}[A-Z0-9]{2}([A-Z0-9]{3})?$` | The BIC change is the one motivating this work: the 8 character ISO 9362 head-office form is entirely normal and was rejected outright, so values such as `BOPIPHMM` and `TRWIBEB1` raised `BadTypeValu`. The other two are pure narrowings, aligned so the two model versions agree on what these identifiers are. ## Model revision 0.2.37 `Str.norm` calls `self.regex.match()` (`synapse/lib/types.py:1862`), which anchors only at the start. All three previous patterns therefore accepted trailing characters after an otherwise valid value -- `DEUTDEFFXXXXX`, `1234567890` and `GB29NWBK60161331926819!!` all normalized. Anchoring the end narrows released types, so data at rest may no longer re-norm. None of these values has a derivable correct replacement, so `ModelMigration_0_2_37` removes the node rather than moving it. Each removal is logged at WARNING with the node's iden, and the full record -- properties, tags, nodedata, both edge directions, and the referring nodes -- goes into the `model_0_2_37:nodes` queue so an operator can rebuild it. The queue is created lazily on first write, so a Cortex with nothing to migrate never gets one. It is smaller than the two existing primary-value migrations because no node's primary value changes: every *valid* value the old patterns accepted is still accepted. `moveNode` is never called, which also leaves the base class's recursive comp-form re-key path dormant. `econ:bank:account` does have properties typed `econ:bank:aba:rtn` and `econ:bank:iban`, so inbound references are cleared. Neither property is read-only, so `removeNode()` deletes the property and the account survives rather than being removed with the identifier. A read-only referrer is removed and queued under its own record instead; the regression cortex covers that arm with a `meta:seen`. ### Repair library `$lib.model.migration.s.model_0_2_37` provides `listNodes()`, `printNode()` and `repairNode()`, matching the existing `model_0_2_31` library. This follows `_0_2_31` rather than `_0_2_35`. The distinction is not whether a migration writes a queue but whether the queue is ever expected to be non-empty: both of `_0_2_35`'s `removeNode()` calls sit inside `except Exception: # pragma: no cover` and its normal path is `moveNode()`, so its queue is expected to stay empty and no test asserts it. Removal is this migration's only outcome. Repair matters more here than the node contents suggest. The `econ:bank:account` properties which pointed at a removed identifier are cleared by the migration, and `repairNode()` re-points them at the corrected value -- rebuilding the node by hand would restore the node but silently leave the account without its identifier. `listNodes()`'s `form` filter, inert on `_0_2_31`'s single-form queue, is useful on a three-form one. `printNode()` marks read-only references, because `repairNode()` cannot re-point a read-only property -- the referring node was removed and queued under its own entry, so restoring it is a second call that was previously undiscoverable. This improves the `_0_2_31` library too, which has the same two-step repair. `LibModelMigrationsQueue` was extracted so both libraries share the implementation and differ only by `queuename`. The three `_meth` entry points are declared on each subclass rather than inherited: `registry.registerLib()` stamps `_storm_funcpath` onto the function object, so with inherited methods both libraries resolve to the same object and the second registration relabels the first -- which would have renamed the released `_0_2_31` library in its own error messages. ## Tests - `test_model_risk.py` -- `:ext:id`/`:ext:ids` with strip applied, the alts resolve `risk:threat=({"ext:id": <peer id>})`, and `:actor` set to both a `risk:threat` and an `ou:org` with pivots to each - `test_model_economic.py` -- BIC 8 character accepted, 7/9/10/13 rejected; RTN and IBAN trailing-character cases rejected; `:swift:bic` set on an account with a value assertion and a `-> econ:bank:swift:bic` pivot - `test_lib_modelrev.py` -- the repair library (form-filtered listing, `printNode` showing the referring property and flagging read-only refs, `repairNode` restoring both the node and the account property, and the queue-absent messages), plus `test_modelrev_0_2_37` in the usual two-block form. The quarantined BIC spans two layers, so the record carries a sode and nodedata for each and the migration's missing-`valu` guard is exercised: `maxvers=(0, 2, 36)` asserting the pre-migration state, then a full boot asserting the survivors keep their properties, all six invalid values are gone (one from a forked view), the referencing account survives with `:number` intact and both identifier properties cleared, and each queued record carries the referring node plus the property which pointed at it `test_lib_modelrev.py` is 36 passed / 0 skipped with `SYN_REGRESSION_REPO` set, and every other regression cortex migrates cleanly through the new step. The no-invalid-nodes path -- what almost every real Cortex will take -- is covered by the existing `model-0.2.36` fixture. ## Changelog A `migration` fragment covering the removals, the cleared account properties and the read-only cascade, plus a `feat` fragment for the new Storm library. No fragments for the model changes themselves -- `ModelDiffer` picks up all of them against the v2.250.0 model ref (both new properties, the `ext:id` `alts` addkey, and all three types with old and new regexes), and they are covered by the generated model update doc. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs --------- Co-authored-by: Claude Opus 5 (1M context) <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
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.
Regression cortex for the Synapse 0.2.37 model revision, which migrates
econ:bank:aba:rtn,econ:bank:ibanandecon:bank:swift:bicnodes whose stored value is no longer valid.This must merge before vertexproject/synapse#5008 can pass CI. That PR's
test_modelrev_0_2_37callsgetRegrCore('model-0.2.37'), and.circleci/config.yml:169clones this repo from its default branch with no ref pin.Why the migration exists
All three types are anchored only at the start, because 2.x
Str.normcallsself.regex.match(). Synapse #5008 adopts the 3.x patterns, which anchor both ends:econ:bank:aba:rtn[0-9]{9}^[0-9]{9}$econ:bank:iban[A-Z]{2}[0-9]{2}[a-zA-Z0-9]{1,30}^[A-Z]{2}[0-9]{2}[a-zA-Z0-9]{1,30}$econ:bank:swift:bic[A-Z]{6}[A-Z0-9]{5}^[A-Z]{6}[A-Z0-9]{2}([A-Z0-9]{3})?$The BIC change also widens, so the 8 character ISO 9362 head-office form is finally accepted. The other two are pure narrowings. Either way data at rest may no longer re-norm, so a migration is required.
Contents
cortexes/model-0.2.37, built byscripts/gen-0.2.37.py:1234567890aba:rtnecon:bank:accountGB29NWBK60161331926819!!iban123456789junkaba:rtnDEUTDEFFXXXXXswift:bicrefsedgeTRWIBEB1XXXjunkswift:bicBNPAFRPPXXXjunkswift:bicfork00meta:seenon the IBANmeta:seen987654321aba:rtnVV09WootWootibanDEUTDEFFXXXswift:bic:businessThe
meta:seenmatters too.meta:seen:nodeis a read-only ndef, soremoveNode()removes the referring node rather than clearing a property -- a separate, data-destroying arm from the account case. Themeta:sourceit points at must survive, since the cascade follows references inbound to the removed node, not outbound from it.The account matters.
econ:bank:accounthas properties typed for bothaba:rtnandiban, so this is the first cortex to exerciseremoveNode()'s inbound-reference path -- no property in the model is typedecon:bank:swift:bic. Neither referring property is read-only, so the account must survive with those properties deleted rather than being removed alongside the identifiers.Generation
Run against the pre-migration model:
The generator guards on two things, not one.
gen-0.2.36.pychecks onlys_version.version <= maxver; that is not sufficient here, because a 2.250.0 checkout which already carries the model change would pass a version check while producing a cortex with no invalid data in it -- a fixture that silently tests nothing. So it also assertss_modelrev.maxvers < (0, 2, 37).Verification
With
SYN_REGRESSION_REPOpointed at this branch,test_lib_modelrev.pyis 36 passed, 0 skipped against synapse#5008, which also exercises the$lib.model.migration.s.model_0_2_37repair library added there. Every other cortex in this repo also migrates cleanly through the new step, so no committed fixture holds data these narrowings would destroy.model-0.2.36additionally covers the no-invalid-nodes path.🤖 Generated with Claude Code
https://claude.ai/code/session_01HNhaKFd1Qi8Mkbn7AfbhPs