[refactor] sampler: derive sequence state from feature configs + accept bare sub-feature attr_fields

[tiankongdeguiji]

Summary

Refactor BaseDataset so negative_sampler.attr_fields uses bare sub-feature names (e.g. "video_id") when item_id_field is a grouped sequence sub-feature, matching the convention every other sampler config (DSSM/MIND/TDM) already uses. HSTUMatch was the outlier writing the qualified flattened form ("cand_seq__video_id"), leaking DataParser's flatten convention into user-facing config.

Design

Two BaseDataset fields, both empty unless item_id_field is a grouped sequence sub-feature:

self._sampler_seq_delim: str   # parent sequence's `sequence_delim`
self._sampler_seq_prefix: str  # f"{sequence_name}{feature._underline}"

_underline (the in-feature separator, _ under RTP / __ otherwise) is read off the matching feature — never hardcoded — so the rewrite is RTP-safe.

launch_sampler_cluster prepends the prefix to every attr_fields entry (deep-copying the sampler sub-message so data_config isn't mutated):

if self._sampler_seq_prefix:
    sampler_config = copy.deepcopy(sampler_config)
    sampler_config.attr_fields[:] = [
        self._sampler_seq_prefix + a for a in sampler_config.attr_fields
    ]

Unconditional prepend — no startswith guard. Bare is the single canonical form; legacy qualified configs ("cand_seq__video_id" from #506) fail loud at sampler init.

The outer-list strip filter scopes to set(sampler_config.attr_fields) (post-rewrite). _merge_sampled_features gates the block-suffix combine on _sampler_seq_delim alone.

Domain invariant the design relies on

When item_id_field is a grouped sequence sub-feature, attr_fields are uniformly sequence-positive — they're all item-side per-positive. There's no "mixed" case where some entries are sequence and others are top-level scalars; the per-row multi-positive layout makes any non-sequence item-side attr structurally incoherent. Confirmed by grepping every config in tzrec/tests/configs/ + examples/. Top-level sequence_id_feature as item_id_field is rejected via an assert feature.is_grouped_sequence (unused by any current config; the two _TestReader tests that posited it are migrated to grouped style).

Config + doc update

 negative_sampler {
   input_path: "data/test/kuairand-1k-match-item-gl.txt"
-  attr_fields: "cand_seq__video_id"
+  attr_fields: "video_id"
   item_id_field: "cand_seq__video_id"
 }

docs/source/models/hstu_match.md gains a one-paragraph note: item_id_field is qualified (it doubles as the sequence-prefix source), attr_fields is bare.

Test plan

• python -m unittest tzrec.datasets.dataset_test tzrec.datasets.utils_test tzrec.datasets.sampler_test — green on local A10.
• python -m unittest tzrec.tests.match_integration_test.MatchIntegrationTest.test_hstu_with_fg_train_eval — green on local A10 (~193s).
• pre-commit run — clean on all touched files.

Source

Slice of #518 (feat/hstu-match-scalar-item-export), reimplemented from scratch with simplification driven by the domain invariant: a uniform prefix string captures the boundary, no per-attr branching, no alias dict, no defensive set probes. The HSTUMatchItemTower scalar export view from #518 is orthogonal and deferred to a follow-up PR.

🤖 Generated with Claude Code

[github-actions]

This is user-facing config validation, not an internal invariant — python -O strips the assert, after which feature.sequence_name (defaulted to None at feature.py:427 for non-grouped features) flows into feature.sequence_name + feature._underline two lines below and raises TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'. A confusing failure mode for what's a clear config error. Convert to raise ValueError(...) to match the explicit-raise pattern used elsewhere in this file (e.g. dataset.py:260).

Suggested change

	assert feature.is_grouped_sequence, (
	f"item_id_field '{self._sampler_item_id_field}' is a "
	f"sequence input but its matching feature is not a "
	f"grouped sequence; only grouped sequence sub-features "
	f"are supported as item_id_field."
	)
	if not feature.is_grouped_sequence:
	raise ValueError(
	f"item_id_field '{self._sampler_item_id_field}' is a "
	"sequence input but its matching feature is not a "
	"grouped sequence; only grouped sequence sub-features "
	"are supported as item_id_field."
	)

Worth pairing with a small negative test (assertRaisesRegex) so the contract is locked.

[github-actions]

Two coverage gaps worth a follow-up commit:

1. No negative test for the assert feature.is_grouped_sequence path at dataset.py:176. A 5-line assertRaisesRegex construction with a top-level sequence_id_feature + matching item_id_field would lock the rejection — without it, the assertion can be silently weakened in a future refactor (and would be invisible under python -O; see the related comment on the assert itself).

2. No large_list coverage in this test. The strip predicate at dataset.py:227 handles both is_list and is_large_list, but every input field here is pa.list_(...). Adding a single pa.large_list(pa.int64()) sub-feature row would cover the second arm.

[github-actions]

Review summary

Net read: clean simplification. Collapsing the _seq_field_delims dict into two correlated strings (_sampler_seq_delim + _sampler_seq_prefix) removes a class of latent bugs from per-key ambiguity and lets the hot path (_merge_sampled_features, build_sampler_input) drop a per-key dict lookup. Deep-copy-on-rewrite is the right pattern for keeping data_config immutable while the sampler sees flattened names. Performance: no regressions; the run-once deepcopy is gated to HSTUMatch and immaterial.

Filed 4 inline comments. Highest-value ones:

1. assert feature.is_grouped_sequence → raise ValueError (dataset.py:176). User-facing config validation; python -O strips the assert and the next line then TypeErrors on None + str for sequence_name. Add a negative test.
2. Legacy qualified attr_fields doesn't actually "fail loud" (dataset.py:212-216). The PR description's claim is contradicted by sampler.py:265-277 (silently appended to _ignore_attr_names with only a logger.warning). Worth an explicit pre-rewrite guard so the migration is genuinely loud.
3. feature._underline cross-module reach (dataset.py:183). One-line property on BaseFeature cleans this up.
4. Missing negative test for the assert path + large_list coverage in the new strip-and-rewrite test.

Doc note at hstu_match.md:214 is fine in isolation but the surrounding cross-reference to the DSSM 负采样配置 section now diverges from the HSTUMatch convention — folding finding #2 into a hard error makes this less load-bearing.

🤖 Generated with Claude Code