kris/afv-library
1
0
Fork 0
mirror of https://github.com/forcedotcom/afv-library.git synced 2026-08-09 00:42:46 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix: address PRizm review findings from internal port PR @W-22196528@

Browse Source 
Addresses 4 critical findings surfaced during PRizm code review of the
internal plugin port of this skill (internal PR #19). Applying the same
fixes here keeps the external canonical source and the internal port in
sync.

1. Test cleanup discipline: switch `test_invalid_json_raises` from
   `try/finally` to `self.addCleanup(p.unlink, missing_ok=True)` — the
   unittest-idiomatic way to guarantee temp-file cleanup regardless of
   how the test exits.

2. `floor()` description in bdt-function-catalog.md: the old row was
   self-contradictory ("toward zero" AND "toward next integer up" in
   the same cell). Replace with a single coherent definition: rounds
   toward negative infinity; for negatives rounds away from zero
   (e.g., `floor(-2.3) = -3`).

3. Split-node documentation in bdt-node-catalog.md: the old doc claimed
   `split` routes rows into downstream branches via `branches[]` with
   per-branch predicates. That is not the canonical schema. Per
   `SplitParametersInputRepresentation` in core-262-public, `split` is
   a string-splitting operation: one `sourceField` + `delimiter` →
   N `targetFields` (one row in, one row out; columns added). Rewrote
   the section with the correct parameters, lineage effect, gotchas,
   and a canonical example. Row-routing belongs in `filter` nodes.

4. Sample `assets/sample_bdts/append_and_split.json`: the old sample
   used the invented `branches[]` shape AND routed the same split into
   two downstream outputs that each expected different rows — which is
   not how `split` works. Rewrote the sample so:
     - `appendV2` unions two order sources (unchanged intent).
     - `split` uses canonical `{sourceField, delimiter, targetFields}`
       splitting `CustomerFullName__c` into first + last name columns.
     - One downstream output consumes the new columns (removes the
       fake two-branch fan-out).

Tests: 92/92 passing. Sample parses and runs through `bdt_analyze.py
summary` cleanly (5 nodes: 2 load + 1 appendV2 + 1 split + 1 outputD360).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gaurav Bajpai 2026-04-24 10:19:36 +05:30
parent fc4d14bce0
commit db1aca7221
No known key found for this signature in database
GPG Key ID: 4EE015E4DD141C47
4 changed files with 60 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
skills/explaining-batch-data-transform/assets/sample_bdts/append_and_split.json
View File

@ -6,7 +6,7 @@
"sources": [], "sources": [],
"parameters": { "parameters": {
"dataset": {"name": "WebOrders__dlo", "type": "dataLakeObject"}, "dataset": {"name": "WebOrders__dlo", "type": "dataLakeObject"},
"fields": ["OrderId__c", "Amount__c", "Channel__c"], "fields": ["OrderId__c", "Amount__c", "Channel__c", "CustomerFullName__c"],
"sampleDetails": {"type": "TopN", "sortBy": []} "sampleDetails": {"type": "TopN", "sortBy": []}
} }
}, },
@ -15,7 +15,7 @@
"sources": [], "sources": [],
"parameters": { "parameters": {
"dataset": {"name": "StoreOrders__dlo", "type": "dataLakeObject"}, "dataset": {"name": "StoreOrders__dlo", "type": "dataLakeObject"},
"fields": ["OrderId__c", "Amount__c", "Channel__c"], "fields": ["OrderId__c", "Amount__c", "Channel__c", "CustomerFullName__c"],
"sampleDetails": {"type": "TopN", "sortBy": []} "sampleDetails": {"type": "TopN", "sortBy": []}
} }
}, },
@ -25,66 +25,55 @@
"parameters": { "parameters": {
"allowImplicitDisjointSchema": false, "allowImplicitDisjointSchema": false,
"fieldMappings": [ "fieldMappings": [
{"targetField": "OrderId__c", "sources": [{"node": "LOAD_WEB_ORDERS", "field": "OrderId__c"}, {"node": "LOAD_STORE_ORDERS", "field": "OrderId__c"}]}, {"targetField": "OrderId__c", "sources": [{"node": "LOAD_WEB_ORDERS", "field": "OrderId__c"}, {"node": "LOAD_STORE_ORDERS", "field": "OrderId__c"}]},
{"targetField": "Amount__c", "sources": [{"node": "LOAD_WEB_ORDERS", "field": "Amount__c"}, {"node": "LOAD_STORE_ORDERS", "field": "Amount__c"}]}, {"targetField": "Amount__c", "sources": [{"node": "LOAD_WEB_ORDERS", "field": "Amount__c"}, {"node": "LOAD_STORE_ORDERS", "field": "Amount__c"}]},
{"targetField": "Channel__c", "sources": [{"node": "LOAD_WEB_ORDERS", "field": "Channel__c"}, {"node": "LOAD_STORE_ORDERS", "field": "Channel__c"}]} {"targetField": "Channel__c", "sources": [{"node": "LOAD_WEB_ORDERS", "field": "Channel__c"}, {"node": "LOAD_STORE_ORDERS", "field": "Channel__c"}]},
{"targetField": "CustomerFullName__c", "sources": [{"node": "LOAD_WEB_ORDERS", "field": "CustomerFullName__c"}, {"node": "LOAD_STORE_ORDERS", "field": "CustomerFullName__c"}]}
] ]
} }
}, },
"SPLIT_BY_AMOUNT": { "SPLIT_CUSTOMER_NAME": {
"action": "split", "action": "split",
"sources": ["APPEND_ALL_ORDERS"], "sources": ["APPEND_ALL_ORDERS"],
"parameters": { "parameters": {
"branches": [ "sourceField": "CustomerFullName__c",
{"name": "high_value", "expression": "Amount__c >= 1000"}, "delimiter": " ",
{"name": "low_value", "expression": "Amount__c < 1000"} "targetFields": [
{"name": "CustomerFirstName__c", "label": "Customer First Name"},
{"name": "CustomerLastName__c", "label": "Customer Last Name"}
] ]
} }
}, },
"OUTPUT_HIGH": { "OUTPUT_ORDERS": {
"action": "outputD360", "action": "outputD360",
"sources": ["SPLIT_BY_AMOUNT"], "sources": ["SPLIT_CUSTOMER_NAME"],
"parameters": { "parameters": {
"name": "HighValueOrders__dlm", "name": "OrdersWithCustomerName__dlm",
"type": "dataModelObject", "type": "dataModelObject",
"writeMode": "OVERWRITE", "writeMode": "OVERWRITE",
"fieldsMappings": [ "fieldsMappings": [
{"sourceField": "OrderId__c", "targetField": "OrderId__c"}, {"sourceField": "OrderId__c", "targetField": "OrderId__c"},
{"sourceField": "Amount__c", "targetField": "Amount__c"}, {"sourceField": "Amount__c", "targetField": "Amount__c"},
{"sourceField": "Channel__c", "targetField": "Channel__c"} {"sourceField": "Channel__c", "targetField": "Channel__c"},
] {"sourceField": "CustomerFirstName__c", "targetField": "FirstName__c"},
} {"sourceField": "CustomerLastName__c", "targetField": "LastName__c"}
},
"OUTPUT_LOW": {
"action": "outputD360",
"sources": ["SPLIT_BY_AMOUNT"],
"parameters": {
"name": "LowValueOrders__dlm",
"type": "dataModelObject",
"writeMode": "OVERWRITE",
"fieldsMappings": [
{"sourceField": "OrderId__c", "targetField": "OrderId__c"},
{"sourceField": "Amount__c", "targetField": "Amount__c"},
{"sourceField": "Channel__c", "targetField": "Channel__c"}
] ]
} }
} }
}, },
"ui": { "ui": {
"nodes": { "nodes": {
"LOAD_WEB_ORDERS": {"label": "Web Orders", "type": "LOAD_DATASET", "top": 100, "left": 100}, "LOAD_WEB_ORDERS": {"label": "Web Orders", "type": "LOAD_DATASET", "top": 100, "left": 100},
"LOAD_STORE_ORDERS": {"label": "Store Orders", "type": "LOAD_DATASET", "top": 260, "left": 100}, "LOAD_STORE_ORDERS": {"label": "Store Orders", "type": "LOAD_DATASET", "top": 260, "left": 100},
"APPEND_ALL_ORDERS": {"label": "Union", "type": "APPEND", "top": 180, "left": 260}, "APPEND_ALL_ORDERS": {"label": "Union", "type": "APPEND", "top": 180, "left": 260},
"SPLIT_BY_AMOUNT": {"label": "Split by $", "type": "SPLIT", "top": 180, "left": 420}, "SPLIT_CUSTOMER_NAME": {"label": "Split full name", "type": "SPLIT", "top": 180, "left": 420},
"OUTPUT_HIGH": {"label": "High value", "type": "OUTPUT", "top": 100, "left": 580}, "OUTPUT_ORDERS": {"label": "Orders with name", "type": "OUTPUT", "top": 180, "left": 580}
"OUTPUT_LOW": {"label": "Low value", "type": "OUTPUT", "top": 260, "left": 580}
}, },
"connectors": [ "connectors": [
{"source": "LOAD_WEB_ORDERS", "target": "APPEND_ALL_ORDERS"}, {"source": "LOAD_WEB_ORDERS", "target": "APPEND_ALL_ORDERS"},
{"source": "LOAD_STORE_ORDERS", "target": "APPEND_ALL_ORDERS"}, {"source": "LOAD_STORE_ORDERS", "target": "APPEND_ALL_ORDERS"},
{"source": "APPEND_ALL_ORDERS", "target": "SPLIT_BY_AMOUNT"}, {"source": "APPEND_ALL_ORDERS", "target": "SPLIT_CUSTOMER_NAME"},
{"source": "SPLIT_BY_AMOUNT", "target": "OUTPUT_HIGH"}, {"source": "SPLIT_CUSTOMER_NAME", "target": "OUTPUT_ORDERS"}
{"source": "SPLIT_BY_AMOUNT", "target": "OUTPUT_LOW"}
] ]
} }
} }

2
skills/explaining-batch-data-transform/references/bdt-function-catalog.md
View File

@ -34,7 +34,7 @@
|---|---|---| |---|---|---|
| `abs(n)` | number → number | Absolute value (strips sign). | | `abs(n)` | number → number | Absolute value (strips sign). |
| `ceiling(n)` | number → number | Round up, away from zero for negatives. | | `ceiling(n)` | number → number | Round up, away from zero for negatives. |
| `floor(n)` | number → number | Round down, toward zero for negatives (or toward the next integer up for negatives). | | `floor(n)` | number → number | Round toward negative infinity (down on the number line). For negatives, rounds away from zero (e.g., `floor(-2.3) = -3`). |
| `exp(n)` | number → number | e raised to n. | | `exp(n)` | number → number | e raised to n. |
| `log(base, n)` | number → number | Logarithm of n in the given base. | | `log(base, n)` | number → number | Logarithm of n in the given base. |
| `max(a, b, …)` | number → number | Largest value. | | `max(a, b, …)` | number → number | Largest value. |

33
skills/explaining-batch-data-transform/references/bdt-node-catalog.md
View File

@ -533,23 +533,42 @@ unmapped source field is discarded.
## `action: "split"` — UI: "Split" ## `action: "split"` — UI: "Split"
**Purpose.** Route rows into multiple downstream branches based on per-branch predicates. **Purpose.** Split the value of one source string field into multiple target columns based on a delimiter. One row in, one row out — each row's `sourceField` is split into the named `targetFields`.
**Key parameters** (class `SplitParametersInputRepresentation`): **Key parameters** (class `SplitParametersInputRepresentation`):
| Param | Type | Notes | | Param | Type | Notes |
|---|---|---| |---|---|---|
| `branches` | array of `{ name, expression }` | Per-branch routing predicate. | | `sourceField` | `string` | Name of the field whose value will be split. |
| `delimiter` | `string` | Delimiter used to split the source value. |
| `targetFields` | `{name, label}[]` | One entry per column the split produces. Order matches the left-to-right order of the split parts. |
**Lineage effect.** **Lineage effect.**
- Rows: partitioned across branches (each branch exposes the rows matching its predicate). - Rows: unchanged. Row cardinality is preserved — split does not route rows into branches.
- Columns: same as input. - Columns: adds each `targetFields[i].name` as a new column. The original `sourceField` passes through unchanged.
**Gotchas.** **Gotchas.**
- A split node has one upstream source but *multiple* downstream consumers (each branch is a - `split` is string-splitting, not row-routing. If you need to route rows into multiple branches based on predicates, use `filter` nodes downstream of a common source, not `split`.
logical output). When narrating lineage, mention which branch feeds which downstream node. - If a row's `sourceField` has fewer delimited parts than the `targetFields` length, the remaining target columns are populated with NULL (no error).
- If a row's `sourceField` has more delimited parts than `targetFields` length, the extra parts are discarded.
**Source.** `SplitNodeInputRepresentation` + `SplitParametersInputRepresentation`. **Example.**
```jsonc
{
"action": "split",
"sources": ["LOAD_RAW"],
"parameters": {
"sourceField": "FullName__c",
"delimiter": " ",
"targetFields": [
{"name": "FirstName__c", "label": "First Name"},
{"name": "LastName__c", "label": "Last Name"}
]
}
}
```
**Source.** `SplitNodeInputRepresentation` + `SplitParametersInputRepresentation` + `NameLabelInputRepresentation`.
--- ---

10
skills/explaining-batch-data-transform/tests/test_bdt_analyze.py
View File

@ -39,12 +39,10 @@ class TestBadInput(unittest.TestCase):
def test_invalid_json_raises(self): def test_invalid_json_raises(self):
p = FIXTURES / "_tmp_invalid.json" p = FIXTURES / "_tmp_invalid.json"
p.write_text("{not valid json") p.write_text("{not valid json")
try: self.addCleanup(p.unlink, missing_ok=True)
with self.assertRaises(bdt_analyze.BdtInputError) as cm: with self.assertRaises(bdt_analyze.BdtInputError) as cm:
bdt_analyze.DataTransform.from_path(p) bdt_analyze.DataTransform.from_path(p)
self.assertIn("Invalid JSON", str(cm.exception)) self.assertIn("Invalid JSON", str(cm.exception))
finally:
p.unlink(missing_ok=True)
def test_missing_nodes_raises(self): def test_missing_nodes_raises(self):
with self.assertRaises(bdt_analyze.BdtInputError): with self.assertRaises(bdt_analyze.BdtInputError):