Skip to content

Adapt Knowledge context to a Harness delivery draft - #446

Open
zengchuang-hw wants to merge 2 commits into
InferMatrixLab:mainfrom
zengchuang-hw:codex/apps-371-context-provider
Open

zengchuang-hw wants to merge 2 commits into
InferMatrixLab:mainfrom
zengchuang-hw:codex/apps-371-context-provider

Conversation

@zengchuang-hw

@zengchuang-hw zengchuang-hw commented Nov 15, 2026 •

Copy link
Copy Markdown
Contributor

Governing issue and delivery slice

Refs #371. Step 4, Apps side of D023-3 / V33. Accepted design and architecture acceptance.

ContextProvider.collect calls the public Knowledge reader once. It passes DefinitionRef.name/revision to Knowledge's revision resolver, preserves Knowledge's selection and omission facts, and returns only delivered item pins with resource roles and per-item byte totals. The draft payload is the selected UTF-8 resource bytes in Knowledge order, concatenated without display framing. The digest covers those exact bytes. The proposed location uses a hash of the round identifier for a safe relative path. Zero bytes yields an empty draft with budget omissions.

The draft is an in-process value. Harness retains the bytes, seals the delivery reference, and writes its generation manifest once the retention route is wired. The production retention failure, commit, and fresh-process read for T38 remain open.

Checks

  • 12 focused tests passed on Windows using the packaged Knowledge reader, covering exact bytes from fixed revisions, deterministic ordering/location, selection, zero and insufficient budgets, unavailable pins, request validation, and corrupted content.
  • 266 Windows tests passed across Apps, Knowledge selection/contract, retrieval projection and module boundaries, including the focused cases above.
  • Repository Ruff and staged diff whitespace checks passed.

Review

Please check the pin mapping against the public Knowledge resolver, actual-byte accounting, and the Harness retention boundary. No change to the shared schema, existing CLI, or Knowledge implementation.

Signed-off-by: zengchuang <zengchuang3@huawei.com>
Agent-assisted-by: Codex (author)
@Dong1017 Dong1017 added area:harness Harness module: profile, A/B loop, quality gates, evidence reports kind:task Bounded delivery task with a delegated slice labels Nov 15, 2026
amy-why-3459
amy-why-3459 previously approved these changes Nov 15, 2026

@amy-why-3459 amy-why-3459 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

PR review verdict — PASS
Reviewed base c2f463f066921a15120a3c86506c4f97b1909acb and head 35f24b61fe1517592995ade1f13a3956f12958c7 (single commit).

What I checked:

  • Request validation in src/inferrsi/apps/context_provider.py L43-66 matches ContextRequest in docs/contracts/schemas/contract-v1.schema.json: exact field set, pins as DefinitionRef {name, revision}, max_bytes in the UInt range, round_id against the Id pattern. Selection options are checked before KnowledgeReader.collect is called.
  • Pin mapping: _revisions passes name -> revision into collect(revisions=...), and conflicting pins are rejected up front. An unavailable pin becomes an unavailable_revision omission and never falls back to the current revision (collect L677-681).
  • Byte accounting: each resource is checked against its sha256, each block against content_bytes, and the total against max_bytes. The draft digest covers exactly the concatenated bytes. max_bytes == 0 skips assembly and records context_limit omissions.
  • Output shape matches DeliveredItem (pin {reference, revision}, deduplicated delivered_roles, delivered_bytes) and ErrorFact (code/stage/message/retryable) for omissions.
  • D023-3 boundary (docs/contracts/p0a-contract-errata.md L48-52): the draft is a process-local dataclass with no artifact and no write to Harness RunStore. Retention, sealing and the T38 fresh-process read stay with Harness and are tracked in #447.
  • All 8 checks on this head are green (full-check 3.10/3.11, installed-package 3.10/3.11, declarations, pr-references, dashboard-browser).

Optional, non-blocking: L72 takes selection from the top-level result["selection"] in relevance mode and from result["retrieval"]["selection"] otherwise, so state / tie_break are present only in explicit-id mode. Always using the retrieval observation would keep the draft's selection facts the same shape in both modes.

Signed-off-by: zengchuang <zengchuang3@huawei.com>
Agent-assisted-by: Codex (author)
@zengchuang-hw

Copy link
Copy Markdown
Contributor Author

Thanks for the selection feedback. 5c5be71f now preserves Knowledge's common retrieval observation in both modes while retaining the extra relevance facts. A test asserts the applied state and tie breaker on the ranked path. All 12 context-provider tests and repository Ruff passed on this head. @amy-why-3459 please re-review this delta when the new checks finish.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

PR review verdict — PASS
Reviewed base c2f463f066921a15120a3c86506c4f97b1909acb and head 5c5be71f27007d456e7a278e1e5f4f2a9b034668.

Re-review after Amy Approve on 35f24b61 was dismissed. Prior optional selection-shape note is fixed: {**result.get("selection", {}), **result["retrieval"]["selection"]} now keeps relevance cue facts and always surfaces retrieval state / tie_break (asserted on the ranked path).

Verified:

  • ContextRequest validation (exact fields, DefinitionRef pins, UInt max_bytes, Id round_id) before KnowledgeReader.collect
  • Pin map name→revision with conflict rejection; unavailable pin → unavailable_revision, no current-revision fallback
  • Byte accounting: per-resource sha256, block content_bytes, total ≤ max_bytes; draft digest over exact concatenated UTF-8 bytes
  • DeliveredItem / ErrorFact shapes; no artifact on draft (D023-3 / V33); retention/T38 remains #447
  • Local: 12/12 tests/test_apps_context_provider.py, ruff clean; CI on this head all success (full-check/installed-package 3.10+3.11, declarations, pr-references, dashboard-browser)

No blocking findings. Did not merge.

@amy-why-3459 amy-why-3459 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

PR review verdict — PASS
Incremental re-review: 35f24b61fe1517592995ade1f13a3956f12958c7...5c5be71f27007d456e7a278e1e5f4f2a9b034668 (base c2f463f066921a15120a3c86506c4f97b1909acb). My earlier approval on 35f24b61 was dismissed by this push.

  • src/inferrsi/apps/context_provider.py L64: selection is now {**result.get("selection", {}), **result["retrieval"]["selection"]}, so state / tie_break from the retrieval observation are present in both modes. The relevance-only cue (and matched_count / returned_count on the task path) is kept. In relevance mode the overlapping keys (method, scope, ranking_applied) take the same values from both sources, so letting retrieval win changes nothing. This resolves my optional note.
  • tests/test_apps_context_provider.py L87-88 now assert state == "applied" and tie_break == "ref_ascending" on the ranked path.
  • No other changes in the delta. All 7 checks on this head are green (full-check 3.10/3.11, installed-package 3.10/3.11, declarations, pr-references, dashboard-browser).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:harness Harness module: profile, A/B loop, quality gates, evidence reports kind:task Bounded delivery task with a delegated slice

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants