fix(odoo-spo): read selection_add + merge across classes (codex #530)

Odoo addons extend an existing Selection field's domain via
`_inherit` + `fields.Selection(selection_add=[('reviewed','Reviewed')])`
WITHOUT redeclaring the base list. `_extract_selection_values` only read
the base `selection=` arg, so the emitted `selection_value` set was
incomplete and a downstream `ASSERT $value IN [...]` would reject records
in legal extension-added states.

Two fixes:

1. `_extract_selection_values` now reads BOTH the base list (positional
   arg 0 or `selection=`) AND `selection_add=`, unioning base-first with
   order-preserving dedup (factored into `_selection_tuple_keys`).

2. `_scan_file` binding MERGES selection keys per (model, field) instead
   of last-write-wins — the base declaration and the `selection_add`
   extension live in DIFFERENT classes, so the field's value set must
   accumulate across scans, not overwrite.

Tests (53 -> 57):
  - selection_add kwarg alone -> added keys
  - base + selection_add -> union, base first
  - selection_add dedups against base
  - cross-class merge: base in _name class + selection_add in _inherit
    class -> accumulated [draft, posted, reviewed]

  python3 -m unittest tests.test_spo_enrich : 57/57 OK

Author: claude

tools/odoo-blueprint-extractor/odoo_blueprint_extractor/spo_enrich.py

@@ -72,9 +72,13 @@
 
     Enables the consumer to lower a Selection field to
     `DEFINE FIELD state … ASSERT $value IN ['draft','posted','cancel']`.
-    Dynamic selections (`selection='_compute_states'`, a bare Name constant,
-    or `related=`) are skipped — their values aren't statically knowable.
-    Source order is preserved. Selection fields bind to the same
+    Both the base list (positional arg 0 or `selection=`) AND a
+    `_inherit` addon's `selection_add=[…]` extension are read and unioned —
+    an extension's keys MERGE onto the field's accumulated set across
+    classes, so the `ASSERT` set includes legal extension-added states (codex
+    #530). Dynamic selections (`selection='_compute_states'`, a bare Name
+    constant, or `related=`) are skipped — their values aren't statically
+    knowable. Source order is preserved. Selection fields bind to the same
     `model_names` as relational fields (`_name`, else `_inherit[0]`).
 
     This is *in addition* to the existing shallow relation read
@@ -162,37 +166,49 @@
 _VALIDATION_KINDS = ("presence", "uniqueness", "range", "format", "lookup")
 
 
+def _selection_tuple_keys(node: Optional[ast.expr], out: List[str], seen: Set[str]) -> None:
+    """Append the first-element string key of each 2-tuple in `node` (a list
+    or tuple of `('key','Label')` pairs) to `out`, de-duplicating against
+    `seen`, preserving order. Non-list/tuple `node` (a str method-ref, a bare
+    Name constant, `related=`) contributes nothing — those aren't static."""
+    if not isinstance(node, (ast.List, ast.Tuple)):
+        return
+    for elt in node.elts:
+        if not isinstance(elt, (ast.Tuple, ast.List)) or len(elt.elts) < 1:
+            continue
+        key = _const_str(elt.elts[0])
+        if key is not None and key not in seen:
+            seen.add(key)
+            out.append(key)
+
+
 def _extract_selection_values(call: ast.Call) -> List[str]:
-    """Pull the value KEYS from a `fields.Selection([('k','Label'), …])`
-    declaration. The selection list is the first positional arg OR the
-    `selection=` kwarg; each entry is a 2-tuple whose first element is the
-    stored key.
-
-    Returns the keys in source order, de-duplicated (first occurrence wins).
-    Returns `[]` for dynamic selections — a bare Name (constant reference),
-    a string (compute-method name), `related=`, or any entry whose first
-    element isn't a string constant. Those values aren't statically known.
+    """Pull the value KEYS from a `fields.Selection(...)` declaration.
+
+    Reads BOTH the base list — first positional arg OR `selection=` kwarg —
+    AND the `selection_add=` kwarg (the Odoo idiom for an `_inherit` addon
+    that *extends* an existing Selection field's domain without redeclaring
+    it). Keys from both are unioned, base first, in source order, de-duped.
+
+    Returns `[]` when neither source is a static list of 2-tuples — a dynamic
+    `selection='_compute_x'` (str), a bare Name constant, or `related=`.
+    A pure `selection_add=` extension (no base list) yields just the added
+    keys, which the caller MERGES onto the field's accumulated set.
     """
-    sel: Optional[ast.expr] = None
+    base: Optional[ast.expr] = None
+    add: Optional[ast.expr] = None
     for kw in call.keywords:
         if kw.arg == "selection":
-            sel = kw.value
-            break
-    if sel is None and call.args:
-        sel = call.args[0]
-    if not isinstance(sel, (ast.List, ast.Tuple)):
-        # `selection='_compute_x'` (str), a Name constant, or related= → skip.
-        return []
+            base = kw.value
+        elif kw.arg == "selection_add":
+            add = kw.value
+    if base is None and call.args:
+        base = call.args[0]
 
     out: List[str] = []
     seen: Set[str] = set()
-    for elt in sel.elts:
-        if not isinstance(elt, (ast.Tuple, ast.List)) or len(elt.elts) < 1:
-            continue
-        key = _const_str(elt.elts[0])
-        if key is not None and key not in seen:
-            seen.add(key)
-            out.append(key)
+    _selection_tuple_keys(base, out, seen)
+    _selection_tuple_keys(add, out, seen)
     return out
 
 
@@ -449,9 +465,15 @@ def _scan_file(
                 relmap[(mu, field_name)] = (comodel, inverse)
             if selections is not None:
                 for field_name, vals in local_selections.items():
-                    # Last write wins (a later reopening that redeclares the
-                    # Selection with a fuller value list supersedes).
-                    selections[(mu, field_name)] = vals
+                    # MERGE, not overwrite: the base `selection=` declaration
+                    # and any `_inherit` addon's `selection_add=` extensions
+                    # live in DIFFERENT classes, so a given (model, field)
+                    # accumulates across scans (codex #530). Order-preserving,
+                    # de-duplicated union.
+                    acc = selections.setdefault((mu, field_name), [])
+                    for v in vals:
+                        if v not in acc:
+                            acc.append(v)
 
         # ── P1b: inherits_from edges ─────────────────────────────────────
         # Only when the class declares a NEW model (_name is set). An

tools/odoo-blueprint-extractor/tests/test_spo_enrich.py

@@ -655,6 +655,27 @@ def test_non_tuple_entries_skipped(self):
         vals = _classify_sel("fields.Selection([('a','A'), 'bogus', ('b','B')])")
         self.assertEqual(vals, ["a", "b"])
 
+    # codex #530 — selection_add (the _inherit extension idiom)
+    def test_selection_add_kwarg_alone(self):
+        # A pure extension: `selection_add=` with no base list → the added keys.
+        vals = _classify_sel(
+            "fields.Selection(selection_add=[('reviewed','Reviewed'),('void','Void')])"
+        )
+        self.assertEqual(vals, ["reviewed", "void"])
+
+    def test_selection_base_plus_add_union_base_first(self):
+        vals = _classify_sel(
+            "fields.Selection(selection=[('draft','Draft'),('posted','Posted')], "
+            "selection_add=[('reviewed','Reviewed')])"
+        )
+        self.assertEqual(vals, ["draft", "posted", "reviewed"])
+
+    def test_selection_add_dedups_against_base(self):
+        vals = _classify_sel(
+            "fields.Selection(selection=[('a','A')], selection_add=[('a','A2'),('b','B')])"
+        )
+        self.assertEqual(vals, ["a", "b"])
+
 
 class TestSelectionScan(unittest.TestCase):
     """`_scan_file` binds Selection values to model_names (per #525 rule)."""
@@ -683,6 +704,24 @@ def test_dynamic_selection_field_records_nothing(self):
         )
         self.assertEqual(sel, {})
 
+    def test_selection_add_merges_across_classes(self):
+        # codex #530: base declaration in the _name class + a selection_add
+        # extension in a separate _inherit class must ACCUMULATE, not
+        # overwrite — else the ASSERT $value IN [...] rejects legal added
+        # states.
+        sel = _scan_sel(
+            "class Base:\n"
+            "    _name = 'account.move'\n"
+            "    state = fields.Selection([('draft','Draft'),('posted','Posted')])\n"
+            "\n"
+            "class Ext:\n"
+            "    _inherit = 'account.move'\n"
+            "    state = fields.Selection(selection_add=[('reviewed','Reviewed')])\n"
+        )
+        self.assertEqual(
+            sel, {("account_move", "state"): ["draft", "posted", "reviewed"]}
+        )
+
 
 class TestP3SelectionValueEmission(unittest.TestCase):
     def _triples(self, field_iri="odoo:account_move.state"):