[doc] align feature docs with fg behavior

[tiankongdeguiji]

Doc-only follow-up to the same fg audit as #672 and #673, rebased onto master now that both have landed. Everything here was checked against the fg C++ (the fg markdown docs contain errors of their own) and, where behavior was involved, measured with pyfg 1.0.6.

Corrections — the docs described behavior fg does not have

• MatchFeature combiner bullet removed: the proto field and the emitter line are both commented out, and fg's match feature has no combiner concept at all.
• need_key / show_pkey / show_skey only work on string output — fg logs "will be ignored, because the value_type is not string" and drops them otherwise, and the match prefixes only exist on the discrete path. Also noted that fg's own default for show_pkey/show_skey is true for a hit match with need_discrete, so migrating a hand-written fg json changes the values.
• MatchFeature need_discrete requires a string output — with a numeric output fg raises need_discrete set, but match output type is not string.
• separator is additive — fg splits on \x1d and the configured character, it does not replace it. StringSplitV2 re-adds \x1d even where the call site passes a single char, and the AVX-512 variant ORs the two masks, so this holds on the batch path tzrec uses, not only in the single-record API.
• separator on RawFeature only applies when value_dim > 1 — and the input must split into exactly value_dim values.
• CombineFeature normalizer runs after the combiner — log10(sum(x)), not sum(log10(x)), unlike RawFeature which normalizes per value. num_buckets additionally truncates the aggregate to an integer.
• CombineFeature value_map misses are skipped, not counted as 0, so they change mean and count.
• ExprFeature int conversion rounds, it does not truncate.
• OverlapFeature method bullet listed 4 of the 9 methods the table below it documents.
• BoolMaskFeature only outputs the first value of each kept element — fg treats bool mask as a sequence feature and normalizes value_dim to 1; measured, value_dim 0 and 1 give identical output.
• CustomFeature value_dim defaults to 1 here (fg's own default is 0), so a multi-value operator output is truncated unless value_dim: 0 is set.
• default_value on LookupFeature/ExprFeature defaults to "0" here while fg's native default is empty, which means "drop the value" — worth knowing because a NaN/Inf expression result becomes a literal 0. Sequence features are called out separately: tzrec rewrites an empty default to "0" before fg sees it.

Additions — behaviors that were documented nowhere

boundaries bucket intervals (left-closed, right-open, measured); num_buckets out-of-range values collapsing to bucket 0; LookupFeature's un-aggregated multi-value output via an empty combiner; bool mask string truthiness (only "" and false are false, so a "1,0,1,0" string mask keeps everything); ExprFeature's % operator and temp-variable/comma expressions; fill_missing not applying to broadcast length-1 variables; CustomFeature's value_type selection, normalizer being ignored for string output, is_op_thread_safe, and *_file params not being collected as assets; and the fact that OverlapFeature/KvDotProduct support far less of RawFeature's surface than "其余配置同RawFeature" implies (including fg's -1 empty-input default for index_of).

Examples that were not valid configs

ExprFeature's sequence-mask example omitted sequence_delim and the BoolMaskFeature example omitted embedding_dim, both proto2 required fields: text_format.Parse accepts them and the getters fall back, but IsInitialized() is False and SerializeToString() raises, so copying either example into a pipeline config breaks as soon as the config is serialized. The BoolMask example additionally had no bucketizer, so num_embeddings raised ValueError as soon as embedding configs were built — it now sets hash_bucket_size.

Review follow-ups

Corrections applied from the review, each re-measured first: the num_buckets bucket-0 claim now covers only out-of-range values, since an empty input follows default_value ("" is dropped, default_value: "3" yields 3); num_buckets added to the need_discrete / need_key string-output enumerations, since match_feature.py and lookup_feature.py both emit value_type: "string" for it; the OverlapFeature method bullet; and the BoolMaskFeature value_dim claim, which measurement showed was about the value count per element, not about value_dim at all. Two other review items resolved themselves in the merge: the KvDotProduct normalizer bullet is accurate now that #673 emits it, and the version bump is gone.

Test Plan

Docs and proto comments only — no emitted fg json changes. Every feature_configs { ... } block in feature.md and faq.md was parsed with text_format and serialized, which is how the invalid examples were found. tzrec/features/*_test.py pass, pre-commit run --files <changed> (mdformat included) and pyrefly check are clean, and the proto regenerates.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VkWBjeAQP8rcsutwL6rkfV

[github-actions]

This doc-only PR bumps __version__ 1.4.8 → 1.4.11, skipping 1.4.9/1.4.10, while the PR body says "Docs and proto comments only". Master is still at 1.4.8, so merging publishes the 3-step jump. Intentional sync with an internal release train, or a rebase artifact? If intentional, worth a mention in the PR body; otherwise drop it or make it a single increment.

[github-actions]

This new claim ("value_dim只能是1 … value_dim=0时会被重置为1，每个元素只保留第一个值") is contradicted by the repo's own pyfg-driven tests:

• bool_mask_feature_test.py builds a non-sequence BoolMaskFeature with value_dim unset and separator: ",", and IdFeature.value_dim returns 0 in that case (id_feature.py:49-57), so the fg json carries value_dim: 0.
• In FG_NORMAL mode, Feature._parse() runs the actual engine: self._fg_op.process_arrow(...) where _fg_op = pyfg.FgArrowHandler(...) (feature.py:966-967, ~1023) — pyfg 1.0.6, the same version this PR says it re-measured with (requirements/runtime.txt:17).
• The green tests expect multi-value output: "1,2,3,4" + mask "true,false,true,false" → values [1, 3], lengths [2] — two values kept from one element, not just the first. The untouched example table right below (输出 ["123", "90"]) shows the same.

If the 0→1 rewrite is real, it must be scoped (e.g. only for bool mask as a sequence sub-feature, or only in the online pai-fg build vs pyfg) — as written it contradicts the checkout's passing tests and the table ten lines below, and would push users away from multi-value bool-mask IDs that demonstrably work in FG_NORMAL. If training pyfg and serving fg genuinely differ here, that's a train/serve skew the doc should say explicitly. The same claim in feature.proto:944-945 needs the same treatment.

[github-actions]

This new bullet advertises a knob that tzrec silently drops: KvDotProduct._fg_json (tzrec/features/kv_dot_product.py:64-84) never copies normalizer into the fg json (no normalizer reference in the file at all), unlike raw_feature.py, overlap_feature.py, combine_feature.py, lookup_feature.py, match_feature.py, custom_feature.py, and there is no generic injection in BaseFeature.fg_json (feature.py:1025-1040). A user setting method=log10 here gets a silent no-op → mis-scaled dense values / mis-bucketization, with no error. Either drop this bullet, or note that it's currently ignored — the real fix (forwarding normalizer in _fg_json) is a code change outside this doc PR's scope and deserves its own issue/PR.

PS, pre-existing and out of scope but squarely in this audit's territory: the kv_delimiter bullet two lines above says the kv separator is configurable (默认 ":"), but the proto field is optional float kv_delimiter = 8 (feature.proto:794) — a string like "|" can't even be parsed into it, and kv_dot_product.py:75-76 would forward a float to fg. Worth a follow-up.

[github-actions]

This section still contains the same class of inconsistency the PR fixes elsewhere: the method bullet above (line 560) still reads "可选 query_common_ratio | title_common_ratio | is_contain | is_equal" — 4 of the 9 methods — while the table right below lists all 9 and this PR updates the proto comment (feature.proto:638-640) to the full list. Worth updating the bullet too (or having it defer to the table), since the PR body specifically calls out this 4-of-9 gap.

[github-actions]

The enumeration misses num_buckets: tzrec's own emitter treats it as a valid discrete bucketizer for match features — match_feature.py:150-154 sets value_type: "string" + needDiscrete: true when num_buckets is configured — so need_discrete=true + num_buckets produces string output and does not hit the fg error. The very next bullet (prefix suppressed "配置num_buckets时不加") implies that combination exists. Suggest "必须同时配置hash_bucket_size/num_buckets/vocab_list/vocab_dict/vocab_file之一". The proto comment (feature.proto:501-503, "a hash/vocab bucketizer must be set") has the same gap.

[github-actions]

"hash/vocab bucketizer" excludes num_buckets, but tzrec's emitter treats num_buckets as a string-output discrete bucketizer for match features (match_feature.py:150-154 sets value_type="string" + needDiscrete=true), so need_discrete=true + num_buckets does not raise. Suggest "a bucketizer (hash_bucket_size/num_buckets/vocab_*) must be set". Minor grammar: "otherwise fg raises" reads truncated — "otherwise fg raises an error". (Same gap in the feature.md bullet; see the comment there.)

[github-actions]

Same concern as the feature.md bullet: bool_mask_feature_test.py runs pyfg 1.0.6 via FG_NORMAL (_parse → pyfg.FgArrowHandler.process_arrow) with value_dim: 0 in the fg json and expects multi-value output ([1, 3], lengths [2]), so "rewrites value_dim 0 to 1 … keeps its first value only" is contradicted by the checkout's green tests for the non-sequence case. Please re-verify and scope the claim (sequence sub-feature? online pai-fg build?) or drop it.

[github-actions]

Review summary (multi-area: code quality, performance, test coverage, security, docs accuracy)

Overall a careful, well-researched audit — most changed lines check out against the code (verified e.g. gap_min/gap_max have zero references repo-wide; LookupFeature empty-combiner note matches lookup_feature.py:182-185; CustomFeature value_dim default 1 and value_type selection match custom_feature.py; OverlapFeature lacks default_value/hash_bucket_size in the proto; the two example fixes were necessary — SequenceFeature.sequence_delim and BoolMaskFeature.embedding_dim are proto2 required; the proto diff is comment-only, no wire-format change; 29 examples confirmed). A few things need a second look before merge — see inline comments:

1. BoolMaskFeature value_dim claim (feature.md + proto) — "value_dim=0时会被重置为1，每个元素只保留第一个值" is contradicted by the checkout's green bool_mask_feature_test.py, which runs pyfg 1.0.6 via FG_NORMAL (_parse → pyfg.FgArrowHandler.process_arrow) with value_dim: 0 in the fg json and expects multi-value output ([1, 3], lengths [2]), and by the untouched example table right below the bullet. Needs re-verification or explicit scoping (sequence sub-feature? online pai-fg vs pyfg?).
2. New KvDotProduct normalizer bullet — kv_dot_product.py never forwards normalizer into the fg json (no reference in the file, no generic injection in BaseFeature.fg_json), so the newly documented knob is a silent no-op today. Drop/qualify the bullet or fix the emitter in a follow-up. (Same comment also notes the pre-existing optional float kv_delimiter vs documented string separator mismatch.)
3. OverlapFeature method bullet still lists 4 of the 9 methods while the table below and the updated proto comment list all 9 — the exact inconsistency the PR set out to fix.
4. MatchFeature need_discrete bucketizer enumeration (doc + proto) omits num_buckets, which match_feature.py:150-154 emits as a string-output discrete bucketizer; also "otherwise fg raises" reads truncated.
5. version.py bump 1.4.8 → 1.4.11 in a PR described as "Docs and proto comments only", skipping two versions while master is at 1.4.8 — intentional release-train sync or rebase artifact?

Optional follow-ups (non-blocking): lookup_feature_test.py has no fg_json() assertions at all, so the need_discrete=true → combiner="" rewrite this PR now documents is uncovered; and since the 29-example text_format parse caught two real bugs here, a small test that parses every feature_configs block in feature.md would prevent this class of doc rot permanently.

Security review found nothing noteworthy. Note: the dedicated documentation-accuracy pass over feature.md was still running when this summary was posted; its substantive overlaps are already covered by items 1–4 above.

🤖 Generated with Claude Code

[github-actions]

Adding embedding_dim makes this example parse/serialize, but it still can't run in a pipeline: BoolMaskFeature extends IdFeature and is always sparse (id_feature.py:64-69), and with no hash_bucket_size/num_buckets/vocab_*/zch, num_embeddings raises ValueError("...must set hash_bucket_size or num_buckets or vocab_list or vocab_dict or zch.zch_size") (id_feature.py:86-95) as soon as embedding configs are built. Both unit tests always set a bucketizer, and the sequence example below sets num_buckets: 1000. Since the PR's stated goal is that copying the example into a pipeline config works, consider adding e.g. hash_bucket_size: 1000 here too.

[github-actions]

The "输入为空…分箱结果为0" half contradicts the repo's own pyfg-driven test: test_id_feature_with_num_buckets (id_feature_test.py:525-563, FG_NORMAL → real pyfg 1.0.6) feeds "" with num_buckets: 100 and expects it dropped — values [0,1,2], lengths [2,0,1] — with the IdFeature default default_value: ""; with default_value: "3" it yields 3, not 0. Empty input only lands in bucket 0 if default_value is explicitly "0". Suggest scoping the bucket-0 + ERROR-log claim to values that actually reach bucketization (non-integer / out-of-range), and letting empty input follow the documented default_value semantics above.

[github-actions]

Same enumeration gap as the MatchFeature need_discrete bullet: num_buckets also produces string output for LookupFeature — lookup_feature.py:160-162 sets value_type: "string" for num_buckets, which then forces needDiscrete: true (lines 180-181) — so need_key does take effect with num_buckets, contrary to this list. Consider adding num_buckets here too.

PS (also applies to the default_value bullet on line 315 and the ExprFeature one on line 386): "配置为空字符串时FG会不输出任何值" is only true for non-sequence usage. For sequence features tzrec rewrites an empty default_value to "0" before fg ever sees it (BaseFeature.default_value, feature.py:557-571, with a WARNING; and the fg_json wrapper at feature.py:1030-1034), so a one-clause caveat would make those bullets accurate for sequence_* usage.

[github-actions]

Follow-up: the documentation-accuracy pass has now completed (all five review areas finished). Three additional findings posted inline on feature.md:

6. BoolMaskFeature example still can't run (line 678) — with embedding_dim added it now parses/serializes, but as an always-sparse IdFeature subclass with no bucketizer it raises ValueError from num_embeddings (id_feature.py:86-95) when embeddings are built; suggest adding hash_bucket_size/num_buckets like the sequence example does.
7. num_buckets "输入为空→分箱结果为0" claim (line 97) — contradicted by test_id_feature_with_num_buckets (pyfg 1.0.6, FG_NORMAL): empty input is dropped (or filled with default_value), never bucketed to 0 under the default default_value: "".
8. LookupFeature need_key enumeration (line 317) misses num_buckets, which also yields string output (lookup_feature.py:160-162); plus a small caveat that the "empty default_value → no output" bullets (lines 315, 386) don't hold for sequence features, where tzrec resets empty defaults to "0" (feature.py:557-571, 1030-1034).

Housekeeping note: one sub-review initially flagged the two required-field example fixes (sequence_delim, embedding_dim) as missing — that was an artifact of a stale checkout on the review runner, not the PR. The PR head (49f007a) contains both fixes; disregard.

Net: no security or wire-format concerns (proto diff is comment-only). The items to resolve before merge are the BoolMaskFeature value_dim claim (contradicted by green pyfg tests — item 1 above), the KvDotProduct normalizer bullet documenting a silently-dropped option (item 2), and the unexplained version.py bump (item 5); the rest are precision fixes.