Record why the embedding metadata copy is a repair net, not a fold - #142
Merged
Conversation
Investigated folding a context_embedding row's metadata onto its owner, so the
rows could eventually be dropped. There is nothing to fold: the metadata is
already on the owner.
Measured on a real ingest, comparing every row against its owner record:
fields IDENTICAL on the owner 29 distinct
fields unique to the row 4 (dim, model, model_ref, storage_options)
The four unique ones are encoder provenance. Everything else is a copy.
record_with_embedding_defaults() fills ONLY fields the owner is missing, so
retrieval is already owner-first and this copy is never consulted in normal
operation. It is a self-repair net for an owner that has lost fields --
test_retrieve_recovers_hot_event_type_from_embedding_metadata builds exactly
that case, stripping event_type/classification/status/source_kind from a
context_event and asserting recovery.
So the question was never "where should the metadata live" but "is the repair
net worth its cost". Measured end to end:
dropping the copy 129,938 -> 126,510 bytes (2.6%)
2.6%, because a row's bytes are the vector, not its metadata -- and it breaks
the repair path. Losing self-repair to save 2.6% is not worth it, and dropping
the rows outright costs more still: eight tests covering hot_event_type
recovery, cross-session profile lineage and memory-layer classification read
them.
Comment only, at the consumer. A knob was implemented and measured first, then
removed rather than shipped: an option nobody should enable is the same
declared-config-that-should-not-be-used pattern already found twice in this
codebase (write_secondary_index, read by nothing until it was wired;
raw_storage_policy, stored and displayed but branched on nowhere). The finding
belongs in the code, not in a switch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Comment only — no behaviour change. It records a measurement so the trade behind
context_embeddingrows is not re-derived from scratch.What was investigated
Whether a
context_embeddingrow's metadata could be folded onto its owner, so the rowscould eventually stop being written. There is nothing to fold: the metadata is already
on the owner.
Measured on a real ingest, comparing every row against its owner record:
The four unique ones are encoder provenance. Everything else is a copy.
record_with_embedding_defaults()fills only fields the owner is missing, so retrievalis already owner-first and this copy is never consulted in normal operation. It is a
self-repair net for an owner that has lost fields —
test_retrieve_recovers_hot_event_type_from_embedding_metadatabuilds exactly that case,stripping
event_type/classification/status/source_kindfrom acontext_eventandasserting recovery.
The trade, measured end to end
2.6%, because a row's bytes are the vector, not its metadata. Losing self-repair for that
is not worth it — and dropping the rows outright costs more still: eight tests covering
hot_event_typerecovery, cross-session profile lineage and memory-layer classificationread them.
Why a comment and not a knob
A knob was implemented and measured first, then removed rather than shipped. An option
nobody should enable is the same declared-config-that-should-not-be-used shape already
found twice in this codebase —
write_secondary_index, read by nothing until it waswired, and
raw_storage_policy, stored and displayed but branched on nowhere. The findingbelongs in the code, not in a switch.
Verified comment-only: every added line is a comment, no removals. Mirror imports clean.