Skip to content

feat(measurement): add measurement gateway feedback contract - #6245

Open
bokelley wants to merge 11 commits into
mainfrom
review-pasted-text
Open

feat(measurement): add measurement gateway feedback contract#6245
bokelley wants to merge 11 commits into
mainfrom
review-pasted-text

Conversation

@bokelley

@bokelley bokelley commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Adds a compact, backwards-compatible performance feedback assertion with explicit baseline, metric identity, provenance, evidence, and maturation semantics.

Introduces distinct experimental capabilities for measurement providers, buyer-orchestrator gateways, and sellers, using two fixed first-tier tasks (get_media_buy_delivery for delivery access and provide_performance_feedback for assertion submission). Webhook and offline paths are explicitly deferred until their registration, credential, payload, and receipt contracts exist.

Updates docs, the new measurement skill, training-agent behavior, compliance, and the WG proposal so providers interact only with the orchestrator and the orchestrator controls seller fan-out.

Validated with focused schema and contract tests, 5,273 repository unit tests, typecheck, current and 3.0-compatible storyboard matrices, MCP projection, documentation checks, and compliance builds.

@bokelley
bokelley marked this pull request as ready for review August 6, 2026 15:28

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ladon verdict: Escalate to human review

Escalate — gated schema paths require human/CODEOWNERS review.

This PR modifies/adds published protocol surface under static/schemas/source/** (a deterministic hard approval gate) and review_decision is REVIEW_REQUIRED, not APPROVED. Per decision-table row 2, the gate is not satisfied and the outcome must be escalate.

The reviewer found no Critical/High/Medium findings: verified wire backwards-compatibility (request required set unchanged; response adds only optional fields; removed core fields were unwired and re-homed), schema↔docs coherence (all enum values resolve, baseline enum matches docs/training tool), and the discriminated oneOf audit is unaffected. The changeset is present and minor-scoped. Mechanically the diff looks clean, but this is a protocol wire-shape change on gated paths that adds new fields and a new experimental measurement_gateway block — a human/CODEOWNERS approval is required before merge.

Escalation reasons

  • Touches gated paths static/schemas/source/** (7 files, incl. new performance-feedback-metric.json and performance-baseline.json; modified performance-feedback.json, index.json, request/response, get-adcp-capabilities-response.json) while review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required.

Why human review

  • Gated paths touched (static/schemas/source/** — 7 files) and review_decision is REVIEW_REQUIRED, not APPROVED; human/CODEOWNERS approval required before merge.
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/performance-feedback-metric.json (added) matches static/schemas/source/**; static/schemas/source/core/performance-feedback.json (modified) matches static/schemas/source/**; static/schemas/source/enums/performance-baseline.json (added) matches static/schemas/source/**; static/schemas/source/index.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/provide-performance-feedback-request.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/provide-performance-feedback-response.json (modified) matches static/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-response.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Aug 6, 2026
@bokelley bokelley changed the title [codex] Add measurement gateway feedback contract feat(measurement): add measurement gateway feedback contract Aug 6, 2026
Comment thread static/schemas/source/core/performance-feedback.json Outdated
Comment thread specs/performance-feedback-contract.md Outdated

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ladon verdict: Request changes

Request changes — 1 blocking finding.

Blocking findings

  • static/schemas/source/core/performance-feedback.json:130 — Published core type core/performance-feedback.json is redefined and loses 4 previously-required fields under a minor changeset. Removing required fields is a Breaking-class wire change and takes major per the constitution; shipping it under minor is spec drift (high).

Medium findings

  • specs/performance-feedback-contract.md:444 — Spec claims the training-seller response path is tested, but no test exercises the handler branch; a regression here would be silent.

Row 1 of the decision table fires: a high finding is present, so the outcome is request-changes regardless of the gated-paths / high-risk / Breaking-class gates that also apply here. Note that even once the high finding is resolved, this PR touches gated schema paths (gated_paths: true) with review_decision: REVIEW_REQUIRED and carries a Breaking-class protocol change — both of which require human/CODEOWNERS approval (escalate) rather than auto-approve, so a subsequent clean pass will not auto-merge without that review.

Fix: reclassify the changeset as major (Breaking-class ratification is a human act), or preserve the removed required fields to keep the change non-breaking. Add a test covering the training-seller response handler branch.

Blocking findings

  • static/schemas/source/core/performance-feedback.json:130 — core/performance-feedback.json redefined and loses 4 required fields under a minor changeset (Breaking-class; requires major changeset)

Medium findings

  • specs/performance-feedback-contract.md:444 — Spec claims training-seller response path is tested; no test exercises the handler

@EvgenyAndroid

Copy link
Copy Markdown
Contributor

Implementer question on the account block, since this PR touches that table.

We run a sales agent (sell.nofluffadvisory.com) declaring:

account: { "require_operator_auth": false, "supported_billing": ["operator"], "sandbox": true }

Accounts auto-provision on first use from the buyer's natural key (brand + operator), and list_accounts is a read over what's been provisioned. We don't implement sync_accounts.

The table pairs require_operator_auth: false with "buyer declares accounts via sync_accounts", and pairs list_accounts with the true path. Is auto-provisioning from the natural key plus a read-only list_accounts conformant under false, or is sync_accounts expected for buyer-declared accounts? A sentence either way would help. The account surface has already changed our grading twice: declaring the sales-non-guaranteed specialism required an accounts surface, and adding list_accounts then switched the storyboard runner into account-discovery mode.

Separately, while this table is being edited: it lists supported_billing values as operator, agent, but the shipped 3.1.5 schema enum is ["operator","agent","advertiser"]. Worth aligning.

@EvgenyAndroid EvgenyAndroid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Asked to review. The contract is the right shape and the receipt-vs-application split is the part worth having. Two things I'd fix before merge, and one of them also clears most of what Ladon is blocking on.

1. measurement.gateway is MUST-declared but doesn't exist in the registry.

get-adcp-capabilities-response.json says orchestrators "MUST include measurement in supported_protocols and measurement.gateway in experimental_features", but docs/reference/experimental-status.mdx isn't in the diff, so the id resolves to nothing. That file calls itself the canonical list, and graduation is defined as removing the id from it, which can't happen for a row that was never added. The last four PRs that minted a feature id registered it in the same PR (#5304, #5305, #5387, #6214). Nothing catches this in CI: experimental_features is validated by pattern only, so this PR's own new test asserts an unregistered id validates, and it does.

Two smaller things in the same area. enums/adcp-protocol.json still says agents implementing measurement must list measurement.core; this PR rewrote that clause in the response schema and in get_adcp_capabilities.mdx but not in the enum, so an orchestrator copying your own gateway example is conformant against one gated file and not the other in the same commit. And the measurement.core why-experimental cell still reads "vendor metric catalog discovery only" after the PR adds produces_performance_feedback and both method arrays to that block.

2. The receipt cluster ships without x-status, and marking it is also the cheapest way out of Ladon's blocker.

measurement and measurement_gateway both carry x-status: experimental. media_buy.performance_feedback doesn't, and neither do application_status, status_reason, or applied_at on the response. execution.trusted_match directly below it does. Per experimental-status.mdx there's no undeclared middle ground, and x-status doesn't inherit through $ref, so the request's new allOf picks up nothing from the capability blocks.

That matters because your own WG question 2 asks whether application_status should ship now or whether sellers should return only feedback_id until there's an async audit path. Unmarked, both ways of answering "no" later, removing the field or shrinking its enum, land in 4.0.

The useful part: if the new surface is marked experimental, most of the Breaking-class objection goes with it, since experimental surfaces can drop required fields inside 3.x under the six-week notice contract.

3. Two corrections on the blocking finding.

The field list is slightly off. applied_at was a property on main but never in required. The fourth field dropped from required is feedback_source, which survives as an optional property.

More usefully, the first suggested fix (reclassify to major) isn't reachable. versioning.mdx:136 bars removing fields without a prior deprecation release, :142 says a 3.x deprecation isn't removed until 4.0, and 4.0 is targeted early 2027 with an 18-month floor. The second option is the only one available now: keep feedback_id, status, and submitted_at on the core type as deprecated: true and non-required through 3.x.

4. The removals aren't disclosed anywhere.

§Compatibility has six bullets and none mentions them. The lead bullet, "Existing request fields and required fields are unchanged", is true of the request via the new allOf but reads as covering the core type, which lost four published properties. §Schema changes describes it only as "becomes the canonical assertion rather than a parallel stored-record model". release-notes.mdx already has a 3.2.0 section and #6214 shipped a migration page in its own feature PR; neither is here. An old→new mapping table in §Compatibility would cover most of it.

5. not_applied is the only disposition with no machine-readable companion.

The orchestrator has to consume it across N sellers to keep the cross-seller audit trail the spec assigns it, and skills/adcp-measurement/SKILL.md tells agents to inspect status_reason, which is a bare string with no code, no maxLength, no PII constraint. Everywhere else AdCP pairs a required $ref'd reason code with bounded free text: core/impairment.json, creative-status-changed-webhook.json, account-status-changed-webhook.json. Suggest enums/performance-feedback-reason-code.json as an open documentary enum plus maxLength: 500 on status_reason. Separately, applied_at's description is a prohibition ("Present only when application_status is applied") and nothing enforces it; the response schema has no if/then at all, where 85 of 690 source schemas use it.

6. methodology is described as producer-scoped but isn't bound to producer.

producer is optional and nothing conditions methodology on it. On the orchestrator→seller hop the authenticated caller is the orchestrator, and the spec explicitly allows per-seller redaction, so forwarding methodology with producer stripped is valid and the seller holds a string it can't scope. dependencies: { methodology: ["producer"] } fixes it, and core/frequency-cap.json already uses draft-07 dependencies with the MCP projection handling it. Worth doing now: optional→required is Breaking once released, and the field is absent from main and v3.1.10 today.

Three smaller ones:

  • supersedes_feedback_id has no discovery bit and no acknowledgement. Your own training seller shows the gap: catalog-event-handlers.ts adds the field to the input schema, never reads it, and returns application_status: 'applied' with a fresh feedback_id. Echoing superseded_feedback_id in the success arm plus a supports_supersession bit alongside reports_application_status would close it. Not asking to reopen the revision graph you ruled out.
  • The sales-catalog-driven narrative says "The seller must distinguish receipt from actual optimizer application", the spec says legacy sellers may keep returning only success, and the response schema scopes the MUST to sellers declaring reports_application_status. Three answers in one PR. The validations block is also unchanged, so none of the new fields is graded; #6218 shipped three vector sets and #5748 shipped two storyboards.
  • evidence.confidence_interval validates with no sample_size. Intentional? sample_size's "when the producer can disclose it" reads like a deliberate allowance, but neither §Evidence nor the task reference says so, and a CI on the performance_index scale isn't interpretable without n. One sentence either way would settle it.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ladon verdict: Escalate to human review

Escalate — gated schema paths without human approval.

This PR modifies the published protocol surface under static/schemas/source/** (7 files, including new performance-feedback-metric.json, performance-baseline.json, and modifications to performance-feedback.json, index.json, both provide-performance-feedback request/response schemas, and get-adcp-capabilities-response.json). gated_paths is true and review_decision is CHANGES_REQUESTED — not APPROVED — so the hard approval gate (decision rule row 2) is not satisfied and the outcome cannot be approve or plain comment.

Iteration credit: Both prior blocking findings are resolved. The [high] performance-feedback.json field-loss concern is addressed — feedback_id/status/submitted_at/applied_at are re-added as deprecated OPTIONAL properties on a standalone sender-side assertion type, a non-breaking deprecation appropriate for the minor changeset. The [medium] untested handler concern is addressed — training-agent.test.ts:1303 now exercises handleProvidePerformanceFeedback with an applied-receipt assertion. The current run surfaces only two [low] findings, no blocking or medium concerns.

The diff is mechanically clean, but the deterministic gated-path gate requires a real human/CODEOWNERS approval before merge. Human review of the schema changes is required to lift the gate.

Escalation reasons:

  • Gated schema files under static/schemas/source/** require human/CODEOWNERS review; review_decision is CHANGES_REQUESTED, not APPROVED.

Why human review

  • Modifies gated protocol schema files under static/schemas/source/** (performance-feedback.json, index.json, provide-performance-feedback request/response, get-adcp-capabilities-response, plus new metric/baseline schemas); review_decision is CHANGES_REQUESTED, not APPROVED — human/CODEOWNERS review required to satisfy the hard approval gate.
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/performance-feedback-metric.json (added) matches static/schemas/source/**; static/schemas/source/core/performance-feedback.json (modified) matches static/schemas/source/**; static/schemas/source/enums/performance-baseline.json (added) matches static/schemas/source/**; static/schemas/source/index.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/provide-performance-feedback-request.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/provide-performance-feedback-response.json (modified) matches static/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-response.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'CHANGES_REQUESTED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

# Conflicts:
#	static/schemas/source/protocol/get-adcp-capabilities-response.json
@EvgenyAndroid

Copy link
Copy Markdown
Contributor

Everything I raised is resolved, most of it more cleanly than I proposed.

Splitting the compact assertion into performance-feedback-assertion.json and leaving core/performance-feedback.json untouched is a better answer than the deprecate-in-place route I suggested: nothing is removed, so the Breaking-class question doesn't arise at all and no migration entry is owed. The registry row, the x-status sweep across the receipt cluster, the dependencies binding on methodology (extended to methodology_version, which I'd missed), and the applied_at / status_reason conditionals all land as asked. The three schema-level gates enforcing experimental_features registration go past what I asked for and are the right call, given the equivalent MUSTs elsewhere in that file are prose-only.

One thing I didn't raise, because it wasn't in the PR when I reviewed it. 3743af5 removes delivery_output_methods, feedback_input_methods, delivery_input_methods and feedback_output_methods, replacing the negotiation with fixed delivery_task and feedback_task consts. I think that's defensible for a first cut — an intersection mechanism with one member on each side isn't earning its complexity. But it narrows the design the WG saw, and two artifacts still describe the deleted version: the PR body promises webhook and offline paths, and WG question 4 asks whether capability intersection plus account provisioning is sufficient or whether a mutable connection resource is needed, which is moot once there's nothing to intersect. A sentence in the body, and either striking question 4 or restating it as "when do we need more than two fixed tasks," would close it.

That's the only thing I'd want settled before approving. The rest I'm satisfied with. Two items I'd take as separate issues rather than hold this PR: status_reason still has no machine-readable companion, and the sales-catalog-driven validations block is unchanged, so none of the new fields is graded.

On WG question 2, now that application_status carries x-status: experimental — ship it. The reason to hold was that freezing a synchronous disposition would foreclose the async audit path, and experimental status removes that risk: it can be withdrawn or reshaped inside 3.x under the six-week notice. Shipping now buys implementer evidence on whether accepted versus applied is a distinction sellers can honor in practice, which is the question an async design would need answered first anyway.

Separately, thanks for the accounts answer — lazy provisioning under require_operator_auth: false with list_accounts as the recovery read is exactly the shape we're running, and the supported_billing alignment to include advertiser closes the docs/schema gap.

@bokelley

bokelley commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Two changes pushed in follow-up commit a7c558e:

  • PR body: second paragraph updated — "negotiated pull/webhook/offline delivery and task/webhook/offline feedback paths" replaced with the fixed-task description that matches the current implementation (two fixed consts, webhook/offline explicitly deferred).
  • specs/performance-feedback-contract.md: WG question 4 restated as "When will the two fixed first-tier tasks need to expand to a negotiated method set?" — the capability-intersection framing is retired now that there's nothing to intersect.

Your blocking point is resolved. The two follow-up items (status_reason machine-readable companion, sales-catalog-driven validations coverage) are noted for separate issues as you suggested.


Generated by Claude Code

@EvgenyAndroid EvgenyAndroid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving. The body and WG question 4 now match what the PR actually implements, which was the only thing I had outstanding.

Recording the two follow-ups so they don't get lost: status_reason has no machine-readable companion, so a not_applied disposition can't be routed programmatically across sellers; and the sales-catalog-driven validations block is unchanged, so none of the new receipt fields is graded anywhere in the suite. Both are additive against an experimental surface, so neither needs to hold this.

@EvgenyAndroid

Copy link
Copy Markdown
Contributor

My approve is in but it doesn't clear the gate — reviewDecision is still CHANGES_REQUESTED from Ladon's 2026-08-06T15:42:59Z review, which is the one that flagged the field removals. Its 07:18Z follow-up was submitted as COMMENTED, so it never dismissed its own earlier blocking review, and the finding behind it no longer exists now that core/performance-feedback.json is untouched.

Needs a dismissal on review 4876414785 from someone with write access.

@bokelley

bokelley commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@bokelley — EvgenyAndroid's read is correct. Ladon's CHANGES_REQUESTED review (ID 4876414785, submitted 2026-08-06T15:42:59Z) flagged field removals in core/performance-feedback.json. The current diff leaves that file untouched, and Ladon's own 07:18Z follow-up confirmed both prior blockers resolved. Because that follow-up was submitted as COMMENTED rather than APPROVED or a dismissal, GitHub's reviewDecision remains stuck at CHANGES_REQUESTED.

Action needed: dismiss review 4876414785. Suggested dismissal note: "Finding resolved — core/performance-feedback.json is untouched in the current diff; blocking concern no longer applies."

After dismissal, the ladon/needs-human-review label and the CODEOWNERS gate on static/schemas/source/** still apply — your approval as a write-access reviewer satisfies both.


Generated by Claude Code

@bokelley
bokelley dismissed aao-secretariat[bot]’s stale review August 8, 2026 14:15

Finding resolved — core/performance-feedback.json is untouched in the current diff; blocking concern no longer applies.

@lukasz-pubx

Copy link
Copy Markdown

Review at the current head d642784 — the post-approval merge of origin/main, which no prior review covered. Baseline: live docs build 3.1.2; provenance diffs against base 83903ac.

Two merge-specific verifications first, since the merge overlaps this PR's own files: main's new oauth capability block survived intact in get-adcp-capabilities-response.json alongside this PR's gates, and the validation claims re-run clean at the merged head (performance-feedback contract tests 7/7, schema validation 27/27, oneOf discriminator audit clean, MCP schema projection 9/9).

The tree delivers what the description promises, and the earlier round's asks are all genuinely in it — the registry rows, the x-status sweep, the dependencies bindings, the applied_at/status_reason conditionals, core/performance-feedback.json untouched. One merit worth naming: the base tree's docs already promised metric and a top-level vendor BrandRef that the base request schema never carried; this PR closes that pre-existing docs↔schema asymmetry.

Three findings I'd want considered, one before merge:

1. The PR silently deletes a ratified privacy MUST from idempotency_key.

Base description (provide-performance-feedback-request.json @ 83903ac, introduced in #1622):

MUST be unique per (seller, request) pair to prevent cross-seller correlation. Use a fresh UUID v4 for each request.

Head description:

Client-generated unique key for this logical assertion. Retries use the same key and payload.

The rule survives verbatim on sibling schemas (si-initiate-session-request.json:53, create-collection-list-request.json:44, others), and the L1 security page's (authenticated agent, account) scoping rule doesn't substitute for it. The drop matters most in exactly this PR's flow: the orchestrator fans one logical assertion out to N sellers, and both specs/performance-feedback-contract.md and skills/adcp-measurement/SKILL.md model a semantic, study-scoped key ("measurement-study-42-july-final"). An orchestrator that forwards that key to every seller is now conformant, and colluding sellers correlate their participation in the same study — the risk the deleted sentence named. §Compatibility's "Existing request fields and required fields are unchanged" doesn't disclose the change.

Ask: restore the uniqueness/anti-correlation sentence (per receiving agent — it composes fine with the new retry sentence) and swap the example key for a non-semantic one; or state explicitly that the relaxation is intentional and why.

2. An Accounts-Protocol rule change rides undisclosed in a measurement changeset.

The lazy-provisioning change (new MUSTs in docs/protocol/required-tasks.mdx, rewritten require_operator_auth and account-ref.json descriptions — both gated schema paths — and docs/accounts/overview.mdx) is well-executed, internally coherent across all four copies, and legitimately answers the implementer question raised in this thread. But the changeset text is 100% measurement: the 3.2.0 changelog will say nothing about accounts while conformance conditions for buyer-declared sellers changed — and this thread itself records that account-surface changes have flipped an implementer's grading twice.

Ask: add a sentence (or a second changeset) naming the lazy-provisioning rule change; splitting it into its own PR is the cleaner alternative but doesn't seem necessary at this stage.

3. The index-direction rule is MUST-shaped in the schema and SHOULD-shaped in the docs.

performance-feedback-assertion.json (imperative):

For higher-is-better ratio metrics use observed divided by baseline; for lower-is-better ratio metrics such as cost per acquisition use baseline divided by observed.

provide_performance_feedback.mdx:

higher-is-better measures SHOULD use observed / baseline; lower-is-better measures such as CPA SHOULD use baseline / observed.

Under SHOULD, a producer that submits CPA as observed/baseline is conformant, and a seller optimizer reads 0.74 as underperformance when it means the opposite — an undetectable inversion that steers optimization the wrong way.

Ask: make the direction rule MUST for compact-contract producers (baseline present) and align the three copies (schema, task doc, spec md); or state why SHOULD is intended.

Two notes, no action needed here: performance-feedback-metric.json is the fourth inline copy of the standard/vendor metric row (declared mirroring, and I verified exact five-key qualifier parity with committed-metric.json — but each copy is additionalProperties: false, so the next qualifier key must land in four places; a follow-up to hoist a shared $ref would cap the drift). And the two follow-ups already recorded in the approving review (status_reason reason code, sales-catalog-driven grading) remain open by design.

Overall: well-constructed. The schema-enforced capability gates, the untouched core stored-record type, and the closed docs↔schema gap go beyond what earlier review asked for, and minor is the right changeset for an additive, experimental-marked surface. Finding 1 is the one I'd resolve before merge; 2 and 3 are cheap to fold into the same push.

Finding 1 — restore idempotency_key privacy MUST: the PR dropped the
uniqueness/anti-correlation sentence introduced in #1622. Restored as
"MUST be unique per receiving agent to prevent cross-agent correlation;
use a fresh UUID v4 for each new assertion" in the schema description,
the task doc required-fields table, and the two example keys in
skills/adcp-measurement/SKILL.md and specs/performance-feedback-contract.md
(swapped from the semantic study-scoped key to a UUID).

Finding 2 — disclose accounts change in changeset: the lazy-provisioning
clarification in docs/protocol/required-tasks.mdx, account-ref.json, and
docs/accounts/overview.mdx was not mentioned in the changeset. Added a
sentence naming the rule change.

Finding 3 — align MUST/SHOULD on index-direction rule: performance-feedback-
assertion.json used imperative prose; provide_performance_feedback.mdx used
SHOULD. Aligned all four copies (schema description, task doc Baseline section,
SKILL.md rule, spec Baseline section) to MUST scoped to compact-contract
producers (baseline present).
@bokelley

bokelley commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

All three findings addressed in follow-up commit c1cfa46.

Finding 1 — idempotency_key privacy MUST restored. Schema description, the required-fields table in the task doc, and the two example keys in SKILL.md and specs/performance-feedback-contract.md all updated. The restored rule reads "MUST be unique per receiving agent to prevent cross-agent correlation; use a fresh UUID v4 for each new assertion. Retries use the same key and payload." The phrasing shifts from "per (seller, request) pair" to "per receiving agent" to fit the two-hop orchestrator flow, but the anti-correlation guarantee is the same. Both semantic study-scoped example keys (measurement-study-42-july-final) are replaced with a UUID.

Finding 2 — changeset now names the accounts change. Added a sentence to .changeset/compact-performance-feedback-measurement-agents.md: "Also clarifies Accounts Protocol conformance: buyer-declared sellers (require_operator_auth: false) MAY omit sync_accounts when they lazily provision from the natural key and expose list_accounts as the cold-start recovery read." The 3.2.0 changelog will now surface this alongside the measurement work.

Finding 3 — MUST/SHOULD mismatch resolved. All four copies aligned to MUST scoped to compact-contract producers (baseline present): schema performance_index description, task doc Baseline section, SKILL.md rules list, and spec Baseline section. The scoping to "baseline present" preserves backwards compatibility for legacy 3.x requests that omit baseline.


Generated by Claude Code

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

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants