Skip to content

enrichment: augment-on-write contract for content_with_weight (latent ordering hazard) #44

Description

@henryle97

Problem

xrag/core/enrichment/table_context.py:69 (set_chunk_text(chunk, ...)) unconditionally overwrites the chunk's content_with_weight field with a freshly composed Section: ... Pre-context: ... Table: ... Post-context: ... wrapper. There is no check for prior content_with_weight content; whatever was there is clobbered.

This violates the Retrieval Text composition policy committed to in ADR 0002: every Retrieval Text write must augment, not replace.

Why this is a latent bug, not a current one

table_context is not in the canonical mvp3 enrichment pipeline. Both configs/rag-ir-mvp3.yml and configs/rag-ir-mvp3-all-docs.yml only run auto_keywords + auto_questions, neither of which writes content_with_weight. The comment in configs/enrichment/all.yml is explicit: "table_context is also opt-in: the chunker already populates pre_text / post_text".

Empirical confirmation against the canonical verify-refresh run:

  • 0 / 56 table chunks have content_with_weight set
  • 0 / 677 text chunks have content_with_weight set

So the unconditional overwrite is firing against an empty field today. The hazard is dormant.

When this would bite

Any future config that:

  1. Enables table_context in the enrichment pipeline, AND
  2. Adds another enricher that writes content_with_weight either before or after table_context

…would lose one of the two writes silently. Concrete example: if a future cross_doc_context enricher computes a similarity-based context summary into content_with_weight before table_context runs, the per-chunk neighbour-text wrapping clobbers it. Or vice-versa. The order of enrichment is determined by the YAML pipeline: list — easy to get wrong without the test bed catching it.

Acceptance Criteria

  • set_chunk_text in xrag/core/enrichment/common.py no longer accepts a single replacement string. Instead, an augment-on-write API: callers pass the new contribution, and the function merges with any existing content_with_weight content using a deterministic, idempotent rule.
  • Idempotence: running table_context twice produces the same content_with_weight as running it once. (No nested Section: ... Section: ... double-wraps.)
  • Order-independence verified by test: auto_keywords → table_context and table_context → auto_keywords produce equivalent content_with_weight for the same chunks (where auto_keywords is hypothetically extended to write to the field — the contract is what's tested).
  • Unit test for table_context re-run idempotence.
  • Unit test asserting set_chunk_text (or its replacement) preserves any non-empty prior content_with_weight value.
  • No changes to the canonical mvp3 configs in this PR. The fix is preventive hardening; current behaviour against current configs stays byte-identical.

Out of Scope

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageMaintainer needs to evaluate

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions