fix(tm1): validate results-CSV header to prevent measure drift (#169)

CHANGELOG.md

@@ -4,6 +4,19 @@ All notable changes to RushTI are documented in this file.
 
 ## [Unreleased]
 
+- **Fixed: results written to the wrong measures after an in-place upgrade**
+  (closes #169). `}rushti.load.results` maps CSV columns to TI variables
+  positionally. A cube/process built by a pre-2.3.0 rushti (before the `chore`
+  column) that was then upgraded without re-running `rushti build` would
+  silently shift every value from `chore`/`parameters` onward into the next
+  measure. The loader now reads the CSV header row (`asciiHeaderRecords=0`) and
+  validates each column against the variable it feeds in the metadata tab: on
+  mismatch it logs the offending columns and `ProcessQuit`s instead of writing
+  scrambled data. **After upgrading rushti you must re-run
+  `rushti build --tm1-instance X`** to refresh the process (and additively add
+  any new measure element) — the guard only turns future drift into a clear
+  error, it does not repair an already-stale process. Existing rows written
+  under the wrong measures should be cleared and the workflow re-run.
 - **Added: `--config PATH` CLI flag** (closes #164). Overrides the location of
   `config.ini` (TM1 connection parameters) for a single invocation, on every
   TM1-connecting command (`run`, `build`, `tasks …`, `resume`). Precedence:

docs/advanced/migration-guide.md

@@ -171,6 +171,9 @@ Task file validation failed:
 
 You don't need a separate command — bare `rushti build` does the right thing. CI/CD pipelines that run `rushti build` on every release continue to work across version upgrades without manual intervention.
 
+!!! warning "Re-run `rushti build` after every upgrade"
+    `}rushti.load.results` maps result-CSV columns to its variables **by position**. If a release adds a measure column (as 2.3.0 did with `chore`) and you upgrade the Python package **without** re-running `rushti build`, the stale process reads each column into the wrong variable and writes values to the next measure over (issue [#169](https://github.com/cubewise-code/rushti/issues/169)). The loader now validates the CSV header and fails with a clear error instead of silently scrambling data — but the fix for a stale deployment is still to **re-run `rushti build --tm1-instance <inst>`**. If you already have scrambled rows, clear them and re-run the workflow.
+
 ---
 
 ## Step-by-Step Migration

src/rushti/tm1_objects.py

@@ -150,6 +150,8 @@
 nErrors             = 0;
 nMetadataRecordCount= 0;
 nDataRecordCount    = 0;
+nHeaderChecked      = 0;
+nDataHeaderChecked  = 0;
 
 ### Process Specific Constants
 cFileSrc            = pSourceFile;
@@ -220,7 +222,9 @@
 If( nErrors = 0 );
     DatasourceType          = 'CHARACTERDELIMITED';
     DatasourceNameForServer = cFileSrc;
-    DatasourceASCIIHeaderRecords = 1;
+    # 0 (not 1): the header row is consumed as the first record so the
+    # metadata tab can validate it against the expected columns (issue #169).
+    DatasourceASCIIHeaderRecords = 0;
     DatasourceASCIIDelimiter=',';
     DatasourceASCIIDecimalSeparator='.';
     DatasourceASCIIThousandSeparator='';
@@ -290,6 +294,8 @@
 nErrors             = 0;
 nMetadataRecordCount= 0;
 nDataRecordCount    = 0;
+nHeaderChecked      = 0;
+nDataHeaderChecked  = 0;
 
 ### Process Specific Constants
 cFileSrc            = pSourceFile;
@@ -356,7 +362,9 @@
 If( nErrors = 0 );
     DatasourceType          = 'CHARACTERDELIMITED';
     DatasourceNameForServer = cFileSrc;
-    DatasourceASCIIHeaderRecords = 1;
+    # 0 (not 1): the header row is consumed as the first record so the
+    # metadata tab can validate it against the expected columns (issue #169).
+    DatasourceASCIIHeaderRecords = 0;
     DatasourceASCIIDelimiter=',';
     DatasourceASCIIDecimalSeparator='.';
     DatasourceASCIIThousandSeparator='';
@@ -366,10 +374,17 @@
 #################################################################################################
 """
 
-PROCESS_METADATA = r"""#****Begin: Generated Statements***
+# Marker pair TM1 Architect uses to inject auto-generated variable/parameter
+# code at the top of a tab. Kept as the first lines of every generated
+# procedure so the process round-trips cleanly if later opened in Architect.
+_GENERATED_STATEMENTS_MARKER = """#****Begin: Generated Statements***
 #****End: Generated Statements****
+"""
 
-If( pLogOutput >= 1 );
+# Body of the metadata tab. The header-validation preamble is generated from
+# PROCESS_VARIABLES and prepended in ``build_metadata_procedure`` so it stays
+# in lockstep with the CSV column contract (see issue #169).
+_PROCESS_METADATA_BODY = r"""If( pLogOutput >= 1 );
    nMetadataRecordCount = nMetadataRecordCount + 1;
 EndIf;
 
@@ -388,10 +403,9 @@
   DimensionElementInsertDirect(pRunId_Dim, '', vrun_id, 'N');
 endif;"""
 
-PROCESS_DATA = r"""#****Begin: Generated Statements***
-#****End: Generated Statements****
-
-If( pLogOutput >= 1 );
+# Body of the data tab. The header-skip preamble is prepended in
+# ``build_data_procedure`` so the CSV header row is not written to the cube.
+_PROCESS_DATA_BODY = r"""If( pLogOutput >= 1 );
    nDataRecordCount = nDataRecordCount + 1;
 EndIf;
 
@@ -544,8 +558,9 @@
 # declaration list. Must match the column order produced by
 # ``rushti.tm1_integration.upload_results_to_tm1`` (which inserts
 # ``workflow`` and ``run_id`` ahead of the columns built by
-# ``build_results_dataframe``). Mismatched ordering silently scrambles
-# the loaded payload.
+# ``build_results_dataframe``). Mismatched ordering scrambles the loaded
+# payload; the metadata-tab header check generated from this list turns a
+# stale-process mismatch into a hard error instead (see issue #169).
 PROCESS_VARIABLES = [
     "vworkflow",
     "vrun_id",
@@ -571,12 +586,73 @@
     "vsucceed_on_minor_errors",
 ]
 
+
+def build_metadata_procedure() -> str:
+    """Assemble the ``}rushti.load.results`` metadata tab.
+
+    Prepends a header-validation preamble generated from ``PROCESS_VARIABLES``
+    so it stays in lockstep with the CSV column contract. The loader maps CSV
+    columns to variables positionally; with ``asciiHeaderRecords=0`` the CSV
+    header row is the first source record, so validating each column name
+    against the variable it feeds makes a process left stale by an in-place
+    rushti upgrade fail loudly instead of silently writing values to the wrong
+    measures (issue #169). Re-run ``rushti build`` to refresh a stale process.
+    """
+    checks = "\n".join(
+        f"    If( {var} @<> '{var[1:]}' );\n"
+        f"        sHeaderError = sHeaderError | ' {var[1:]}';\n"
+        f"    EndIf;"
+        for var in PROCESS_VARIABLES
+    )
+    preamble = f"""### Validate the CSV header against the columns this process expects.
+### The loader maps columns to variables positionally, so a process built by
+### an older rushti scrambles measures (issue #169). Fail loudly on mismatch
+### and skip the header so it is not loaded as data.
+If( nHeaderChecked = 0 );
+    nHeaderChecked = 1;
+    sHeaderError = '';
+{checks}
+    If( Trim( sHeaderError ) @<> '' );
+        nErrors = nErrors + 1;
+        sMessage = Expand( 'Results CSV header does not match the columns }}rushti.load.results expects (mismatched:%sHeaderError% ). This process is out of date with the rushti version that produced the file - re-run: rushti build --tm1-instance <instance>.' );
+        LogOutput( cMsgErrorLevel, Expand( cMsgErrorContent ) );
+        ProcessQuit;
+    EndIf;
+    ItemSkip;
+EndIf;
+
+"""
+    return _GENERATED_STATEMENTS_MARKER + "\n" + preamble + _PROCESS_METADATA_BODY
+
+
+def build_data_procedure() -> str:
+    """Assemble the ``}rushti.load.results`` data tab.
+
+    Skips the CSV header row (validated in the metadata tab) so it is not
+    written to the cube, then delegates to the static body. See issue #169.
+    """
+    preamble = """### Skip the CSV header row (asciiHeaderRecords=0; validated in the
+### metadata tab). See issue #169.
+If( nDataHeaderChecked = 0 );
+    nDataHeaderChecked = 1;
+    ItemSkip;
+EndIf;
+
+"""
+    return _GENERATED_STATEMENTS_MARKER + "\n" + preamble + _PROCESS_DATA_BODY
+
+
+PROCESS_METADATA = build_metadata_procedure()
+PROCESS_DATA = build_data_procedure()
+
 PROCESS_DATASOURCE = {
     "Type": "ASCII",
     "asciiDecimalSeparator": ".",
     "asciiDelimiterChar": ",",
     "asciiDelimiterType": "Character",
-    "asciiHeaderRecords": 1,
+    # 0: the header row is validated (not skipped) by the metadata tab; see
+    # the DatasourceASCIIHeaderRecords note in the prolog and issue #169.
+    "asciiHeaderRecords": 0,
     "asciiQuoteCharacter": '"',
     "asciiThousandSeparator": ",",
     "dataSourceNameForClient": "",

tests/unit/test_tm1_build.py

@@ -10,6 +10,12 @@
 from rushti.tm1_objects import (
     MEASURE_ELEMENTS,
     MEASURE_ATTRIBUTES,
+    PROCESS_DATA,
+    PROCESS_DATASOURCE,
+    PROCESS_METADATA,
+    PROCESS_PROLOG_V11,
+    PROCESS_PROLOG_V12,
+    PROCESS_VARIABLES,
     TASK_ID_ELEMENT_COUNT,
     WORKFLOW_SEED_ELEMENTS,
     RUN_ID_SEED_ELEMENTS,
@@ -101,6 +107,50 @@ def test_sample_data_records_have_required_keys(self):
                 self.assertIn("value", record)
 
 
+class TestLoadResultsHeaderValidation(unittest.TestCase):
+    """Guards the }rushti.load.results header-validation contract (issue #169).
+
+    The loader maps CSV columns to process variables positionally. A process
+    built by an older rushti silently wrote values to the wrong measures after
+    a column was added. These tests lock in the metadata-tab header check that
+    turns such drift into a hard error.
+    """
+
+    def test_datasource_reads_header_as_data(self):
+        """Header row must reach the tabs (records=0) so it can be validated."""
+        self.assertEqual(PROCESS_DATASOURCE["asciiHeaderRecords"], 0)
+
+    def test_prologs_read_header_as_data_and_seed_skip_flags(self):
+        """Both prolog variants set records=0 and init the header-skip flags."""
+        for prolog in (PROCESS_PROLOG_V11, PROCESS_PROLOG_V12):
+            self.assertIn("DatasourceASCIIHeaderRecords = 0;", prolog)
+            self.assertNotIn("DatasourceASCIIHeaderRecords = 1;", prolog)
+            self.assertIn("nHeaderChecked", prolog)
+            self.assertIn("nDataHeaderChecked", prolog)
+
+    def test_metadata_validates_every_expected_column(self):
+        """Every CSV column is checked against the variable that feeds it.
+
+        Generated from PROCESS_VARIABLES, so adding a column without updating
+        the loader keeps the check and the CSV contract in lockstep.
+        """
+        for var in PROCESS_VARIABLES:
+            column = var[1:]  # strip the leading 'v'
+            self.assertIn(f"If( {var} @<> '{column}' );", PROCESS_METADATA)
+
+    def test_metadata_fails_loud_and_skips_header(self):
+        """On mismatch the process quits; the header itself is never loaded."""
+        self.assertIn("If( nHeaderChecked = 0 );", PROCESS_METADATA)
+        self.assertIn("ProcessQuit;", PROCESS_METADATA)
+        self.assertIn("ItemSkip;", PROCESS_METADATA)
+        self.assertIn("rushti build --tm1-instance", PROCESS_METADATA)
+
+    def test_data_tab_skips_header_row(self):
+        """The data tab drops the header record so it is not written to cube."""
+        self.assertIn("If( nDataHeaderChecked = 0 );", PROCESS_DATA)
+        self.assertIn("ItemSkip;", PROCESS_DATA)
+
+
 class TestDimensionBuilders(unittest.TestCase):
     """Tests for dimension builder functions."""