DM-51789: Add support for looking up column references by name

[JeremyMcCormick]

Summary

This adds support for looking up column references by name throughout the data model. Previously all in-table references, including from the primaryKey field, constraints, and indices, required globally-unique @id values. They now accept column names directly. ID-based references are retained as a deprecated fallback that logs a warning, so existing schemas continue to validate and build without modification.

New lookup methods on Table

• _find_column(ref) — preferred entry point; resolves by name first, falls back to ID with a deprecation warning, and raises KeyError if the reference is ambiguous (matches one column's name and a different column's ID simultaneously)
• _find_column_by_name(name) — strict name-only lookup; no fallback, no warning
• _find_column_by_id(id) — ID-only lookup; always emits a deprecation warning

New ForeignKey reference style

A new reference field on ForeignKeyConstraint replaces the old referencedColumns list of global IDs. It names the target table and columns directly by name:

constraints:
  - name: fk_orders_customer
    "@type": ForeignKey
    columns:
      - customer_id
    reference:
      table: customers
      columns:
        - customer_id

The old referencedColumns style continues to work but is deprecated.

Schema-level validation

Three @model_validator validators on Schema now catch unresolvable column references at schema-load time rather than deferring errors to metadata construction:

• check_constraints — validates source columns of ForeignKey and Unique constraints; for name-based FK reference style, validates the referenced table and columns strictly by name with no ID fallback
• check_indexes — validates index column references
• check_primary_key — validates primaryKey column references; implemented at schema level (not table level) so that columns imported via columnRefs are fully populated before validation runs

Updated callers

metadata.py and tap_schema.py updated to use _find_column() for all in-table column resolution and _find_column_by_name() for name-based FK targets.

Documentation

User guide updated with a new "Referencing Columns by Name" section covering the preferred name-based style, the backward-compatible ID fallback, worked YAML examples, and deprecation notes for primaryKey, columnGroups, constraints, and indexes.

Tests

New tests cover name-based lookup and ID fallback for all reference contexts (primary keys, constraints, indexes, column groups), including the ambiguous reference error path and composite primary keys. The test_builder test in test_metadata.py was updated to use table._find_column() instead of the ID-only Schema.__getitem__ for primary key verification.

tests/data/sales.yaml and tests/data/test_composite_keys.yaml updated to use name-based primaryKey as the canonical example of the preferred style.

Checklist

• Ran Jenkins
• Added a release note for user-visible changes to docs/changes

[codecov]

Codecov Report

❌ Patch coverage is 99.20635% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.26%. Comparing base (346a149) to head (ad1b11f).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
python/felis/metadata.py	95.83%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #189      +/-   ##
==========================================
+ Coverage   91.89%   92.26%   +0.37%     
==========================================
  Files          13       13              
  Lines        2060     2133      +73     
  Branches      299      317      +18     
==========================================
+ Hits         1893     1968      +75     
+ Misses        109      108       -1     
+ Partials       58       57       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[Copilot]

Pull request overview

Adds name-based resolution of column references across the Felis datamodel and downstream consumers, while preserving backward compatibility with legacy global column IDs.

Changes:

• Introduces name-first column resolution (Table._find_column) and updates schema validation for constraints/indexes.
• Adds ForeignKeyReference to support a name-based foreign key target (reference) alongside legacy referencedColumns.
• Updates SQLAlchemy metadata builder, TAP_SCHEMA loader, and test fixtures to use name-based references.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/test_metadata.py	Updates metadata assertions to resolve constraint/index column refs via name-first lookup.
tests/test_datamodel.py	Adds tests covering name-based lookup for column groups, indexes, and foreign keys (including reference).
tests/data/sales.yaml	Migrates fixture constraints to name-based column refs and adds new constraints using reference.
python/felis/tap_schema.py	Resolves foreign keys using name-based refs (and legacy fallback) when generating TAP_SCHEMA keys/key_columns rows.
python/felis/metadata.py	Switches metadata building to resolve columns by name-first lookup and maps Felis objects to SQLAlchemy objects by identity.
python/felis/datamodel.py	Adds ForeignKeyReference, name-first column lookup, FK exclusivity checks, and schema validation for index/constraint refs.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (6)

python/felis/datamodel.py:1981

• Schema.check_indexes() reports any KeyError from Table._find_column() as "not found", which can misreport ambiguous references. Use the exception message so ambiguity details are preserved.

                    except KeyError:
                        _append_error(
                            errors,
                            (
                                "tables",

python/felis/datamodel.py:1880

• Schema.check_constraints() reports any KeyError from Table._find_column() as "not found", which can misreport ambiguous references (Table._find_column raises KeyError with an "Ambiguous" message). Use the exception message so ambiguous cases surface correctly.

                    except KeyError:
                        _append_error(
                            errors,
                            ("tables", table_index, "constraints", constraint_index, "columns", column_id),
                            column_id,

python/felis/datamodel.py:2029

• Schema.check_primary_key() reports any KeyError from Table._find_column() as "not found", which can misreport ambiguous references. Use the exception message so ambiguity details are preserved.

                except KeyError:
                    _append_error(
                        errors,
                        ("tables", table_index, "primaryKey", pk_index),
                        column_ref,

python/felis/datamodel.py:864

• ColumnGroup dereferencing converts any KeyError from Table._find_column() into a generic "not found" ValueError, which hides the new ambiguous-reference error path (and drops the detailed message). Preserve the original KeyError message so users can distinguish "not found" vs "ambiguous" failures.

This issue also appears in the following locations of the same file:

• line 1876
• line 1977
• line 2025

                try:
                    col_obj = self.table._find_column(col)
                except KeyError as e:
                    raise ValueError(f"Column '{col}' not found in table '{self.table.name}'") from e
                dereferenced_columns.append(col_obj)

python/felis/datamodel.py:2002

• Docstring typo: missing space after comma in "tables,so".

        tables,so all columns are available. Primary key columns are resolved

python/felis/datamodel.py:2092

• Schema.find_object_by_id() now logs a WARNING for every ID lookup, including non-column objects. This can introduce noisy logs for legitimate ID-based APIs that are not described as deprecated. Consider limiting the warning to column lookups (or moving the warning to the deprecated call sites) so only deprecated behavior emits warnings.

        logger.warning("Lookup by object ID '%s'; prefer name-based lookup where available", id)

[timj]

This code seems fine but I'm really worried about the approach in general.

In lsst/images#84 we have a whole machinery in place for doing schema migrations and this is completely missing in felis. The irony is that felis is a tool for schema management.

Somehow in the future we need a way for felis to understand the schema being used for a given felis file. There are two versions in play: there is the version associated with the fundamental layout of the file as read by felis and there is a version of the user schema represented in that file. Currently neither are versioned and felis has to try to guess what is happening on context. Something for discussion later. I will post this comment on Jira.

[timj]

As discussed, it might be better to accumulate the schema warnings (by warning type?) and issue a summary at the end as a single warning, and then use VERBOSE here.

[JeremyMcCormick]

@timj I've implemented one warning message per table in 0e6016b.

The messages look like:

WARNING:felis.datamodel:Deprecated ID lookup(s) encountered in table IsolatedStarStellarMotions: 
['#IsolatedStarStellarMotions.isolated_star_id', '#IsolatedStarStellarMotions.referenceId',
 '#IsolatedStarStellarMotions.isolated_star_id'] - see https://felis.lsst.io/user-guide/model.html#referencing-columns-by-name for name-based referencing style

This will still be somewhat noisy for schemas with many tables but accumulating all of the warnings across the entire schema and printing them in a batch is probably beyond the scope of this PR as it would require some substantial changes to the codebase.