fix(thoughtspot): a malformed column or model name should not crash the whole conversion

[AmirF194]

A columns[] entry with no name key, or one whose name is present but not a
string (an int, null, or a bool), was read straight into identifiers.normalise()
and raised a bare KeyError or TypeError instead of the converter's own
"never a bare traceback" contract (stash.py's phrase, quoted in the issue). A model
name of the same wrongly typed shape hit the same TypeError in convert().

convert_field and convert_metric now validate the column's name through a
shared _column_display_name() helper before using it, and convert() applies the
same type check to the model's own name. All three degrade the way the existing
TS-{KIND}-* gaps already do: log a WARNING and skip the object, rather than
aborting the whole document.

The issue reports this same shape (a value read without a type check) across eight
to-ossie cases and six to-tml cases. This fixes the two verified here: a column
with no name, and name as int/null/bool at both column and model scope. The
remaining cases (aggregation, data_type, formulas[].expr, join on, and
db_column_name wrong types, plus the whole to-tml stash direction) are untouched.

Added tests in test_tml_to_ossie_fields.py, test_tml_to_ossie_metrics.py, and
test_tml_to_ossie.py covering all four shapes (missing name; name as int, null,
bool, float, or list; a metric name as bool; a model name as int or bool). Each
fails on unpatched main with the traceback the issue reports and passes on this
branch, confirmed both ways in the same container. Full suite: 965 passed (947
existing + 18 new), also run under uv sync && uv run pytest on the CI matrix's
Python extremes (3.10, 3.14). Coverage confirms every changed line is exercised.
I have not run the other scenarios in the issue's table, listed above.

Refs #469

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[kayemkim]

Ran this merged onto current main (e2d94d3): the thoughtspot workflow passes on all five Pythons, 965 against 947 on main. Reproduced the issue through the CLI on the minimal fixture: a column with no name and one with name: 42 both give the bare KeyError/TypeError on main, and on this branch come back as TS-FIELD-NO-NAME/TS-FIELD-NAME-INVALID warnings with the rest of the model converted and the output passing validate.py. A model name: 7 falls back to model the same way.

One thing I noticed while trying it, not something this PR introduces: once a column can be skipped, a formula elsewhere that still references it by name comes out as TS-EXPR-PARAM, "references the ThoughtSpot runtime parameter total_order_amount". A reference to a name that never existed gets the same code on main, so it is the existing dangling-reference path, but now that a dropped column is a possible cause a reader could take it for a parameter. Might be worth a line when the rest of #469's table gets its follow-up.

For sequencing, #463 touches the same four files and is currently conflicting with main, so whichever of the two lands second will need a rebase. LGTM.

[AmirF194]

Good catch on the dangling-reference case, that's not something this PR touches, and a note on it belongs in the #469 follow-up table rather than folded in here. On #463: it does touch the same four files, so whichever lands first, I'll rebase this one over it.

[jbonofre]

Thanks @AmirF194! The graceful degradation with _column_display_name() is very clean, and the column-level test coverage across various types is great.

I left two comments regarding model name handling: right now model_body.get("name") or "" causes falsy non-strings (0, False, None) to bypass the TS-MODEL-NAME-INVALID warning check while 42 and True are warned on. Once we check the raw value before falsy coercion so both truthy and falsy bad types behave consistently, this should be good to go.

[jbonofre]

I think there is a bug here with model_body.get("name") or "". Because falsy values (0, False, None, [], {}) are coarced to "" before this check, they bypass if model_display_name and not instance(...):

• name: True logs TS-MODEL-NAME-INVALID, but name: False logs nothing.
• name: 42 logs TS-MODEL-NAME-INVALID, but name: 0 logs nothing.
• name: null logs nothing for a model, but logs TS-FIELD-NAME-INVALID for a column.

In YAML/TML, name: 0 or name: false is not "no name at all" (missing key). It is an explicit non-string value of the wrong type and should be warned about consistently with truthy values and column names.

       model_name_raw = model_body.get("name")
       if "name" in model_body and not isinstance(model_name_raw, str):
           # Same malformed-type hazard as a column `name` (see
           # `_column_display_name`): the model has no field to skip, so it
           # falls back the same way an empty name already does, just with a
           # WARNING naming what was dropped.
           log.add(
               code="TS-MODEL-NAME-INVALID",
               severity=Severity.WARNING,
               message=(
                   f"model name {model_name_raw!r} is not a string; the "
                   f"semantic model is named 'model' instead"
               ),
               object_ref=f"model:{model_name_raw!r}",
           )
           model_display_name = ""
       else:
           model_display_name = model_name_raw or ""

[jbonofre]

Once the type check is adjusted to inspect model_body["name"] before default coercion, 0, False, and None should be reported under test_a_non_string_model_name_falls_back_and_is_reported rather than failing back silently.

We should test:

• All malformed types ([42, 0, True, False, None]) are reported with TS-MODEL-NAME-INVALID.
• Only genuinely missing names (no name key) or empty strings (name: "") fall back silently to "model".

[AmirF194]

Good catch, fixed: the type check now reads model_body.get("name") before the or "" coercion, so 0/False/None are reported with TS-MODEL-NAME-INVALID the same as 42/True. Split the old test into a missing-key case and an empty-string case, both still falling back silently, and parametrized the malformed case over all five bad types. Full suite still 969 passing.

[jbonofre]

Thanks for the quick turnaround and updates @AmirF194!

The raw-value check nicely handles both truthy and falsy non-string model names, and the split tests for missing keys vs empty strings vs invalid types provide solid coverage. All 969 tests pass cleanly across the matrix.

LGTM!

[AmirF194]

The falsy-vs-truthy split you flagged was a real gap, glad you caught it before merge. Splitting missing-key from empty-string in the tests made the parametrize easier to follow too.