Skip to content

Commit 247aaa3

Browse files
djwaldoclaude
andcommitted
fix(thoughtspot): keep formulas that no column surfaces, and keep them hidden
A model's `formulas[]` may hold entries no `columns[]` entry references. Those are HIDDEN in the ThoughtSpot UI, and the usual reason to write one is that another formula uses it -- a date-parameter model is the common case, where `_startDate`/`_endDate` compute a window the visible formulas then apply. The converter walks `columns[]`, so a formula no column surfaces was never visited and was dropped outright. Even the unattributed-formula stash could not catch them: that only holds formulas that WERE visited and could not be attributed to a dataset. On the real model that surfaced this, 32 of 41 formulas were lost, and the 9 visible ones referencing them came back with dangling `[formula__startDate]` references. Round-tripping it now: dangling references 4 -> NONE reverse-leg ERRORs 4 -> none (the run exited 1 before) surfacing columns 45 -> 45 unchanged They are stashed with their `id` -- that is what the surviving references name, and re-minting an id from the display name is exactly how the references came to point at nothing -- and restored as `formulas[]` entries with NO surfacing column. Giving them one, as the unattributed-formula path deliberately does, would make a helper the modeller kept private visible to users. Mutation-checked. 936 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 9892bf1 commit 247aaa3

5 files changed

Lines changed: 115 additions & 0 deletions

File tree

‎converters/thoughtspot/docs/vendor-payload.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ Every `custom_extensions` entry this converter writes uses `vendor_name` `THOUGH
5959
| `endpoints_swapped` | Relationship | shadows_derivable | Restored only if its witness companion key still matches the live document's current value; a mismatch means the document changed since the stash was written, so the value is re-derived instead. |
6060
| `referencing_join` | Relationship | shadows_derivable | Restored only if reconstructing it from the live document still agrees with the stashed value (self-verifying — no separate witness key); disagreement re-derives instead. |
6161
| `join_shape` | Relationship | information_only | Restored as-is whenever present — nothing on the Ossie side could have diverged from it. |
62+
| `unsurfaced_formulas` | Model | information_only | Restored as-is whenever present — nothing on the Ossie side could have diverged from it. |
6263
| `unattributed_formulas` | Model | information_only | Restored as-is whenever present — nothing on the Ossie side could have diverged from it. |
6364
| `unrepresentable_joins` | Model | information_only | Restored as-is whenever present — nothing on the Ossie side could have diverged from it. |
6465
| `model_properties` | Model | information_only | Restored as-is whenever present — nothing on the Ossie side could have diverged from it. |

‎converters/thoughtspot/src/ossie_thoughtspot/constants.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,23 @@
9999
#: must agree on the exact spelling and nothing else enforces that.
100100
FIELD_STASH_DB_COLUMN_NAME = "db_column_name"
101101

102+
#: Formulas present in `model.formulas[]` that NO `columns[]` entry surfaces --
103+
#: internal helpers, referenced by other formulas but not exposed to users. A
104+
#: date-parameter model is the common case: `_startDate`, `_endDate` and the
105+
#: like compute a window that the visible formulas then use.
106+
#:
107+
#: They were dropped outright: the converter walks `columns[]`, so a formula no
108+
#: column surfaces is never visited at all, and even the unattributed-formula
109+
#: stash only catches ones that WERE visited and could not be attributed. In one
110+
#: real model that was 32 of 41 formulas -- and the 9 visible ones that
111+
#: referenced them came back with dangling `[formula__startDate]` references, so
112+
#: the emitted document would not import.
113+
#:
114+
#: Stashed with their `id`, because that is what the surviving references name.
115+
#: Restored as `formulas[]` entries with NO surfacing column, which is what they
116+
#: were: giving them one would make an internal helper user-visible.
117+
MODEL_STASH_UNSURFACED_FORMULAS = "unsurfaced_formulas"
118+
102119
#: A surfacing column's `aggregation` that is LOAD-BEARING and has no home in
103120
#: the Ossie metric's own expression, so it is preserved verbatim instead.
104121
#:
@@ -533,6 +550,7 @@ class StashKeyClass(Enum):
533550
RELATIONSHIP_STASH_JOIN_SHAPE: StashKeyClass.INFORMATION_ONLY,
534551

535552
# -- Model scope --
553+
MODEL_STASH_UNSURFACED_FORMULAS: StashKeyClass.INFORMATION_ONLY,
536554
MODEL_STASH_UNATTRIBUTED_FORMULAS: StashKeyClass.INFORMATION_ONLY,
537555
MODEL_STASH_UNREPRESENTABLE_JOINS: StashKeyClass.INFORMATION_ONLY,
538556
MODEL_STASH_MODEL_PROPERTIES: StashKeyClass.INFORMATION_ONLY,

‎converters/thoughtspot/src/ossie_thoughtspot/ossie_to_thoughtspot.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@
104104
MODEL_STASH_MODEL_JOINS_WITH,
105105
MODEL_STASH_MODEL_PROPERTIES,
106106
MODEL_STASH_PARAMETERS,
107+
MODEL_STASH_UNSURFACED_FORMULAS,
107108
MODEL_STASH_UNATTRIBUTED_FORMULAS,
108109
MODEL_STASH_UNREPRESENTABLE_JOINS,
109110
PORTABLE_DIALECT,
@@ -2083,6 +2084,23 @@ def build_model(semantic_model: dict, tables: Sequence[TmlDocument], log: IssueL
20832084
formulas.append(formulas_entry)
20842085
columns.append(columns_entry)
20852086

2087+
for entry in model_payload.get(MODEL_STASH_UNSURFACED_FORMULAS) or []:
2088+
# Re-emitted with NO surfacing columns[] entry, because that is what
2089+
# they were: internal helpers other formulas reference. Giving them one
2090+
# -- as the unattributed-formula path deliberately does -- would make a
2091+
# formula the source kept private visible to users.
2092+
#
2093+
# The id is preserved rather than re-minted: it is what the surviving
2094+
# references name, and re-minting it from the display name is exactly
2095+
# how `[formula__startDate]` came back pointing at nothing.
2096+
unsurfaced_entry = {
2097+
"id": entry.get("id") or _formula_id_from(entry.get("name") or "formula"),
2098+
"name": entry.get("name") or "",
2099+
"expr": entry.get("expr") or "",
2100+
}
2101+
_allocate_formula_id(unsurfaced_entry, None, taken_formula_ids, log, preserved=True)
2102+
formulas.append(unsurfaced_entry)
2103+
20862104
for entry in model_payload.get(MODEL_STASH_UNATTRIBUTED_FORMULAS) or []:
20872105
# A formula spanning two or more Ossie datasets has no single
20882106
# dataset to belong to, which is exactly why the forward direction

‎converters/thoughtspot/src/ossie_thoughtspot/tml_to_ossie.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@
118118
MODEL_STASH_MODEL_JOINS_WITH,
119119
MODEL_STASH_MODEL_PROPERTIES,
120120
MODEL_STASH_PARAMETERS,
121+
MODEL_STASH_UNSURFACED_FORMULAS,
121122
MODEL_STASH_UNATTRIBUTED_FORMULAS,
122123
MODEL_STASH_UNREPRESENTABLE_JOINS,
123124
PORTABLE_DIALECT,
@@ -2329,6 +2330,22 @@ def resolve(table: str, column: str) -> str | None:
23292330
unattributed[FIELD_STASH_COLUMN_PROPERTIES] = properties
23302331
model_stash.setdefault(MODEL_STASH_UNATTRIBUTED_FORMULAS, []).append(unattributed)
23312332

2333+
# Formulas no columns[] entry surfaces. The loop above walks columns[], so
2334+
# these were never visited and were dropped outright -- and the references
2335+
# to them, from formulas that ARE surfaced, then dangled. Preserved with
2336+
# their ids, which is what those references name.
2337+
surfaced_formula_ids = {
2338+
column["formula_id"] for column in model_columns if column.get("formula_id")
2339+
}
2340+
for formula_id, formula_entry in formulas.items():
2341+
if formula_id in surfaced_formula_ids or "expr" not in formula_entry:
2342+
continue
2343+
model_stash.setdefault(MODEL_STASH_UNSURFACED_FORMULAS, []).append({
2344+
"id": formula_id,
2345+
"name": formula_entry.get("name") or formula_id,
2346+
"expr": formula_entry["expr"],
2347+
})
2348+
23322349
# -- Phase 3.5: unsurfaced physical columns, and SQL View output aliases --
23332350
# A Table/SQL-View column with no Ossie FIELD of its own is not part of
23342351
# the semantic model *as a field*, but has to be preserved verbatim

‎converters/thoughtspot/tests/test_roundtrip.py‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,3 +1009,64 @@ def test_a_wrapped_group_aggregate_is_unaffected(self):
10091009

10101010
def test_a_plain_aggregate_is_unaffected(self):
10111011
assert self._round_trip("sum ( [T::a] )", "SUM") == "SUM"
1012+
1013+
1014+
class TestAFormulaNoColumnSurfaces:
1015+
"""An internal helper formula must survive, and stay internal.
1016+
1017+
A model's `formulas[]` may hold entries that no `columns[]` entry surfaces --
1018+
helpers other formulas reference but users never see. A date-parameter model
1019+
is the common case: `_startDate` computes a window the visible formulas use.
1020+
1021+
The converter walks `columns[]`, so these were never visited and were
1022+
dropped outright. In one real model that was 32 of 41 formulas, and the
1023+
visible formulas referencing them came back with dangling
1024+
`[formula__startDate]` references -- an ERROR, a non-zero exit, and a
1025+
document ThoughtSpot would refuse.
1026+
"""
1027+
1028+
@staticmethod
1029+
def _round_trip():
1030+
table = TmlDocument(kind="table", guid=None, body={
1031+
"name": "T", "db": "D", "schema": "S", "db_table": "T",
1032+
"connection": {"name": "Conn"},
1033+
"columns": [{"name": "d", "db_column_name": "D",
1034+
"db_column_properties": {"data_type": "DATE"}}]})
1035+
model = TmlDocument(kind="model", guid=None, body={
1036+
"name": "M", "model_tables": [{"name": "T"}],
1037+
"columns": [
1038+
{"name": "d", "column_id": "T::d", "properties": {"column_type": "ATTRIBUTE"}},
1039+
{"name": "In Period", "formula_id": "f_vis",
1040+
"properties": {"column_type": "ATTRIBUTE"}},
1041+
],
1042+
"formulas": [
1043+
# Surfaced by nothing -- an internal helper.
1044+
{"id": "formula__startDate", "name": "_startDate",
1045+
"expr": "start_of_year ( [T::d] )"},
1046+
{"id": "f_vis", "name": "In Period",
1047+
"expr": "[T::d] >= [formula__startDate]"},
1048+
]})
1049+
ossie = tml_to_ossie.convert(DocumentSet(model=model, tables=(table,)))
1050+
return ossie_to_thoughtspot.convert(ossie.model)
1051+
1052+
def test_the_helper_formula_survives(self):
1053+
formulas = {f["name"] for f in self._round_trip().documents.model.body["formulas"]}
1054+
assert "_startDate" in formulas
1055+
1056+
def test_it_keeps_its_id_so_references_still_resolve(self):
1057+
body = self._round_trip().documents.model.body
1058+
emitted = {f["id"] for f in body["formulas"]}
1059+
referenced = {
1060+
ref for f in body["formulas"]
1061+
for ref in re.findall(r"\[(formula_[A-Za-z0-9_]+)\]", f["expr"])
1062+
}
1063+
assert not (referenced - emitted), f"dangling: {sorted(referenced - emitted)}"
1064+
1065+
def test_it_is_not_given_a_surfacing_column(self):
1066+
# It was internal in the source; surfacing it would expose a helper
1067+
# the modeller deliberately kept private.
1068+
body = self._round_trip().documents.model.body
1069+
assert "_startDate" not in {c["name"] for c in body["columns"]}
1070+
1071+
def test_the_conversion_reports_no_error(self):
1072+
assert not self._round_trip().issues.has_errors()

0 commit comments

Comments
 (0)