Skip to content

test(fix): dictionary↔model drift test for the FIX message set (closes #437)#440

Merged
TexasCoding merged 1 commit into
mainfrom
feat/437-fix-drift-test
Jun 6, 2026
Merged

test(fix): dictionary↔model drift test for the FIX message set (closes #437)#440
TexasCoding merged 1 commit into
mainfrom
feat/437-fix-drift-test

Conversation

@TexasCoding

Copy link
Copy Markdown
Owner

Summary

The FIX analogue of the REST contract-drift suite. The REST SDK hard-fails on drift between its models and specs/openapi.yaml (TestRequestParamDrift / TestRequestBodyDrift); the FIX message layer had no equivalent — field tag/type faithfulness across the 8 flows was guarded only by hand-written golden-wire + round-trip tests, so a future dictionary revision (renamed tag, changed type) would silently pass. (Surfaced by the #429 pre-PR review.)

Changes

  • specs/kalshi-fix-dictionary.xml — the authoritative Kalshi FIX dictionary, checked into the repo (mirrors specs/openapi.yaml).
  • tests/fix/test_fix_drift.py — parametrized over every FixMessage/FixGroup model field (~278 cases). For each it asserts:
    • the field's tag exists in the dictionary's global field table with an equivalent FIX type;
    • every group's NumInGroup tag is typed NUMINGROUP;
    • every message's MsgType is a real dictionary message.

EXCLUSIONS (the only allowed drift — each documented)

  • Market-data tags (263/268/269/270/271/279/281/326) and msgtypes (V/W/X/Y/e/f) — absent from the bundled v1.03 dictionary because it predates market data; those models were derived from docs.kalshi.com/fix/market-data. (Swap in a refreshed dictionary when one ships.)
  • One type-equivalence rule: the RFQ Y/N flags (RestRemainder/ReplaceExisting/PreferBetterQuote, tags 21015/21016/21022) are typed CHAR with {Y,N} values in the dictionary; the SDK models them as BOOLEAN (identical Y/N wire), which the test accepts.

test_exclusions_not_stale asserts every excluded tag/msgtype is still used by a model and still absent from the dictionary — so the exclusions can't silently rot.

Verification

  • uv run ruff check . — clean
  • uv run mypy kalshi/ (strict) — clean; the test itself is also mypy-clean
  • uv run pytest tests/fix/530 passed (incl. the new drift parametrization)

Closes #437. Part of #402.

🤖 Generated with Claude Code

 #437)

The REST SDK hard-fails on model<->OpenAPI drift; the FIX message layer had no
analogous guard — field tag/type faithfulness for the 8 flows was checked only by
hand-written golden/round-trip tests, so a dictionary revision (renamed tag,
changed type) would silently pass.

- Check the authoritative Kalshi FIX dictionary into the repo as
  specs/kalshi-fix-dictionary.xml (mirrors specs/openapi.yaml).
- tests/fix/test_fix_drift.py: parametrized over every FixMessage/FixGroup model
  field, diffs (tag -> FIX type) against the dictionary's global field table,
  asserts each group's NumInGroup tag is NUMINGROUP, and each message's MsgType
  is a real dictionary message. ~278 parametrized cases.
- EXCLUSIONS (the only allowed drift, each with a reason): the market-data tags
  (263/268/269/270/271/279/281/326) and msgtypes (V/W/X/Y/e/f), absent from the
  stale v1.03 dictionary because it predates market data (those models derive
  from the docs). One type-equivalence rule: the SDK models the RFQ Y/N flags
  (21015/21016/21022), typed CHAR-with-{Y,N} in the dictionary, as BOOLEAN
  (identical wire). test_exclusions_not_stale keeps the exclusions honest.

ruff + mypy --strict clean; 530 FIX tests pass (incl. the new drift parametrization).

Closes #437. Part of #402.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jun 6, 2026

Copy link
Copy Markdown

Code Review — FIX Dictionary↔Model Drift Test (#437)

Overview

This PR closes the gap surfaced in #429: the REST SDK had hard-failing drift tests against specs/openapi.yaml, but the FIX message layer had nothing equivalent. The PR ships two files:

  • specs/kalshi-fix-dictionary.xml — the authoritative Kalshi FIX Dictionary v1.03 (778 lines), checked in as the spec anchor.
  • tests/fix/test_fix_drift.py — a parametrized drift suite (~170 lines) covering all FixMessage/FixGroup model fields against that dictionary.

The design is consistent with the REST contract tests and the exclusion mechanism is well-thought-out. A few things worth addressing before merge.


Issues

Minor (worth fixing)

1. Missing assertion message in the group exclusion branch

test_group_count_tag_is_numingroup (line 930) silently passes the assert count_tag not in DICT_FIELDS check with no message, while the analogous line in test_scalar_field_matches_dictionary (line 917) provides "tag {tag} is now defined in the dictionary; drop it from MD_DOC_DERIVED_TAGS". The group branch should have the same message for consistency — otherwise a future dictionary refresh that adds NoMDEntries will produce a cryptic failure.

# current
assert count_tag not in DICT_FIELDS

# should be
assert count_tag not in DICT_FIELDS, (
    f"tag {count_tag} is now defined in the dictionary; drop it from MD_DOC_DERIVED_TAGS"
)

2. Module-level docstring violates project style

CLAUDE.md says "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max." The file opens with a 12-line module docstring. The PR description already contains this explanation; the module docstring can be trimmed to a single line (e.g., """FIX dictionary↔model drift tests (GH #437).""") or dropped entirely.

3. XML quote style inconsistency in the dictionary

ExecutionReport's second NoPartyIDs group (lines 166-169 of the XML) uses double-quotes, while every other element in the file uses single-quotes:

<!-- rest of file uses single quotes -->
<group name="NoPartyIDs" required="N">   <!-- ← double quotes -->
    <field name="PartyID" required="N"/>
    <field name="PartyRole" required="N"/>
</group>

This is cosmetic but will stand out to anyone diffing the file against an upstream dictionary update.


Notes (no action required, but worth being aware of)

4. root.find("fields") is unguarded at module load time

_load_dictionary() calls for f in root.find("fields"): with a # type: ignore[union-attr]. If the XML is ever malformed or the <fields> element is missing, all 530+ tests in the file will fail with an AttributeError at import rather than a skippable test failure. Since this is a static file, it's not a real operational risk — just something to keep in mind if the dictionary is regenerated.

5. Private API imports from base (_GroupSpec, _ScalarSpec, _layout)

The test imports leading-underscore symbols directly from kalshi.fix.messages.base. This is appropriate for tightly-coupled test infrastructure, but any rename of those symbols will break this file. The # type: ignore[attr-defined] annotations on _layout() calls signal this coupling. Worth a comment, or the symbols could be officially re-exported — but that's a separate decision.

6. _all_models() depends on the import side-effect at the top

import kalshi.fix.messages # noqa: F401 is the only reason _all_models() returns a complete set. This is a known and valid pattern, but the # noqa suppression hides it from readers. A short inline comment would clarify intent.


What's done well

  • test_exclusions_not_stale is the standout feature of this PR. It enforces that every exclusion is actively used by a model AND genuinely absent from the dictionary — preventing the exclusion list from silently rotting after a dictionary refresh. This is exactly the right guard.
  • The parametrize structure gives per-field test IDs (e.g., NewOrderSingle.ClOrdID), making CI output actionable.
  • The _type_equivalent() BOOLEAN↔CHAR mapping is clearly documented with the rationale.
  • The dictionary and test file together give this repo a full "spec-first" story for both the REST and FIX layers.

Summary

The design and coverage are solid — this is a meaningful addition to the contract-test story. Two things worth fixing before merge: the missing assertion message in the group exclusion branch (a real usability issue on future failures) and the module docstring length (project convention). The XML quote inconsistency is cosmetic but easy to clean up.

@TexasCoding
TexasCoding merged commit c0716de into main Jun 6, 2026
5 checks passed
@TexasCoding
TexasCoding deleted the feat/437-fix-drift-test branch June 6, 2026 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FIX] Dictionary↔model drift test for the FIX message set (analogous to REST TestRequestBodyDrift)

1 participant