feat(core): add provider/adapter/display_name fields to ProviderKey (Phase A skeleton)

[moonming]

Summary

Second sub-PR of api7/AISIX-Cloud#302 Phase A (DP-side Provider→Adapter refactor). Builds on #297 (which landed the Adapter enum).

This PR is intentionally zero-behavior-change — pure type extension on ProviderKey. Nothing in the gateway reads the new fields yet; Provider continues to drive 100% of dispatch.

What this PR does

1. Adds four new fields to aisix_core::ProviderKey:

   • provider: String — vendor identity (e.g. "deepseek", "openai"). Free-form String in this PR; closed-set validation is deferred to a later Phase A sub-PR that wires dispatch.
   • adapter: Option<Adapter> — wire-shape, pinned to the closed Adapter enum from feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297. None until a follow-up populates it.
   • telemetry_tags: TelemetryTags — attribution tags emitted alongside requests routed through this key.

2. Adds a new TelemetryTags struct with five optional fields:

   • kind: Option<String> — closed-set "catalog" | "byo" (enforced by the JSON Schema; the Rust type is Option<String> to stay forward-compatible if cp-api ships a new variant ahead of a DP rollout)
   • featured: bool — defaults to false
   • branded_provider: Option<String>
   • pk_label: Option<String>
   • byo_label: Option<String>

   TelemetryTags derives Default and is #[serde(deny_unknown_fields)] so an unknown tag from cp-api fails loudly on the DP rather than silently dropping.

3. Updates the provider_key JSON Schema with matching optional properties. additionalProperties: false is preserved both at the top level and inside telemetry_tags. adapter is constrained to the five Adapter enum values; kind is constrained to "catalog" | "byo".

4. Re-exports TelemetryTags from aisix_core::models and the crate root, mirroring Adapter.

Backward compatibility

All four new struct fields use #[serde(default)]. A pre-#302 payload like

{"display_name":"openai-prod","secret":"sk-x","api_base":"https://api.openai.com/v1"}

still deserializes — provider lands as "", adapter as None, telemetry_tags as TelemetryTags::default(). A dedicated test pins this contract (legacy_payload_without_phase_a_fields_deserialises_with_defaults).

The JSON Schema mirrors this: only display_name and secret remain required.

What this PR does NOT do

• Does not remove or modify Provider
• Does not change display_name (pre-existed on ProviderKey)
• Does not touch secret / api_base
• Does not change Hub, Bridges, dispatch, or any wire transform
• Does not change mustMarshalProviderKeyKV on the CP side (that's Phase D)
• Does not delete the wrapper crate
• Does not touch rerank.rs (A5) or OpenAiBridge (A4)

Design notes

• Option<Adapter> over Adapter::Default: keeping adapterNone-able makes "Phase A hasn't backfilled this yet" representable. Adding a default variant would lie about adapter shape for un-tagged keys.
• TelemetryTags as a struct (not HashMap<String, Value>): the catalog/byo attribution shape is a closed set known to both cp-api and the DP; a typed struct with deny_unknown_fields catches drift between the two sides at parse time. A free-form map would silently swallow typos.

Test plan

• cargo test -p aisix-core — 150 passed, 0 failed (+14 over feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297)
• cargo clippy --workspace --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean
• Legacy payload without Phase A fields deserializes (compat contract)
• Full Phase A payload (catalog shape) deserializes and round-trips
• BYO telemetry shape (branded_provider:null + byo_label) deserializes
• Unknown adapter string rejected (closed-set guard)
• Unknown telemetry_tags field rejected (deny_unknown_fields guard)
• Unknown telemetry_tags.kind value rejected at schema layer
• Schema accepts both legacy and Phase A payloads; rejects unknown top-level fields

References

• Tracking issue: api7/AISIX-Cloud#302
• Predecessor: feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297 (Adapter enum)

Summary by CodeRabbit

• New Features

  • Extended provider configuration to support telemetry tagging, adapter specification, and additional provider metadata fields while maintaining backward compatibility with existing payloads.

• Tests

  • Added comprehensive test coverage for new schema fields, legacy payload compatibility, and validation constraints.

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: ddb92b36-e041-4434-bcf7-10268b7dc3ca

📥 Commits

Reviewing files that changed from the base of the PR and between 83c48c9 and 28024f6.

📒 Files selected for processing (4)

• crates/aisix-core/src/lib.rs
• crates/aisix-core/src/models/mod.rs
• crates/aisix-core/src/models/provider_key.rs
• crates/aisix-core/src/models/schema.rs

---

📝 Walkthrough

Walkthrough

This PR extends the ProviderKey model with Phase A fields (provider, adapter, telemetry_tags) to support provider identity and telemetry attribution. A new TelemetryTags struct is introduced with serde defaults and strict field validation. The changes maintain backward compatibility so legacy payloads deserialize with zero values, and comprehensive tests verify both JSON schema validation and round-trip serialization.

Changes

Provider Key Phase A Extension

Layer / File(s)	Summary
Type definitions and public exports crates/aisix-core/src/models/provider_key.rs, crates/aisix-core/src/models/mod.rs, crates/aisix-core/src/lib.rs	New TelemetryTags struct introduced with serde defaults and deny_unknown_fields attribute. ProviderKey extended with three new fields: provider (string), adapter (optional), and telemetry_tags. Adapter imported to support the new field type. Public re-exports in mod.rs and lib.rs updated to expose TelemetryTags.
JSON schema validation and documentation crates/aisix-core/src/models/schema.rs	provider_key_schema() extended with Phase A fields as optional (preserving backward compatibility). Documentation added explaining that provider, adapter, and telemetry_tags are not yet wired to dispatch behavior. Schema tests added covering minimal payloads, legacy payloads without Phase A fields, and payloads with Phase A fields including both catalog and BYO telemetry shapes. Rejection tests verify unknown adapter values, unknown telemetry fields, and invalid kind discriminator values are caught.
Deserialization and round-trip compatibility tests crates/aisix-core/src/models/provider_key.rs	Test suite verifies legacy payloads without Phase A fields deserialize to defaults. Payloads with all Phase A fields (including telemetry tag values and BYO dual-label handling) deserialize correctly. adapter field rejects unknown adapter strings. telemetry_tags rejects unknown fields. A ProviderKey containing default Phase A fields successfully round-trips via JSON without losing semantic equality.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

---

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[moonming]

Independent audit (CLAUDE.md §8)

Cold re-read of the diff via gh pr diff 298 — no shared context with the implementation session. Reviewed against the six required angles.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. No maxLength bound on free-form telemetry fields (crates/aisix-core/src/models/schema.rs:288-307). branded_provider, pk_label, byo_label, and the top-level provider are all unbounded strings in the JSON Schema. A misbehaving (or compromised) cp-api could publish a 10MB-per-tag value and the DP would happily persist it into the snapshot. This is consistent with the pre-existing lack of maxLength on display_name/secret/api_base, so it is not a regression introduced here — but it is worth tightening in a follow-up Phase A PR (suggest 120 chars to match cache_policy.name).
   Suggested follow-up edit:

   "provider": { "type": "string", "maxLength": 120 },
   "branded_provider": { "type": ["string", "null"], "maxLength": 120 },
   "pk_label": { "type": ["string", "null"], "maxLength": 120 },
   "byo_label": { "type": ["string", "null"], "maxLength": 120 }

2. Schema does not enforce the catalog/byo mutual-exclusion shape (crates/aisix-core/src/models/schema.rs:288-307). The PR description notes that the canonical BYO shape has branded_provider:null + byo_label="…" and the catalog shape has branded_provider:"vendor" + byo_label:null, but the schema allows any combination (both set, both null, etc.). This is a deliberate Phase A skeleton choice — no dispatch reads the field yet, and adding a conditional oneOf now would foreclose what Phase A migration learns about real cp-api payloads. Worth pinning the constraint once Phase A wires the telemetry consumer.

3. TelemetryTags::Default produces all-None including kind: None, but the schema constrains kind to "catalog"|"byo" (no null allowed). A round-trip of TelemetryTags::default() via serde_json::to_value produces {} (because of skip_serializing_if = "Option::is_none"), which is schema-valid. A round-trip via serde_json::to_value with serialize_none = true (not used today) would emit kind: null and fail schema. This is a non-issue with current code, but worth a comment near the struct definition so a future PR doesn't add a null-emitting serializer.

Per-angle assessment

• Correctness: The PR claims "zero behavior change" and the diff bears it out — no dispatch site, snapshot loader, hub, or bridge reads the new fields. Verified by running cargo test --workspace (all 18 crate test suites pass, no downstream breakage). All five test helpers across the workspace that build ProviderKey go through serde_json::from_str, never struct-literal syntax, so the addition of new required-but-defaulted fields does not break any existing test. PASS.
• Reliability: Pure type extension; no async, no I/O, no time-sensitive code. PASS.
• Security: No new credential flow; secret semantics unchanged. The new fields are operator-supplied labels with no privileged interpretation in this PR. PASS for this PR (see LOW 1 for a hardening follow-up).
• Sensitive-info leakage: New fields are non-secret. The Debug impl on ProviderKey already leaks secret (pre-existing), and the new fields do not widen that exposure. PASS.
• Breaking changes: aisix-core is a workspace-internal crate (no publish = true). All in-workspace consumers reach ProviderKey via serde_json, never struct-literal construction with full field list — verified by grep. The wire format adds optional fields only; additionalProperties: false plus the existing on-disk payloads with no provider/adapter/telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes). PASS.
• E2E test coverage: Adequate for the "skeleton, no consumer" contract this PR claims. Both struct-level and schema-level legacy-payload-compat tests are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No need for cross-crate or HTTP-level e2e in this PR because there is no behavior under test — that lands in the follow-up Phase A sub-PR that wires dispatch.

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Independent third-party audit per CLAUDE.md §8 (separate from author self-review)

Cold re-read of this PR by a fresh agent with no shared context with the implementation session or with the author's own audit comment above. Reviewed against the six required angles and against the canonical wire shapes in api7/AISIX-Cloud#302 §5.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. PR title slightly misrepresents the diff. The title says "add provider/adapter/display_name fields" but display_name is pre-existing on ProviderKey (verified by git show main:crates/aisix-core/src/models/provider_key.rs). The diff only adds provider, adapter, and telemetry_tags. The body is accurate ("Adds four new fields…" actually says 4 but lists 3 — display_name is correctly absent from the body's enumeration). The body explicitly says "Does not change display_name (pre-existed on ProviderKey)". Non-blocking — squash-merge commit title is what would land on main, suggest tightening to feat(core): add provider/adapter/telemetry_tags fields to ProviderKey (Phase A skeleton).

2. No maxLength on any of the new string fields (crates/aisix-core/src/models/schema.rs:288-307). Independently verified: a 100,000-char provider / pk_label / byo_label / branded_provider passes both schema validation and serde deserialisation. Consistent with the pre-existing lack of maxLength on display_name / secret / api_base, so not a regression — but AISIX-Cloud#302 §5 specifies display_name length 1-64 with sluggified-form ≥ 3. The DP currently enforces neither, leaving cp-api as the sole gatekeeper for label length. Worth tightening in a follow-up alongside the §5 validation work cp-api is committing to.

3. adapter:null is silently accepted by serde but rejected by schema. Independently verified:

   • Struct path: serde_json::from_str of {"adapter": null, …} → adapter = None, Ok(()).
   • Schema path: validate_provider_key of the same → rejected ("null is not one of [openai,…]").
   • Because the etcd loader runs schema validation before struct deserialisation, a wire payload with adapter:null is rejected at the schema gate, so this divergence has no observable impact in this PR. But it is worth a one-line code comment near the adapter field saying "schema is the gatekeeper for null-vs-absent; the serde Option only handles the absent case". Otherwise a future PR that builds a ProviderKey from a serde_json::Valuebypassing the schema (e.g. a unit test reading a fixture, or a CLI tool) could see adapter:None semantics for a payload that the schema would have rejected.

Per-angle assessment

• Correctness — PR claims "zero behavior change" and the diff bears it out. No dispatch site, snapshot loader, hub, or bridge reads the new fields. Independently verified by running cargo test -p aisix-core (150 passed, 0 failed) and cargo check --workspace (clean). All five ProviderKey builder helpers across the workspace (aisix-gateway/src/bridge.rs:361, aisix-provider-anthropic/src/bridge.rs:553, aisix-provider-openai/src/bridge.rs:787, aisix-core/src/models/snapshot.rs:74, aisix-proxy/src/dispatch.rs:283) use serde_json::from_str, never struct-literal — so the new fields ride on #[serde(default)] and existing tests don't need to be touched. The single struct-literal site (in the new round-trip test) is the only place that sets all fields explicitly, and it's correct. Independently verified the legacy-payload-compat contract by running the test under cargo test. PASS.

• Reliability — Pure type extension; no async, no I/O, no time-sensitive code, no new error path. The schema-then-struct two-stage parse on the etcd loader path is unchanged. PASS.

• Security — No new credential flow; secret semantics unchanged. The new fields are operator-supplied attribution labels with no privileged interpretation in this PR (no dispatch site reads them). AISIX-Cloud#302 §5 places the validation responsibility on cp-api's createProviderKey — the DP is correctly second-line-of-defence-only, which matches the spec. PASS for this PR (see LOW 2 for the future hardening alignment with §5).

• Sensitive-info leakage — New fields are non-secret by construction (vendor identity, adapter shape, telemetry tags). The pre-existing Debug impl on ProviderKey already leaks secret (out of scope here), and the new fields do not widen that exposure. The new Debug impl on TelemetryTags is fine — all fields are labels intended for log/metric emission anyway. PASS.

• Breaking changes — aisix-core is a workspace-internal crate (no publish = true in Cargo.toml). New struct fields are all #[serde(default)] so the wire format adds optional fields only. additionalProperties: false on the schema continues to apply, with the three new fields explicitly enumerated. Existing on-disk KV payloads with no provider / adapter / telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes and legacy_payload_without_phase_a_fields_deserialises_with_defaults). The new pub use TelemetryTags does not collide with any existing export (verified by rg "TelemetryTags" crates/ — only the new declaration and re-exports appear). PASS.

• E2E test coverage — Adequate for the "skeleton, no consumer" contract this PR claims. The 14 new tests cover both directions:

  • Schema-level: minimal, legacy-no-phase-a, full-phase-a, byo-shape, unknown-adapter, unknown-telemetry-field, unknown-top-level, unknown-kind.
  • Struct-level: legacy-no-phase-a-defaults, full-phase-a, byo-shape, unknown-telemetry-field, unknown-adapter, round-trip-with-defaults.
    Both layers are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No cross-crate or HTTP-level e2e needed in this PR because there is no behavior under test — the dispatch wiring lands in the follow-up Phase A sub-PR per the PR description. PASS.

Independent edge-case probe (not added to the PR — purely for audit verification, see findings above)

Verified the following edge cases by spawning a temporary integration test in crates/aisix-core/tests/audit_check.rs (since deleted, working tree is clean):

Payload fragment	Serde result	Schema result	Outcome
"provider": null	Err — invalid type: null, expected a string	Err — null is not of type string	Both gate; OK
"telemetry_tags": null	Err — invalid type: null, expected struct	Err — null is not of type object	Both gate; OK
"adapter": null	Ok — None	Err — null is not one of [openai,…]	Divergence; see LOW 3
omitted (legacy)	Ok — defaults	Ok	Compat pinned
"telemetry_tags": {}	Ok — default	Ok	Compat pinned
provider = 100k chars	Ok	Ok	See LOW 2
Round-trip of TelemetryTags::default()	Emits {"featured": false} (kind/branded_provider/pk_label/byo_label all skip_serializing_if)	Schema accepts	OK

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Summary

Second sub-PR of api7/AISIX-Cloud#302 Phase A (DP-side Provider→Adapter refactor). Builds on #297 (which landed the Adapter enum).

This PR is intentionally zero-behavior-change — pure type extension on ProviderKey. Nothing in the gateway reads the new fields yet; Provider continues to drive 100% of dispatch.

What this PR does

1. Adds four new fields to aisix_core::ProviderKey:

   • provider: String — vendor identity (e.g. "deepseek", "openai"). Free-form String in this PR; closed-set validation is deferred to a later Phase A sub-PR that wires dispatch.
   • adapter: Option<Adapter> — wire-shape, pinned to the closed Adapter enum from feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297. None until a follow-up populates it.
   • telemetry_tags: TelemetryTags — attribution tags emitted alongside requests routed through this key.

2. Adds a new TelemetryTags struct with five optional fields:

   • kind: Option<String> — closed-set "catalog" | "byo" (enforced by the JSON Schema; the Rust type is Option<String> to stay forward-compatible if cp-api ships a new variant ahead of a DP rollout)
   • featured: bool — defaults to false
   • branded_provider: Option<String>
   • pk_label: Option<String>
   • byo_label: Option<String>

   TelemetryTags derives Default and is #[serde(deny_unknown_fields)] so an unknown tag from cp-api fails loudly on the DP rather than silently dropping.

3. Updates the provider_key JSON Schema with matching optional properties. additionalProperties: false is preserved both at the top level and inside telemetry_tags. adapter is constrained to the five Adapter enum values; kind is constrained to "catalog" | "byo".

4. Re-exports TelemetryTags from aisix_core::models and the crate root, mirroring Adapter.

Backward compatibility

All four new struct fields use #[serde(default)]. A pre-#302 payload like

{"display_name":"openai-prod","secret":"sk-x","api_base":"https://api.openai.com/v1"}

still deserializes — provider lands as "", adapter as None, telemetry_tags as TelemetryTags::default(). A dedicated test pins this contract (legacy_payload_without_phase_a_fields_deserialises_with_defaults).

The JSON Schema mirrors this: only display_name and secret remain required.

What this PR does NOT do

• Does not remove or modify Provider
• Does not change display_name (pre-existed on ProviderKey)
• Does not touch secret / api_base
• Does not change Hub, Bridges, dispatch, or any wire transform
• Does not change mustMarshalProviderKeyKV on the CP side (that's Phase D)
• Does not delete the wrapper crate
• Does not touch rerank.rs (A5) or OpenAiBridge (A4)

Design notes

• Option<Adapter> over Adapter::Default: keeping adapterNone-able makes "Phase A hasn't backfilled this yet" representable. Adding a default variant would lie about adapter shape for un-tagged keys.
• TelemetryTags as a struct (not HashMap<String, Value>): the catalog/byo attribution shape is a closed set known to both cp-api and the DP; a typed struct with deny_unknown_fields catches drift between the two sides at parse time. A free-form map would silently swallow typos.

Test plan

• cargo test -p aisix-core — 150 passed, 0 failed (+14 over feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297)
• cargo clippy --workspace --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean
• Legacy payload without Phase A fields deserializes (compat contract)
• Full Phase A payload (catalog shape) deserializes and round-trips
• BYO telemetry shape (branded_provider:null + byo_label) deserializes
• Unknown adapter string rejected (closed-set guard)
• Unknown telemetry_tags field rejected (deny_unknown_fields guard)
• Unknown telemetry_tags.kind value rejected at schema layer
• Schema accepts both legacy and Phase A payloads; rejects unknown top-level fields

References

• Tracking issue: api7/AISIX-Cloud#302
• Predecessor: feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297 (Adapter enum)

Summary by CodeRabbit

• New Features

  • Extended provider configuration to support telemetry tagging, adapter specification, and additional provider metadata fields while maintaining backward compatibility with existing payloads.

• Tests

  • Added comprehensive test coverage for new schema fields, legacy payload compatibility, and validation constraints.

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: ddb92b36-e041-4434-bcf7-10268b7dc3ca

📥 Commits

Reviewing files that changed from the base of the PR and between 83c48c9 and 28024f6.

📒 Files selected for processing (4)

• crates/aisix-core/src/lib.rs
• crates/aisix-core/src/models/mod.rs
• crates/aisix-core/src/models/provider_key.rs
• crates/aisix-core/src/models/schema.rs

---

📝 Walkthrough

Walkthrough

This PR extends the ProviderKey model with Phase A fields (provider, adapter, telemetry_tags) to support provider identity and telemetry attribution. A new TelemetryTags struct is introduced with serde defaults and strict field validation. The changes maintain backward compatibility so legacy payloads deserialize with zero values, and comprehensive tests verify both JSON schema validation and round-trip serialization.

Changes

Provider Key Phase A Extension

Layer / File(s)	Summary
Type definitions and public exports crates/aisix-core/src/models/provider_key.rs, crates/aisix-core/src/models/mod.rs, crates/aisix-core/src/lib.rs	New TelemetryTags struct introduced with serde defaults and deny_unknown_fields attribute. ProviderKey extended with three new fields: provider (string), adapter (optional), and telemetry_tags. Adapter imported to support the new field type. Public re-exports in mod.rs and lib.rs updated to expose TelemetryTags.
JSON schema validation and documentation crates/aisix-core/src/models/schema.rs	provider_key_schema() extended with Phase A fields as optional (preserving backward compatibility). Documentation added explaining that provider, adapter, and telemetry_tags are not yet wired to dispatch behavior. Schema tests added covering minimal payloads, legacy payloads without Phase A fields, and payloads with Phase A fields including both catalog and BYO telemetry shapes. Rejection tests verify unknown adapter values, unknown telemetry fields, and invalid kind discriminator values are caught.
Deserialization and round-trip compatibility tests crates/aisix-core/src/models/provider_key.rs	Test suite verifies legacy payloads without Phase A fields deserialize to defaults. Payloads with all Phase A fields (including telemetry tag values and BYO dual-label handling) deserialize correctly. adapter field rejects unknown adapter strings. telemetry_tags rejects unknown fields. A ProviderKey containing default Phase A fields successfully round-trips via JSON without losing semantic equality.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

---

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[moonming]

Independent audit (CLAUDE.md §8)

Cold re-read of the diff via gh pr diff 298 — no shared context with the implementation session. Reviewed against the six required angles.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. No maxLength bound on free-form telemetry fields (crates/aisix-core/src/models/schema.rs:288-307). branded_provider, pk_label, byo_label, and the top-level provider are all unbounded strings in the JSON Schema. A misbehaving (or compromised) cp-api could publish a 10MB-per-tag value and the DP would happily persist it into the snapshot. This is consistent with the pre-existing lack of maxLength on display_name/secret/api_base, so it is not a regression introduced here — but it is worth tightening in a follow-up Phase A PR (suggest 120 chars to match cache_policy.name).
   Suggested follow-up edit:

   "provider": { "type": "string", "maxLength": 120 },
   "branded_provider": { "type": ["string", "null"], "maxLength": 120 },
   "pk_label": { "type": ["string", "null"], "maxLength": 120 },
   "byo_label": { "type": ["string", "null"], "maxLength": 120 }

2. Schema does not enforce the catalog/byo mutual-exclusion shape (crates/aisix-core/src/models/schema.rs:288-307). The PR description notes that the canonical BYO shape has branded_provider:null + byo_label="…" and the catalog shape has branded_provider:"vendor" + byo_label:null, but the schema allows any combination (both set, both null, etc.). This is a deliberate Phase A skeleton choice — no dispatch reads the field yet, and adding a conditional oneOf now would foreclose what Phase A migration learns about real cp-api payloads. Worth pinning the constraint once Phase A wires the telemetry consumer.

3. TelemetryTags::Default produces all-None including kind: None, but the schema constrains kind to "catalog"|"byo" (no null allowed). A round-trip of TelemetryTags::default() via serde_json::to_value produces {} (because of skip_serializing_if = "Option::is_none"), which is schema-valid. A round-trip via serde_json::to_value with serialize_none = true (not used today) would emit kind: null and fail schema. This is a non-issue with current code, but worth a comment near the struct definition so a future PR doesn't add a null-emitting serializer.

Per-angle assessment

• Correctness: The PR claims "zero behavior change" and the diff bears it out — no dispatch site, snapshot loader, hub, or bridge reads the new fields. Verified by running cargo test --workspace (all 18 crate test suites pass, no downstream breakage). All five test helpers across the workspace that build ProviderKey go through serde_json::from_str, never struct-literal syntax, so the addition of new required-but-defaulted fields does not break any existing test. PASS.
• Reliability: Pure type extension; no async, no I/O, no time-sensitive code. PASS.
• Security: No new credential flow; secret semantics unchanged. The new fields are operator-supplied labels with no privileged interpretation in this PR. PASS for this PR (see LOW 1 for a hardening follow-up).
• Sensitive-info leakage: New fields are non-secret. The Debug impl on ProviderKey already leaks secret (pre-existing), and the new fields do not widen that exposure. PASS.
• Breaking changes: aisix-core is a workspace-internal crate (no publish = true). All in-workspace consumers reach ProviderKey via serde_json, never struct-literal construction with full field list — verified by grep. The wire format adds optional fields only; additionalProperties: false plus the existing on-disk payloads with no provider/adapter/telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes). PASS.
• E2E test coverage: Adequate for the "skeleton, no consumer" contract this PR claims. Both struct-level and schema-level legacy-payload-compat tests are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No need for cross-crate or HTTP-level e2e in this PR because there is no behavior under test — that lands in the follow-up Phase A sub-PR that wires dispatch.

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Independent third-party audit per CLAUDE.md §8 (separate from author self-review)

Cold re-read of this PR by a fresh agent with no shared context with the implementation session or with the author's own audit comment above. Reviewed against the six required angles and against the canonical wire shapes in api7/AISIX-Cloud#302 §5.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. PR title slightly misrepresents the diff. The title says "add provider/adapter/display_name fields" but display_name is pre-existing on ProviderKey (verified by git show main:crates/aisix-core/src/models/provider_key.rs). The diff only adds provider, adapter, and telemetry_tags. The body is accurate ("Adds four new fields…" actually says 4 but lists 3 — display_name is correctly absent from the body's enumeration). The body explicitly says "Does not change display_name (pre-existed on ProviderKey)". Non-blocking — squash-merge commit title is what would land on main, suggest tightening to feat(core): add provider/adapter/telemetry_tags fields to ProviderKey (Phase A skeleton).

2. No maxLength on any of the new string fields (crates/aisix-core/src/models/schema.rs:288-307). Independently verified: a 100,000-char provider / pk_label / byo_label / branded_provider passes both schema validation and serde deserialisation. Consistent with the pre-existing lack of maxLength on display_name / secret / api_base, so not a regression — but AISIX-Cloud#302 §5 specifies display_name length 1-64 with sluggified-form ≥ 3. The DP currently enforces neither, leaving cp-api as the sole gatekeeper for label length. Worth tightening in a follow-up alongside the §5 validation work cp-api is committing to.

3. adapter:null is silently accepted by serde but rejected by schema. Independently verified:

   • Struct path: serde_json::from_str of {"adapter": null, …} → adapter = None, Ok(()).
   • Schema path: validate_provider_key of the same → rejected ("null is not one of [openai,…]").
   • Because the etcd loader runs schema validation before struct deserialisation, a wire payload with adapter:null is rejected at the schema gate, so this divergence has no observable impact in this PR. But it is worth a one-line code comment near the adapter field saying "schema is the gatekeeper for null-vs-absent; the serde Option only handles the absent case". Otherwise a future PR that builds a ProviderKey from a serde_json::Valuebypassing the schema (e.g. a unit test reading a fixture, or a CLI tool) could see adapter:None semantics for a payload that the schema would have rejected.

Per-angle assessment

• Correctness — PR claims "zero behavior change" and the diff bears it out. No dispatch site, snapshot loader, hub, or bridge reads the new fields. Independently verified by running cargo test -p aisix-core (150 passed, 0 failed) and cargo check --workspace (clean). All five ProviderKey builder helpers across the workspace (aisix-gateway/src/bridge.rs:361, aisix-provider-anthropic/src/bridge.rs:553, aisix-provider-openai/src/bridge.rs:787, aisix-core/src/models/snapshot.rs:74, aisix-proxy/src/dispatch.rs:283) use serde_json::from_str, never struct-literal — so the new fields ride on #[serde(default)] and existing tests don't need to be touched. The single struct-literal site (in the new round-trip test) is the only place that sets all fields explicitly, and it's correct. Independently verified the legacy-payload-compat contract by running the test under cargo test. PASS.

• Reliability — Pure type extension; no async, no I/O, no time-sensitive code, no new error path. The schema-then-struct two-stage parse on the etcd loader path is unchanged. PASS.

• Security — No new credential flow; secret semantics unchanged. The new fields are operator-supplied attribution labels with no privileged interpretation in this PR (no dispatch site reads them). AISIX-Cloud#302 §5 places the validation responsibility on cp-api's createProviderKey — the DP is correctly second-line-of-defence-only, which matches the spec. PASS for this PR (see LOW 2 for the future hardening alignment with §5).

• Sensitive-info leakage — New fields are non-secret by construction (vendor identity, adapter shape, telemetry tags). The pre-existing Debug impl on ProviderKey already leaks secret (out of scope here), and the new fields do not widen that exposure. The new Debug impl on TelemetryTags is fine — all fields are labels intended for log/metric emission anyway. PASS.

• Breaking changes — aisix-core is a workspace-internal crate (no publish = true in Cargo.toml). New struct fields are all #[serde(default)] so the wire format adds optional fields only. additionalProperties: false on the schema continues to apply, with the three new fields explicitly enumerated. Existing on-disk KV payloads with no provider / adapter / telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes and legacy_payload_without_phase_a_fields_deserialises_with_defaults). The new pub use TelemetryTags does not collide with any existing export (verified by rg "TelemetryTags" crates/ — only the new declaration and re-exports appear). PASS.

• E2E test coverage — Adequate for the "skeleton, no consumer" contract this PR claims. The 14 new tests cover both directions:

  • Schema-level: minimal, legacy-no-phase-a, full-phase-a, byo-shape, unknown-adapter, unknown-telemetry-field, unknown-top-level, unknown-kind.
  • Struct-level: legacy-no-phase-a-defaults, full-phase-a, byo-shape, unknown-telemetry-field, unknown-adapter, round-trip-with-defaults.
    Both layers are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No cross-crate or HTTP-level e2e needed in this PR because there is no behavior under test — the dispatch wiring lands in the follow-up Phase A sub-PR per the PR description. PASS.

Independent edge-case probe (not added to the PR — purely for audit verification, see findings above)

Verified the following edge cases by spawning a temporary integration test in crates/aisix-core/tests/audit_check.rs (since deleted, working tree is clean):

Payload fragment	Serde result	Schema result	Outcome
"provider": null	Err — invalid type: null, expected a string	Err — null is not of type string	Both gate; OK
"telemetry_tags": null	Err — invalid type: null, expected struct	Err — null is not of type object	Both gate; OK
"adapter": null	Ok — None	Err — null is not one of [openai,…]	Divergence; see LOW 3
omitted (legacy)	Ok — defaults	Ok	Compat pinned
"telemetry_tags": {}	Ok — default	Ok	Compat pinned
provider = 100k chars	Ok	Ok	See LOW 2
Round-trip of TelemetryTags::default()	Emits {"featured": false} (kind/branded_provider/pk_label/byo_label all skip_serializing_if)	Schema accepts	OK

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Summary

Second sub-PR of api7/AISIX-Cloud#302 Phase A (DP-side Provider→Adapter refactor). Builds on #297 (which landed the Adapter enum).

This PR is intentionally zero-behavior-change — pure type extension on ProviderKey. Nothing in the gateway reads the new fields yet; Provider continues to drive 100% of dispatch.

What this PR does

1. Adds four new fields to aisix_core::ProviderKey:

   • provider: String — vendor identity (e.g. "deepseek", "openai"). Free-form String in this PR; closed-set validation is deferred to a later Phase A sub-PR that wires dispatch.
   • adapter: Option<Adapter> — wire-shape, pinned to the closed Adapter enum from feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297. None until a follow-up populates it.
   • telemetry_tags: TelemetryTags — attribution tags emitted alongside requests routed through this key.

2. Adds a new TelemetryTags struct with five optional fields:

   • kind: Option<String> — closed-set "catalog" | "byo" (enforced by the JSON Schema; the Rust type is Option<String> to stay forward-compatible if cp-api ships a new variant ahead of a DP rollout)
   • featured: bool — defaults to false
   • branded_provider: Option<String>
   • pk_label: Option<String>
   • byo_label: Option<String>

   TelemetryTags derives Default and is #[serde(deny_unknown_fields)] so an unknown tag from cp-api fails loudly on the DP rather than silently dropping.

3. Updates the provider_key JSON Schema with matching optional properties. additionalProperties: false is preserved both at the top level and inside telemetry_tags. adapter is constrained to the five Adapter enum values; kind is constrained to "catalog" | "byo".

4. Re-exports TelemetryTags from aisix_core::models and the crate root, mirroring Adapter.

Backward compatibility

All four new struct fields use #[serde(default)]. A pre-#302 payload like

{"display_name":"openai-prod","secret":"sk-x","api_base":"https://api.openai.com/v1"}

still deserializes — provider lands as "", adapter as None, telemetry_tags as TelemetryTags::default(). A dedicated test pins this contract (legacy_payload_without_phase_a_fields_deserialises_with_defaults).

The JSON Schema mirrors this: only display_name and secret remain required.

What this PR does NOT do

• Does not remove or modify Provider
• Does not change display_name (pre-existed on ProviderKey)
• Does not touch secret / api_base
• Does not change Hub, Bridges, dispatch, or any wire transform
• Does not change mustMarshalProviderKeyKV on the CP side (that's Phase D)
• Does not delete the wrapper crate
• Does not touch rerank.rs (A5) or OpenAiBridge (A4)

Design notes

• Option<Adapter> over Adapter::Default: keeping adapterNone-able makes "Phase A hasn't backfilled this yet" representable. Adding a default variant would lie about adapter shape for un-tagged keys.
• TelemetryTags as a struct (not HashMap<String, Value>): the catalog/byo attribution shape is a closed set known to both cp-api and the DP; a typed struct with deny_unknown_fields catches drift between the two sides at parse time. A free-form map would silently swallow typos.

Test plan

• cargo test -p aisix-core — 150 passed, 0 failed (+14 over feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297)
• cargo clippy --workspace --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean
• Legacy payload without Phase A fields deserializes (compat contract)
• Full Phase A payload (catalog shape) deserializes and round-trips
• BYO telemetry shape (branded_provider:null + byo_label) deserializes
• Unknown adapter string rejected (closed-set guard)
• Unknown telemetry_tags field rejected (deny_unknown_fields guard)
• Unknown telemetry_tags.kind value rejected at schema layer
• Schema accepts both legacy and Phase A payloads; rejects unknown top-level fields

References

• Tracking issue: api7/AISIX-Cloud#302
• Predecessor: feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297 (Adapter enum)

Summary by CodeRabbit

• New Features

  • Extended provider configuration to support telemetry tagging, adapter specification, and additional provider metadata fields while maintaining backward compatibility with existing payloads.

• Tests

  • Added comprehensive test coverage for new schema fields, legacy payload compatibility, and validation constraints.

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: ddb92b36-e041-4434-bcf7-10268b7dc3ca

📥 Commits

Reviewing files that changed from the base of the PR and between 83c48c9 and 28024f6.

📒 Files selected for processing (4)

• crates/aisix-core/src/lib.rs
• crates/aisix-core/src/models/mod.rs
• crates/aisix-core/src/models/provider_key.rs
• crates/aisix-core/src/models/schema.rs

---

📝 Walkthrough

Walkthrough

This PR extends the ProviderKey model with Phase A fields (provider, adapter, telemetry_tags) to support provider identity and telemetry attribution. A new TelemetryTags struct is introduced with serde defaults and strict field validation. The changes maintain backward compatibility so legacy payloads deserialize with zero values, and comprehensive tests verify both JSON schema validation and round-trip serialization.

Changes

Provider Key Phase A Extension

Layer / File(s)	Summary
Type definitions and public exports crates/aisix-core/src/models/provider_key.rs, crates/aisix-core/src/models/mod.rs, crates/aisix-core/src/lib.rs	New TelemetryTags struct introduced with serde defaults and deny_unknown_fields attribute. ProviderKey extended with three new fields: provider (string), adapter (optional), and telemetry_tags. Adapter imported to support the new field type. Public re-exports in mod.rs and lib.rs updated to expose TelemetryTags.
JSON schema validation and documentation crates/aisix-core/src/models/schema.rs	provider_key_schema() extended with Phase A fields as optional (preserving backward compatibility). Documentation added explaining that provider, adapter, and telemetry_tags are not yet wired to dispatch behavior. Schema tests added covering minimal payloads, legacy payloads without Phase A fields, and payloads with Phase A fields including both catalog and BYO telemetry shapes. Rejection tests verify unknown adapter values, unknown telemetry fields, and invalid kind discriminator values are caught.
Deserialization and round-trip compatibility tests crates/aisix-core/src/models/provider_key.rs	Test suite verifies legacy payloads without Phase A fields deserialize to defaults. Payloads with all Phase A fields (including telemetry tag values and BYO dual-label handling) deserialize correctly. adapter field rejects unknown adapter strings. telemetry_tags rejects unknown fields. A ProviderKey containing default Phase A fields successfully round-trips via JSON without losing semantic equality.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

---

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[moonming]

Independent audit (CLAUDE.md §8)

Cold re-read of the diff via gh pr diff 298 — no shared context with the implementation session. Reviewed against the six required angles.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. No maxLength bound on free-form telemetry fields (crates/aisix-core/src/models/schema.rs:288-307). branded_provider, pk_label, byo_label, and the top-level provider are all unbounded strings in the JSON Schema. A misbehaving (or compromised) cp-api could publish a 10MB-per-tag value and the DP would happily persist it into the snapshot. This is consistent with the pre-existing lack of maxLength on display_name/secret/api_base, so it is not a regression introduced here — but it is worth tightening in a follow-up Phase A PR (suggest 120 chars to match cache_policy.name).
   Suggested follow-up edit:

   "provider": { "type": "string", "maxLength": 120 },
   "branded_provider": { "type": ["string", "null"], "maxLength": 120 },
   "pk_label": { "type": ["string", "null"], "maxLength": 120 },
   "byo_label": { "type": ["string", "null"], "maxLength": 120 }

2. Schema does not enforce the catalog/byo mutual-exclusion shape (crates/aisix-core/src/models/schema.rs:288-307). The PR description notes that the canonical BYO shape has branded_provider:null + byo_label="…" and the catalog shape has branded_provider:"vendor" + byo_label:null, but the schema allows any combination (both set, both null, etc.). This is a deliberate Phase A skeleton choice — no dispatch reads the field yet, and adding a conditional oneOf now would foreclose what Phase A migration learns about real cp-api payloads. Worth pinning the constraint once Phase A wires the telemetry consumer.

3. TelemetryTags::Default produces all-None including kind: None, but the schema constrains kind to "catalog"|"byo" (no null allowed). A round-trip of TelemetryTags::default() via serde_json::to_value produces {} (because of skip_serializing_if = "Option::is_none"), which is schema-valid. A round-trip via serde_json::to_value with serialize_none = true (not used today) would emit kind: null and fail schema. This is a non-issue with current code, but worth a comment near the struct definition so a future PR doesn't add a null-emitting serializer.

Per-angle assessment

• Correctness: The PR claims "zero behavior change" and the diff bears it out — no dispatch site, snapshot loader, hub, or bridge reads the new fields. Verified by running cargo test --workspace (all 18 crate test suites pass, no downstream breakage). All five test helpers across the workspace that build ProviderKey go through serde_json::from_str, never struct-literal syntax, so the addition of new required-but-defaulted fields does not break any existing test. PASS.
• Reliability: Pure type extension; no async, no I/O, no time-sensitive code. PASS.
• Security: No new credential flow; secret semantics unchanged. The new fields are operator-supplied labels with no privileged interpretation in this PR. PASS for this PR (see LOW 1 for a hardening follow-up).
• Sensitive-info leakage: New fields are non-secret. The Debug impl on ProviderKey already leaks secret (pre-existing), and the new fields do not widen that exposure. PASS.
• Breaking changes: aisix-core is a workspace-internal crate (no publish = true). All in-workspace consumers reach ProviderKey via serde_json, never struct-literal construction with full field list — verified by grep. The wire format adds optional fields only; additionalProperties: false plus the existing on-disk payloads with no provider/adapter/telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes). PASS.
• E2E test coverage: Adequate for the "skeleton, no consumer" contract this PR claims. Both struct-level and schema-level legacy-payload-compat tests are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No need for cross-crate or HTTP-level e2e in this PR because there is no behavior under test — that lands in the follow-up Phase A sub-PR that wires dispatch.

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Independent third-party audit per CLAUDE.md §8 (separate from author self-review)

Cold re-read of this PR by a fresh agent with no shared context with the implementation session or with the author's own audit comment above. Reviewed against the six required angles and against the canonical wire shapes in api7/AISIX-Cloud#302 §5.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. PR title slightly misrepresents the diff. The title says "add provider/adapter/display_name fields" but display_name is pre-existing on ProviderKey (verified by git show main:crates/aisix-core/src/models/provider_key.rs). The diff only adds provider, adapter, and telemetry_tags. The body is accurate ("Adds four new fields…" actually says 4 but lists 3 — display_name is correctly absent from the body's enumeration). The body explicitly says "Does not change display_name (pre-existed on ProviderKey)". Non-blocking — squash-merge commit title is what would land on main, suggest tightening to feat(core): add provider/adapter/telemetry_tags fields to ProviderKey (Phase A skeleton).

2. No maxLength on any of the new string fields (crates/aisix-core/src/models/schema.rs:288-307). Independently verified: a 100,000-char provider / pk_label / byo_label / branded_provider passes both schema validation and serde deserialisation. Consistent with the pre-existing lack of maxLength on display_name / secret / api_base, so not a regression — but AISIX-Cloud#302 §5 specifies display_name length 1-64 with sluggified-form ≥ 3. The DP currently enforces neither, leaving cp-api as the sole gatekeeper for label length. Worth tightening in a follow-up alongside the §5 validation work cp-api is committing to.

3. adapter:null is silently accepted by serde but rejected by schema. Independently verified:

   • Struct path: serde_json::from_str of {"adapter": null, …} → adapter = None, Ok(()).
   • Schema path: validate_provider_key of the same → rejected ("null is not one of [openai,…]").
   • Because the etcd loader runs schema validation before struct deserialisation, a wire payload with adapter:null is rejected at the schema gate, so this divergence has no observable impact in this PR. But it is worth a one-line code comment near the adapter field saying "schema is the gatekeeper for null-vs-absent; the serde Option only handles the absent case". Otherwise a future PR that builds a ProviderKey from a serde_json::Valuebypassing the schema (e.g. a unit test reading a fixture, or a CLI tool) could see adapter:None semantics for a payload that the schema would have rejected.

Per-angle assessment

• Correctness — PR claims "zero behavior change" and the diff bears it out. No dispatch site, snapshot loader, hub, or bridge reads the new fields. Independently verified by running cargo test -p aisix-core (150 passed, 0 failed) and cargo check --workspace (clean). All five ProviderKey builder helpers across the workspace (aisix-gateway/src/bridge.rs:361, aisix-provider-anthropic/src/bridge.rs:553, aisix-provider-openai/src/bridge.rs:787, aisix-core/src/models/snapshot.rs:74, aisix-proxy/src/dispatch.rs:283) use serde_json::from_str, never struct-literal — so the new fields ride on #[serde(default)] and existing tests don't need to be touched. The single struct-literal site (in the new round-trip test) is the only place that sets all fields explicitly, and it's correct. Independently verified the legacy-payload-compat contract by running the test under cargo test. PASS.

• Reliability — Pure type extension; no async, no I/O, no time-sensitive code, no new error path. The schema-then-struct two-stage parse on the etcd loader path is unchanged. PASS.

• Security — No new credential flow; secret semantics unchanged. The new fields are operator-supplied attribution labels with no privileged interpretation in this PR (no dispatch site reads them). AISIX-Cloud#302 §5 places the validation responsibility on cp-api's createProviderKey — the DP is correctly second-line-of-defence-only, which matches the spec. PASS for this PR (see LOW 2 for the future hardening alignment with §5).

• Sensitive-info leakage — New fields are non-secret by construction (vendor identity, adapter shape, telemetry tags). The pre-existing Debug impl on ProviderKey already leaks secret (out of scope here), and the new fields do not widen that exposure. The new Debug impl on TelemetryTags is fine — all fields are labels intended for log/metric emission anyway. PASS.

• Breaking changes — aisix-core is a workspace-internal crate (no publish = true in Cargo.toml). New struct fields are all #[serde(default)] so the wire format adds optional fields only. additionalProperties: false on the schema continues to apply, with the three new fields explicitly enumerated. Existing on-disk KV payloads with no provider / adapter / telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes and legacy_payload_without_phase_a_fields_deserialises_with_defaults). The new pub use TelemetryTags does not collide with any existing export (verified by rg "TelemetryTags" crates/ — only the new declaration and re-exports appear). PASS.

• E2E test coverage — Adequate for the "skeleton, no consumer" contract this PR claims. The 14 new tests cover both directions:

  • Schema-level: minimal, legacy-no-phase-a, full-phase-a, byo-shape, unknown-adapter, unknown-telemetry-field, unknown-top-level, unknown-kind.
  • Struct-level: legacy-no-phase-a-defaults, full-phase-a, byo-shape, unknown-telemetry-field, unknown-adapter, round-trip-with-defaults.
    Both layers are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No cross-crate or HTTP-level e2e needed in this PR because there is no behavior under test — the dispatch wiring lands in the follow-up Phase A sub-PR per the PR description. PASS.

Independent edge-case probe (not added to the PR — purely for audit verification, see findings above)

Verified the following edge cases by spawning a temporary integration test in crates/aisix-core/tests/audit_check.rs (since deleted, working tree is clean):

Payload fragment	Serde result	Schema result	Outcome
"provider": null	Err — invalid type: null, expected a string	Err — null is not of type string	Both gate; OK
"telemetry_tags": null	Err — invalid type: null, expected struct	Err — null is not of type object	Both gate; OK
"adapter": null	Ok — None	Err — null is not one of [openai,…]	Divergence; see LOW 3
omitted (legacy)	Ok — defaults	Ok	Compat pinned
"telemetry_tags": {}	Ok — default	Ok	Compat pinned
provider = 100k chars	Ok	Ok	See LOW 2
Round-trip of TelemetryTags::default()	Emits {"featured": false} (kind/branded_provider/pk_label/byo_label all skip_serializing_if)	Schema accepts	OK

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Summary

Second sub-PR of api7/AISIX-Cloud#302 Phase A (DP-side Provider→Adapter refactor). Builds on #297 (which landed the Adapter enum).

This PR is intentionally zero-behavior-change — pure type extension on ProviderKey. Nothing in the gateway reads the new fields yet; Provider continues to drive 100% of dispatch.

What this PR does

1. Adds four new fields to aisix_core::ProviderKey:

   • provider: String — vendor identity (e.g. "deepseek", "openai"). Free-form String in this PR; closed-set validation is deferred to a later Phase A sub-PR that wires dispatch.
   • adapter: Option<Adapter> — wire-shape, pinned to the closed Adapter enum from feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297. None until a follow-up populates it.
   • telemetry_tags: TelemetryTags — attribution tags emitted alongside requests routed through this key.

2. Adds a new TelemetryTags struct with five optional fields:

   • kind: Option<String> — closed-set "catalog" | "byo" (enforced by the JSON Schema; the Rust type is Option<String> to stay forward-compatible if cp-api ships a new variant ahead of a DP rollout)
   • featured: bool — defaults to false
   • branded_provider: Option<String>
   • pk_label: Option<String>
   • byo_label: Option<String>

   TelemetryTags derives Default and is #[serde(deny_unknown_fields)] so an unknown tag from cp-api fails loudly on the DP rather than silently dropping.

3. Updates the provider_key JSON Schema with matching optional properties. additionalProperties: false is preserved both at the top level and inside telemetry_tags. adapter is constrained to the five Adapter enum values; kind is constrained to "catalog" | "byo".

4. Re-exports TelemetryTags from aisix_core::models and the crate root, mirroring Adapter.

Backward compatibility

All four new struct fields use #[serde(default)]. A pre-#302 payload like

{"display_name":"openai-prod","secret":"sk-x","api_base":"https://api.openai.com/v1"}

still deserializes — provider lands as "", adapter as None, telemetry_tags as TelemetryTags::default(). A dedicated test pins this contract (legacy_payload_without_phase_a_fields_deserialises_with_defaults).

The JSON Schema mirrors this: only display_name and secret remain required.

What this PR does NOT do

• Does not remove or modify Provider
• Does not change display_name (pre-existed on ProviderKey)
• Does not touch secret / api_base
• Does not change Hub, Bridges, dispatch, or any wire transform
• Does not change mustMarshalProviderKeyKV on the CP side (that's Phase D)
• Does not delete the wrapper crate
• Does not touch rerank.rs (A5) or OpenAiBridge (A4)

Design notes

• Option<Adapter> over Adapter::Default: keeping adapterNone-able makes "Phase A hasn't backfilled this yet" representable. Adding a default variant would lie about adapter shape for un-tagged keys.
• TelemetryTags as a struct (not HashMap<String, Value>): the catalog/byo attribution shape is a closed set known to both cp-api and the DP; a typed struct with deny_unknown_fields catches drift between the two sides at parse time. A free-form map would silently swallow typos.

Test plan

• cargo test -p aisix-core — 150 passed, 0 failed (+14 over feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297)
• cargo clippy --workspace --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean
• Legacy payload without Phase A fields deserializes (compat contract)
• Full Phase A payload (catalog shape) deserializes and round-trips
• BYO telemetry shape (branded_provider:null + byo_label) deserializes
• Unknown adapter string rejected (closed-set guard)
• Unknown telemetry_tags field rejected (deny_unknown_fields guard)
• Unknown telemetry_tags.kind value rejected at schema layer
• Schema accepts both legacy and Phase A payloads; rejects unknown top-level fields

References

• Tracking issue: api7/AISIX-Cloud#302
• Predecessor: feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297 (Adapter enum)

Summary by CodeRabbit

• New Features

  • Extended provider configuration to support telemetry tagging, adapter specification, and additional provider metadata fields while maintaining backward compatibility with existing payloads.

• Tests

  • Added comprehensive test coverage for new schema fields, legacy payload compatibility, and validation constraints.

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: ddb92b36-e041-4434-bcf7-10268b7dc3ca

📥 Commits

Reviewing files that changed from the base of the PR and between 83c48c9 and 28024f6.

📒 Files selected for processing (4)

• crates/aisix-core/src/lib.rs
• crates/aisix-core/src/models/mod.rs
• crates/aisix-core/src/models/provider_key.rs
• crates/aisix-core/src/models/schema.rs

---

📝 Walkthrough

Walkthrough

This PR extends the ProviderKey model with Phase A fields (provider, adapter, telemetry_tags) to support provider identity and telemetry attribution. A new TelemetryTags struct is introduced with serde defaults and strict field validation. The changes maintain backward compatibility so legacy payloads deserialize with zero values, and comprehensive tests verify both JSON schema validation and round-trip serialization.

Changes

Provider Key Phase A Extension

Layer / File(s)	Summary
Type definitions and public exports crates/aisix-core/src/models/provider_key.rs, crates/aisix-core/src/models/mod.rs, crates/aisix-core/src/lib.rs	New TelemetryTags struct introduced with serde defaults and deny_unknown_fields attribute. ProviderKey extended with three new fields: provider (string), adapter (optional), and telemetry_tags. Adapter imported to support the new field type. Public re-exports in mod.rs and lib.rs updated to expose TelemetryTags.
JSON schema validation and documentation crates/aisix-core/src/models/schema.rs	provider_key_schema() extended with Phase A fields as optional (preserving backward compatibility). Documentation added explaining that provider, adapter, and telemetry_tags are not yet wired to dispatch behavior. Schema tests added covering minimal payloads, legacy payloads without Phase A fields, and payloads with Phase A fields including both catalog and BYO telemetry shapes. Rejection tests verify unknown adapter values, unknown telemetry fields, and invalid kind discriminator values are caught.
Deserialization and round-trip compatibility tests crates/aisix-core/src/models/provider_key.rs	Test suite verifies legacy payloads without Phase A fields deserialize to defaults. Payloads with all Phase A fields (including telemetry tag values and BYO dual-label handling) deserialize correctly. adapter field rejects unknown adapter strings. telemetry_tags rejects unknown fields. A ProviderKey containing default Phase A fields successfully round-trips via JSON without losing semantic equality.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

---

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[moonming]

Independent audit (CLAUDE.md §8)

Cold re-read of the diff via gh pr diff 298 — no shared context with the implementation session. Reviewed against the six required angles.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. No maxLength bound on free-form telemetry fields (crates/aisix-core/src/models/schema.rs:288-307). branded_provider, pk_label, byo_label, and the top-level provider are all unbounded strings in the JSON Schema. A misbehaving (or compromised) cp-api could publish a 10MB-per-tag value and the DP would happily persist it into the snapshot. This is consistent with the pre-existing lack of maxLength on display_name/secret/api_base, so it is not a regression introduced here — but it is worth tightening in a follow-up Phase A PR (suggest 120 chars to match cache_policy.name).
   Suggested follow-up edit:

   "provider": { "type": "string", "maxLength": 120 },
   "branded_provider": { "type": ["string", "null"], "maxLength": 120 },
   "pk_label": { "type": ["string", "null"], "maxLength": 120 },
   "byo_label": { "type": ["string", "null"], "maxLength": 120 }

2. Schema does not enforce the catalog/byo mutual-exclusion shape (crates/aisix-core/src/models/schema.rs:288-307). The PR description notes that the canonical BYO shape has branded_provider:null + byo_label="…" and the catalog shape has branded_provider:"vendor" + byo_label:null, but the schema allows any combination (both set, both null, etc.). This is a deliberate Phase A skeleton choice — no dispatch reads the field yet, and adding a conditional oneOf now would foreclose what Phase A migration learns about real cp-api payloads. Worth pinning the constraint once Phase A wires the telemetry consumer.

3. TelemetryTags::Default produces all-None including kind: None, but the schema constrains kind to "catalog"|"byo" (no null allowed). A round-trip of TelemetryTags::default() via serde_json::to_value produces {} (because of skip_serializing_if = "Option::is_none"), which is schema-valid. A round-trip via serde_json::to_value with serialize_none = true (not used today) would emit kind: null and fail schema. This is a non-issue with current code, but worth a comment near the struct definition so a future PR doesn't add a null-emitting serializer.

Per-angle assessment

• Correctness: The PR claims "zero behavior change" and the diff bears it out — no dispatch site, snapshot loader, hub, or bridge reads the new fields. Verified by running cargo test --workspace (all 18 crate test suites pass, no downstream breakage). All five test helpers across the workspace that build ProviderKey go through serde_json::from_str, never struct-literal syntax, so the addition of new required-but-defaulted fields does not break any existing test. PASS.
• Reliability: Pure type extension; no async, no I/O, no time-sensitive code. PASS.
• Security: No new credential flow; secret semantics unchanged. The new fields are operator-supplied labels with no privileged interpretation in this PR. PASS for this PR (see LOW 1 for a hardening follow-up).
• Sensitive-info leakage: New fields are non-secret. The Debug impl on ProviderKey already leaks secret (pre-existing), and the new fields do not widen that exposure. PASS.
• Breaking changes: aisix-core is a workspace-internal crate (no publish = true). All in-workspace consumers reach ProviderKey via serde_json, never struct-literal construction with full field list — verified by grep. The wire format adds optional fields only; additionalProperties: false plus the existing on-disk payloads with no provider/adapter/telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes). PASS.
• E2E test coverage: Adequate for the "skeleton, no consumer" contract this PR claims. Both struct-level and schema-level legacy-payload-compat tests are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No need for cross-crate or HTTP-level e2e in this PR because there is no behavior under test — that lands in the follow-up Phase A sub-PR that wires dispatch.

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Independent third-party audit per CLAUDE.md §8 (separate from author self-review)

Cold re-read of this PR by a fresh agent with no shared context with the implementation session or with the author's own audit comment above. Reviewed against the six required angles and against the canonical wire shapes in api7/AISIX-Cloud#302 §5.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. PR title slightly misrepresents the diff. The title says "add provider/adapter/display_name fields" but display_name is pre-existing on ProviderKey (verified by git show main:crates/aisix-core/src/models/provider_key.rs). The diff only adds provider, adapter, and telemetry_tags. The body is accurate ("Adds four new fields…" actually says 4 but lists 3 — display_name is correctly absent from the body's enumeration). The body explicitly says "Does not change display_name (pre-existed on ProviderKey)". Non-blocking — squash-merge commit title is what would land on main, suggest tightening to feat(core): add provider/adapter/telemetry_tags fields to ProviderKey (Phase A skeleton).

2. No maxLength on any of the new string fields (crates/aisix-core/src/models/schema.rs:288-307). Independently verified: a 100,000-char provider / pk_label / byo_label / branded_provider passes both schema validation and serde deserialisation. Consistent with the pre-existing lack of maxLength on display_name / secret / api_base, so not a regression — but AISIX-Cloud#302 §5 specifies display_name length 1-64 with sluggified-form ≥ 3. The DP currently enforces neither, leaving cp-api as the sole gatekeeper for label length. Worth tightening in a follow-up alongside the §5 validation work cp-api is committing to.

3. adapter:null is silently accepted by serde but rejected by schema. Independently verified:

   • Struct path: serde_json::from_str of {"adapter": null, …} → adapter = None, Ok(()).
   • Schema path: validate_provider_key of the same → rejected ("null is not one of [openai,…]").
   • Because the etcd loader runs schema validation before struct deserialisation, a wire payload with adapter:null is rejected at the schema gate, so this divergence has no observable impact in this PR. But it is worth a one-line code comment near the adapter field saying "schema is the gatekeeper for null-vs-absent; the serde Option only handles the absent case". Otherwise a future PR that builds a ProviderKey from a serde_json::Valuebypassing the schema (e.g. a unit test reading a fixture, or a CLI tool) could see adapter:None semantics for a payload that the schema would have rejected.

Per-angle assessment

• Correctness — PR claims "zero behavior change" and the diff bears it out. No dispatch site, snapshot loader, hub, or bridge reads the new fields. Independently verified by running cargo test -p aisix-core (150 passed, 0 failed) and cargo check --workspace (clean). All five ProviderKey builder helpers across the workspace (aisix-gateway/src/bridge.rs:361, aisix-provider-anthropic/src/bridge.rs:553, aisix-provider-openai/src/bridge.rs:787, aisix-core/src/models/snapshot.rs:74, aisix-proxy/src/dispatch.rs:283) use serde_json::from_str, never struct-literal — so the new fields ride on #[serde(default)] and existing tests don't need to be touched. The single struct-literal site (in the new round-trip test) is the only place that sets all fields explicitly, and it's correct. Independently verified the legacy-payload-compat contract by running the test under cargo test. PASS.

• Reliability — Pure type extension; no async, no I/O, no time-sensitive code, no new error path. The schema-then-struct two-stage parse on the etcd loader path is unchanged. PASS.

• Security — No new credential flow; secret semantics unchanged. The new fields are operator-supplied attribution labels with no privileged interpretation in this PR (no dispatch site reads them). AISIX-Cloud#302 §5 places the validation responsibility on cp-api's createProviderKey — the DP is correctly second-line-of-defence-only, which matches the spec. PASS for this PR (see LOW 2 for the future hardening alignment with §5).

• Sensitive-info leakage — New fields are non-secret by construction (vendor identity, adapter shape, telemetry tags). The pre-existing Debug impl on ProviderKey already leaks secret (out of scope here), and the new fields do not widen that exposure. The new Debug impl on TelemetryTags is fine — all fields are labels intended for log/metric emission anyway. PASS.

• Breaking changes — aisix-core is a workspace-internal crate (no publish = true in Cargo.toml). New struct fields are all #[serde(default)] so the wire format adds optional fields only. additionalProperties: false on the schema continues to apply, with the three new fields explicitly enumerated. Existing on-disk KV payloads with no provider / adapter / telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes and legacy_payload_without_phase_a_fields_deserialises_with_defaults). The new pub use TelemetryTags does not collide with any existing export (verified by rg "TelemetryTags" crates/ — only the new declaration and re-exports appear). PASS.

• E2E test coverage — Adequate for the "skeleton, no consumer" contract this PR claims. The 14 new tests cover both directions:

  • Schema-level: minimal, legacy-no-phase-a, full-phase-a, byo-shape, unknown-adapter, unknown-telemetry-field, unknown-top-level, unknown-kind.
  • Struct-level: legacy-no-phase-a-defaults, full-phase-a, byo-shape, unknown-telemetry-field, unknown-adapter, round-trip-with-defaults.
    Both layers are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No cross-crate or HTTP-level e2e needed in this PR because there is no behavior under test — the dispatch wiring lands in the follow-up Phase A sub-PR per the PR description. PASS.

Independent edge-case probe (not added to the PR — purely for audit verification, see findings above)

Verified the following edge cases by spawning a temporary integration test in crates/aisix-core/tests/audit_check.rs (since deleted, working tree is clean):

Payload fragment	Serde result	Schema result	Outcome
"provider": null	Err — invalid type: null, expected a string	Err — null is not of type string	Both gate; OK
"telemetry_tags": null	Err — invalid type: null, expected struct	Err — null is not of type object	Both gate; OK
"adapter": null	Ok — None	Err — null is not one of [openai,…]	Divergence; see LOW 3
omitted (legacy)	Ok — defaults	Ok	Compat pinned
"telemetry_tags": {}	Ok — default	Ok	Compat pinned
provider = 100k chars	Ok	Ok	See LOW 2
Round-trip of TelemetryTags::default()	Emits {"featured": false} (kind/branded_provider/pk_label/byo_label all skip_serializing_if)	Schema accepts	OK

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Summary

Second sub-PR of api7/AISIX-Cloud#302 Phase A (DP-side Provider→Adapter refactor). Builds on #297 (which landed the Adapter enum).

This PR is intentionally zero-behavior-change — pure type extension on ProviderKey. Nothing in the gateway reads the new fields yet; Provider continues to drive 100% of dispatch.

What this PR does

1. Adds four new fields to aisix_core::ProviderKey:

   • provider: String — vendor identity (e.g. "deepseek", "openai"). Free-form String in this PR; closed-set validation is deferred to a later Phase A sub-PR that wires dispatch.
   • adapter: Option<Adapter> — wire-shape, pinned to the closed Adapter enum from feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297. None until a follow-up populates it.
   • telemetry_tags: TelemetryTags — attribution tags emitted alongside requests routed through this key.

2. Adds a new TelemetryTags struct with five optional fields:

   • kind: Option<String> — closed-set "catalog" | "byo" (enforced by the JSON Schema; the Rust type is Option<String> to stay forward-compatible if cp-api ships a new variant ahead of a DP rollout)
   • featured: bool — defaults to false
   • branded_provider: Option<String>
   • pk_label: Option<String>
   • byo_label: Option<String>

   TelemetryTags derives Default and is #[serde(deny_unknown_fields)] so an unknown tag from cp-api fails loudly on the DP rather than silently dropping.

3. Updates the provider_key JSON Schema with matching optional properties. additionalProperties: false is preserved both at the top level and inside telemetry_tags. adapter is constrained to the five Adapter enum values; kind is constrained to "catalog" | "byo".

4. Re-exports TelemetryTags from aisix_core::models and the crate root, mirroring Adapter.

Backward compatibility

All four new struct fields use #[serde(default)]. A pre-#302 payload like

{"display_name":"openai-prod","secret":"sk-x","api_base":"https://api.openai.com/v1"}

still deserializes — provider lands as "", adapter as None, telemetry_tags as TelemetryTags::default(). A dedicated test pins this contract (legacy_payload_without_phase_a_fields_deserialises_with_defaults).

The JSON Schema mirrors this: only display_name and secret remain required.

What this PR does NOT do

• Does not remove or modify Provider
• Does not change display_name (pre-existed on ProviderKey)
• Does not touch secret / api_base
• Does not change Hub, Bridges, dispatch, or any wire transform
• Does not change mustMarshalProviderKeyKV on the CP side (that's Phase D)
• Does not delete the wrapper crate
• Does not touch rerank.rs (A5) or OpenAiBridge (A4)

Design notes

• Option<Adapter> over Adapter::Default: keeping adapterNone-able makes "Phase A hasn't backfilled this yet" representable. Adding a default variant would lie about adapter shape for un-tagged keys.
• TelemetryTags as a struct (not HashMap<String, Value>): the catalog/byo attribution shape is a closed set known to both cp-api and the DP; a typed struct with deny_unknown_fields catches drift between the two sides at parse time. A free-form map would silently swallow typos.

Test plan

• cargo test -p aisix-core — 150 passed, 0 failed (+14 over feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297)
• cargo clippy --workspace --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean
• Legacy payload without Phase A fields deserializes (compat contract)
• Full Phase A payload (catalog shape) deserializes and round-trips
• BYO telemetry shape (branded_provider:null + byo_label) deserializes
• Unknown adapter string rejected (closed-set guard)
• Unknown telemetry_tags field rejected (deny_unknown_fields guard)
• Unknown telemetry_tags.kind value rejected at schema layer
• Schema accepts both legacy and Phase A payloads; rejects unknown top-level fields

References

• Tracking issue: api7/AISIX-Cloud#302
• Predecessor: feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297 (Adapter enum)

Summary by CodeRabbit

• New Features

  • Extended provider configuration to support telemetry tagging, adapter specification, and additional provider metadata fields while maintaining backward compatibility with existing payloads.

• Tests

  • Added comprehensive test coverage for new schema fields, legacy payload compatibility, and validation constraints.

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: ddb92b36-e041-4434-bcf7-10268b7dc3ca

📥 Commits

Reviewing files that changed from the base of the PR and between 83c48c9 and 28024f6.

📒 Files selected for processing (4)

• crates/aisix-core/src/lib.rs
• crates/aisix-core/src/models/mod.rs
• crates/aisix-core/src/models/provider_key.rs
• crates/aisix-core/src/models/schema.rs

---

📝 Walkthrough

Walkthrough

This PR extends the ProviderKey model with Phase A fields (provider, adapter, telemetry_tags) to support provider identity and telemetry attribution. A new TelemetryTags struct is introduced with serde defaults and strict field validation. The changes maintain backward compatibility so legacy payloads deserialize with zero values, and comprehensive tests verify both JSON schema validation and round-trip serialization.

Changes

Provider Key Phase A Extension

Layer / File(s)	Summary
Type definitions and public exports crates/aisix-core/src/models/provider_key.rs, crates/aisix-core/src/models/mod.rs, crates/aisix-core/src/lib.rs	New TelemetryTags struct introduced with serde defaults and deny_unknown_fields attribute. ProviderKey extended with three new fields: provider (string), adapter (optional), and telemetry_tags. Adapter imported to support the new field type. Public re-exports in mod.rs and lib.rs updated to expose TelemetryTags.
JSON schema validation and documentation crates/aisix-core/src/models/schema.rs	provider_key_schema() extended with Phase A fields as optional (preserving backward compatibility). Documentation added explaining that provider, adapter, and telemetry_tags are not yet wired to dispatch behavior. Schema tests added covering minimal payloads, legacy payloads without Phase A fields, and payloads with Phase A fields including both catalog and BYO telemetry shapes. Rejection tests verify unknown adapter values, unknown telemetry fields, and invalid kind discriminator values are caught.
Deserialization and round-trip compatibility tests crates/aisix-core/src/models/provider_key.rs	Test suite verifies legacy payloads without Phase A fields deserialize to defaults. Payloads with all Phase A fields (including telemetry tag values and BYO dual-label handling) deserialize correctly. adapter field rejects unknown adapter strings. telemetry_tags rejects unknown fields. A ProviderKey containing default Phase A fields successfully round-trips via JSON without losing semantic equality.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

---

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[moonming]

Independent audit (CLAUDE.md §8)

Cold re-read of the diff via gh pr diff 298 — no shared context with the implementation session. Reviewed against the six required angles.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. No maxLength bound on free-form telemetry fields (crates/aisix-core/src/models/schema.rs:288-307). branded_provider, pk_label, byo_label, and the top-level provider are all unbounded strings in the JSON Schema. A misbehaving (or compromised) cp-api could publish a 10MB-per-tag value and the DP would happily persist it into the snapshot. This is consistent with the pre-existing lack of maxLength on display_name/secret/api_base, so it is not a regression introduced here — but it is worth tightening in a follow-up Phase A PR (suggest 120 chars to match cache_policy.name).
   Suggested follow-up edit:

   "provider": { "type": "string", "maxLength": 120 },
   "branded_provider": { "type": ["string", "null"], "maxLength": 120 },
   "pk_label": { "type": ["string", "null"], "maxLength": 120 },
   "byo_label": { "type": ["string", "null"], "maxLength": 120 }

2. Schema does not enforce the catalog/byo mutual-exclusion shape (crates/aisix-core/src/models/schema.rs:288-307). The PR description notes that the canonical BYO shape has branded_provider:null + byo_label="…" and the catalog shape has branded_provider:"vendor" + byo_label:null, but the schema allows any combination (both set, both null, etc.). This is a deliberate Phase A skeleton choice — no dispatch reads the field yet, and adding a conditional oneOf now would foreclose what Phase A migration learns about real cp-api payloads. Worth pinning the constraint once Phase A wires the telemetry consumer.

3. TelemetryTags::Default produces all-None including kind: None, but the schema constrains kind to "catalog"|"byo" (no null allowed). A round-trip of TelemetryTags::default() via serde_json::to_value produces {} (because of skip_serializing_if = "Option::is_none"), which is schema-valid. A round-trip via serde_json::to_value with serialize_none = true (not used today) would emit kind: null and fail schema. This is a non-issue with current code, but worth a comment near the struct definition so a future PR doesn't add a null-emitting serializer.

Per-angle assessment

• Correctness: The PR claims "zero behavior change" and the diff bears it out — no dispatch site, snapshot loader, hub, or bridge reads the new fields. Verified by running cargo test --workspace (all 18 crate test suites pass, no downstream breakage). All five test helpers across the workspace that build ProviderKey go through serde_json::from_str, never struct-literal syntax, so the addition of new required-but-defaulted fields does not break any existing test. PASS.
• Reliability: Pure type extension; no async, no I/O, no time-sensitive code. PASS.
• Security: No new credential flow; secret semantics unchanged. The new fields are operator-supplied labels with no privileged interpretation in this PR. PASS for this PR (see LOW 1 for a hardening follow-up).
• Sensitive-info leakage: New fields are non-secret. The Debug impl on ProviderKey already leaks secret (pre-existing), and the new fields do not widen that exposure. PASS.
• Breaking changes: aisix-core is a workspace-internal crate (no publish = true). All in-workspace consumers reach ProviderKey via serde_json, never struct-literal construction with full field list — verified by grep. The wire format adds optional fields only; additionalProperties: false plus the existing on-disk payloads with no provider/adapter/telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes). PASS.
• E2E test coverage: Adequate for the "skeleton, no consumer" contract this PR claims. Both struct-level and schema-level legacy-payload-compat tests are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No need for cross-crate or HTTP-level e2e in this PR because there is no behavior under test — that lands in the follow-up Phase A sub-PR that wires dispatch.

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Independent third-party audit per CLAUDE.md §8 (separate from author self-review)

Cold re-read of this PR by a fresh agent with no shared context with the implementation session or with the author's own audit comment above. Reviewed against the six required angles and against the canonical wire shapes in api7/AISIX-Cloud#302 §5.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. PR title slightly misrepresents the diff. The title says "add provider/adapter/display_name fields" but display_name is pre-existing on ProviderKey (verified by git show main:crates/aisix-core/src/models/provider_key.rs). The diff only adds provider, adapter, and telemetry_tags. The body is accurate ("Adds four new fields…" actually says 4 but lists 3 — display_name is correctly absent from the body's enumeration). The body explicitly says "Does not change display_name (pre-existed on ProviderKey)". Non-blocking — squash-merge commit title is what would land on main, suggest tightening to feat(core): add provider/adapter/telemetry_tags fields to ProviderKey (Phase A skeleton).

2. No maxLength on any of the new string fields (crates/aisix-core/src/models/schema.rs:288-307). Independently verified: a 100,000-char provider / pk_label / byo_label / branded_provider passes both schema validation and serde deserialisation. Consistent with the pre-existing lack of maxLength on display_name / secret / api_base, so not a regression — but AISIX-Cloud#302 §5 specifies display_name length 1-64 with sluggified-form ≥ 3. The DP currently enforces neither, leaving cp-api as the sole gatekeeper for label length. Worth tightening in a follow-up alongside the §5 validation work cp-api is committing to.

3. adapter:null is silently accepted by serde but rejected by schema. Independently verified:

   • Struct path: serde_json::from_str of {"adapter": null, …} → adapter = None, Ok(()).
   • Schema path: validate_provider_key of the same → rejected ("null is not one of [openai,…]").
   • Because the etcd loader runs schema validation before struct deserialisation, a wire payload with adapter:null is rejected at the schema gate, so this divergence has no observable impact in this PR. But it is worth a one-line code comment near the adapter field saying "schema is the gatekeeper for null-vs-absent; the serde Option only handles the absent case". Otherwise a future PR that builds a ProviderKey from a serde_json::Valuebypassing the schema (e.g. a unit test reading a fixture, or a CLI tool) could see adapter:None semantics for a payload that the schema would have rejected.

Per-angle assessment

• Correctness — PR claims "zero behavior change" and the diff bears it out. No dispatch site, snapshot loader, hub, or bridge reads the new fields. Independently verified by running cargo test -p aisix-core (150 passed, 0 failed) and cargo check --workspace (clean). All five ProviderKey builder helpers across the workspace (aisix-gateway/src/bridge.rs:361, aisix-provider-anthropic/src/bridge.rs:553, aisix-provider-openai/src/bridge.rs:787, aisix-core/src/models/snapshot.rs:74, aisix-proxy/src/dispatch.rs:283) use serde_json::from_str, never struct-literal — so the new fields ride on #[serde(default)] and existing tests don't need to be touched. The single struct-literal site (in the new round-trip test) is the only place that sets all fields explicitly, and it's correct. Independently verified the legacy-payload-compat contract by running the test under cargo test. PASS.

• Reliability — Pure type extension; no async, no I/O, no time-sensitive code, no new error path. The schema-then-struct two-stage parse on the etcd loader path is unchanged. PASS.

• Security — No new credential flow; secret semantics unchanged. The new fields are operator-supplied attribution labels with no privileged interpretation in this PR (no dispatch site reads them). AISIX-Cloud#302 §5 places the validation responsibility on cp-api's createProviderKey — the DP is correctly second-line-of-defence-only, which matches the spec. PASS for this PR (see LOW 2 for the future hardening alignment with §5).

• Sensitive-info leakage — New fields are non-secret by construction (vendor identity, adapter shape, telemetry tags). The pre-existing Debug impl on ProviderKey already leaks secret (out of scope here), and the new fields do not widen that exposure. The new Debug impl on TelemetryTags is fine — all fields are labels intended for log/metric emission anyway. PASS.

• Breaking changes — aisix-core is a workspace-internal crate (no publish = true in Cargo.toml). New struct fields are all #[serde(default)] so the wire format adds optional fields only. additionalProperties: false on the schema continues to apply, with the three new fields explicitly enumerated. Existing on-disk KV payloads with no provider / adapter / telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes and legacy_payload_without_phase_a_fields_deserialises_with_defaults). The new pub use TelemetryTags does not collide with any existing export (verified by rg "TelemetryTags" crates/ — only the new declaration and re-exports appear). PASS.

• E2E test coverage — Adequate for the "skeleton, no consumer" contract this PR claims. The 14 new tests cover both directions:

  • Schema-level: minimal, legacy-no-phase-a, full-phase-a, byo-shape, unknown-adapter, unknown-telemetry-field, unknown-top-level, unknown-kind.
  • Struct-level: legacy-no-phase-a-defaults, full-phase-a, byo-shape, unknown-telemetry-field, unknown-adapter, round-trip-with-defaults.
    Both layers are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No cross-crate or HTTP-level e2e needed in this PR because there is no behavior under test — the dispatch wiring lands in the follow-up Phase A sub-PR per the PR description. PASS.

Independent edge-case probe (not added to the PR — purely for audit verification, see findings above)

Verified the following edge cases by spawning a temporary integration test in crates/aisix-core/tests/audit_check.rs (since deleted, working tree is clean):

Payload fragment	Serde result	Schema result	Outcome
"provider": null	Err — invalid type: null, expected a string	Err — null is not of type string	Both gate; OK
"telemetry_tags": null	Err — invalid type: null, expected struct	Err — null is not of type object	Both gate; OK
"adapter": null	Ok — None	Err — null is not one of [openai,…]	Divergence; see LOW 3
omitted (legacy)	Ok — defaults	Ok	Compat pinned
"telemetry_tags": {}	Ok — default	Ok	Compat pinned
provider = 100k chars	Ok	Ok	See LOW 2
Round-trip of TelemetryTags::default()	Emits {"featured": false} (kind/branded_provider/pk_label/byo_label all skip_serializing_if)	Schema accepts	OK

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Summary

Second sub-PR of api7/AISIX-Cloud#302 Phase A (DP-side Provider→Adapter refactor). Builds on #297 (which landed the Adapter enum).

This PR is intentionally zero-behavior-change — pure type extension on ProviderKey. Nothing in the gateway reads the new fields yet; Provider continues to drive 100% of dispatch.

What this PR does

1. Adds four new fields to aisix_core::ProviderKey:

   • provider: String — vendor identity (e.g. "deepseek", "openai"). Free-form String in this PR; closed-set validation is deferred to a later Phase A sub-PR that wires dispatch.
   • adapter: Option<Adapter> — wire-shape, pinned to the closed Adapter enum from feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297. None until a follow-up populates it.
   • telemetry_tags: TelemetryTags — attribution tags emitted alongside requests routed through this key.

2. Adds a new TelemetryTags struct with five optional fields:

   • kind: Option<String> — closed-set "catalog" | "byo" (enforced by the JSON Schema; the Rust type is Option<String> to stay forward-compatible if cp-api ships a new variant ahead of a DP rollout)
   • featured: bool — defaults to false
   • branded_provider: Option<String>
   • pk_label: Option<String>
   • byo_label: Option<String>

   TelemetryTags derives Default and is #[serde(deny_unknown_fields)] so an unknown tag from cp-api fails loudly on the DP rather than silently dropping.

3. Updates the provider_key JSON Schema with matching optional properties. additionalProperties: false is preserved both at the top level and inside telemetry_tags. adapter is constrained to the five Adapter enum values; kind is constrained to "catalog" | "byo".

4. Re-exports TelemetryTags from aisix_core::models and the crate root, mirroring Adapter.

Backward compatibility

All four new struct fields use #[serde(default)]. A pre-#302 payload like

{"display_name":"openai-prod","secret":"sk-x","api_base":"https://api.openai.com/v1"}

still deserializes — provider lands as "", adapter as None, telemetry_tags as TelemetryTags::default(). A dedicated test pins this contract (legacy_payload_without_phase_a_fields_deserialises_with_defaults).

The JSON Schema mirrors this: only display_name and secret remain required.

What this PR does NOT do

• Does not remove or modify Provider
• Does not change display_name (pre-existed on ProviderKey)
• Does not touch secret / api_base
• Does not change Hub, Bridges, dispatch, or any wire transform
• Does not change mustMarshalProviderKeyKV on the CP side (that's Phase D)
• Does not delete the wrapper crate
• Does not touch rerank.rs (A5) or OpenAiBridge (A4)

Design notes

• Option<Adapter> over Adapter::Default: keeping adapterNone-able makes "Phase A hasn't backfilled this yet" representable. Adding a default variant would lie about adapter shape for un-tagged keys.
• TelemetryTags as a struct (not HashMap<String, Value>): the catalog/byo attribution shape is a closed set known to both cp-api and the DP; a typed struct with deny_unknown_fields catches drift between the two sides at parse time. A free-form map would silently swallow typos.

Test plan

• cargo test -p aisix-core — 150 passed, 0 failed (+14 over feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297)
• cargo clippy --workspace --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean
• Legacy payload without Phase A fields deserializes (compat contract)
• Full Phase A payload (catalog shape) deserializes and round-trips
• BYO telemetry shape (branded_provider:null + byo_label) deserializes
• Unknown adapter string rejected (closed-set guard)
• Unknown telemetry_tags field rejected (deny_unknown_fields guard)
• Unknown telemetry_tags.kind value rejected at schema layer
• Schema accepts both legacy and Phase A payloads; rejects unknown top-level fields

References

• Tracking issue: api7/AISIX-Cloud#302
• Predecessor: feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297 (Adapter enum)

Summary by CodeRabbit

• New Features

  • Extended provider configuration to support telemetry tagging, adapter specification, and additional provider metadata fields while maintaining backward compatibility with existing payloads.

• Tests

  • Added comprehensive test coverage for new schema fields, legacy payload compatibility, and validation constraints.

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: ddb92b36-e041-4434-bcf7-10268b7dc3ca

📥 Commits

Reviewing files that changed from the base of the PR and between 83c48c9 and 28024f6.

📒 Files selected for processing (4)

• crates/aisix-core/src/lib.rs
• crates/aisix-core/src/models/mod.rs
• crates/aisix-core/src/models/provider_key.rs
• crates/aisix-core/src/models/schema.rs

---

📝 Walkthrough

Walkthrough

This PR extends the ProviderKey model with Phase A fields (provider, adapter, telemetry_tags) to support provider identity and telemetry attribution. A new TelemetryTags struct is introduced with serde defaults and strict field validation. The changes maintain backward compatibility so legacy payloads deserialize with zero values, and comprehensive tests verify both JSON schema validation and round-trip serialization.

Changes

Provider Key Phase A Extension

Layer / File(s)	Summary
Type definitions and public exports crates/aisix-core/src/models/provider_key.rs, crates/aisix-core/src/models/mod.rs, crates/aisix-core/src/lib.rs	New TelemetryTags struct introduced with serde defaults and deny_unknown_fields attribute. ProviderKey extended with three new fields: provider (string), adapter (optional), and telemetry_tags. Adapter imported to support the new field type. Public re-exports in mod.rs and lib.rs updated to expose TelemetryTags.
JSON schema validation and documentation crates/aisix-core/src/models/schema.rs	provider_key_schema() extended with Phase A fields as optional (preserving backward compatibility). Documentation added explaining that provider, adapter, and telemetry_tags are not yet wired to dispatch behavior. Schema tests added covering minimal payloads, legacy payloads without Phase A fields, and payloads with Phase A fields including both catalog and BYO telemetry shapes. Rejection tests verify unknown adapter values, unknown telemetry fields, and invalid kind discriminator values are caught.
Deserialization and round-trip compatibility tests crates/aisix-core/src/models/provider_key.rs	Test suite verifies legacy payloads without Phase A fields deserialize to defaults. Payloads with all Phase A fields (including telemetry tag values and BYO dual-label handling) deserialize correctly. adapter field rejects unknown adapter strings. telemetry_tags rejects unknown fields. A ProviderKey containing default Phase A fields successfully round-trips via JSON without losing semantic equality.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

---

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[moonming]

Independent audit (CLAUDE.md §8)

Cold re-read of the diff via gh pr diff 298 — no shared context with the implementation session. Reviewed against the six required angles.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. No maxLength bound on free-form telemetry fields (crates/aisix-core/src/models/schema.rs:288-307). branded_provider, pk_label, byo_label, and the top-level provider are all unbounded strings in the JSON Schema. A misbehaving (or compromised) cp-api could publish a 10MB-per-tag value and the DP would happily persist it into the snapshot. This is consistent with the pre-existing lack of maxLength on display_name/secret/api_base, so it is not a regression introduced here — but it is worth tightening in a follow-up Phase A PR (suggest 120 chars to match cache_policy.name).
   Suggested follow-up edit:

   "provider": { "type": "string", "maxLength": 120 },
   "branded_provider": { "type": ["string", "null"], "maxLength": 120 },
   "pk_label": { "type": ["string", "null"], "maxLength": 120 },
   "byo_label": { "type": ["string", "null"], "maxLength": 120 }

2. Schema does not enforce the catalog/byo mutual-exclusion shape (crates/aisix-core/src/models/schema.rs:288-307). The PR description notes that the canonical BYO shape has branded_provider:null + byo_label="…" and the catalog shape has branded_provider:"vendor" + byo_label:null, but the schema allows any combination (both set, both null, etc.). This is a deliberate Phase A skeleton choice — no dispatch reads the field yet, and adding a conditional oneOf now would foreclose what Phase A migration learns about real cp-api payloads. Worth pinning the constraint once Phase A wires the telemetry consumer.

3. TelemetryTags::Default produces all-None including kind: None, but the schema constrains kind to "catalog"|"byo" (no null allowed). A round-trip of TelemetryTags::default() via serde_json::to_value produces {} (because of skip_serializing_if = "Option::is_none"), which is schema-valid. A round-trip via serde_json::to_value with serialize_none = true (not used today) would emit kind: null and fail schema. This is a non-issue with current code, but worth a comment near the struct definition so a future PR doesn't add a null-emitting serializer.

Per-angle assessment

• Correctness: The PR claims "zero behavior change" and the diff bears it out — no dispatch site, snapshot loader, hub, or bridge reads the new fields. Verified by running cargo test --workspace (all 18 crate test suites pass, no downstream breakage). All five test helpers across the workspace that build ProviderKey go through serde_json::from_str, never struct-literal syntax, so the addition of new required-but-defaulted fields does not break any existing test. PASS.
• Reliability: Pure type extension; no async, no I/O, no time-sensitive code. PASS.
• Security: No new credential flow; secret semantics unchanged. The new fields are operator-supplied labels with no privileged interpretation in this PR. PASS for this PR (see LOW 1 for a hardening follow-up).
• Sensitive-info leakage: New fields are non-secret. The Debug impl on ProviderKey already leaks secret (pre-existing), and the new fields do not widen that exposure. PASS.
• Breaking changes: aisix-core is a workspace-internal crate (no publish = true). All in-workspace consumers reach ProviderKey via serde_json, never struct-literal construction with full field list — verified by grep. The wire format adds optional fields only; additionalProperties: false plus the existing on-disk payloads with no provider/adapter/telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes). PASS.
• E2E test coverage: Adequate for the "skeleton, no consumer" contract this PR claims. Both struct-level and schema-level legacy-payload-compat tests are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No need for cross-crate or HTTP-level e2e in this PR because there is no behavior under test — that lands in the follow-up Phase A sub-PR that wires dispatch.

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Independent third-party audit per CLAUDE.md §8 (separate from author self-review)

Cold re-read of this PR by a fresh agent with no shared context with the implementation session or with the author's own audit comment above. Reviewed against the six required angles and against the canonical wire shapes in api7/AISIX-Cloud#302 §5.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. PR title slightly misrepresents the diff. The title says "add provider/adapter/display_name fields" but display_name is pre-existing on ProviderKey (verified by git show main:crates/aisix-core/src/models/provider_key.rs). The diff only adds provider, adapter, and telemetry_tags. The body is accurate ("Adds four new fields…" actually says 4 but lists 3 — display_name is correctly absent from the body's enumeration). The body explicitly says "Does not change display_name (pre-existed on ProviderKey)". Non-blocking — squash-merge commit title is what would land on main, suggest tightening to feat(core): add provider/adapter/telemetry_tags fields to ProviderKey (Phase A skeleton).

2. No maxLength on any of the new string fields (crates/aisix-core/src/models/schema.rs:288-307). Independently verified: a 100,000-char provider / pk_label / byo_label / branded_provider passes both schema validation and serde deserialisation. Consistent with the pre-existing lack of maxLength on display_name / secret / api_base, so not a regression — but AISIX-Cloud#302 §5 specifies display_name length 1-64 with sluggified-form ≥ 3. The DP currently enforces neither, leaving cp-api as the sole gatekeeper for label length. Worth tightening in a follow-up alongside the §5 validation work cp-api is committing to.

3. adapter:null is silently accepted by serde but rejected by schema. Independently verified:

   • Struct path: serde_json::from_str of {"adapter": null, …} → adapter = None, Ok(()).
   • Schema path: validate_provider_key of the same → rejected ("null is not one of [openai,…]").
   • Because the etcd loader runs schema validation before struct deserialisation, a wire payload with adapter:null is rejected at the schema gate, so this divergence has no observable impact in this PR. But it is worth a one-line code comment near the adapter field saying "schema is the gatekeeper for null-vs-absent; the serde Option only handles the absent case". Otherwise a future PR that builds a ProviderKey from a serde_json::Valuebypassing the schema (e.g. a unit test reading a fixture, or a CLI tool) could see adapter:None semantics for a payload that the schema would have rejected.

Per-angle assessment

• Correctness — PR claims "zero behavior change" and the diff bears it out. No dispatch site, snapshot loader, hub, or bridge reads the new fields. Independently verified by running cargo test -p aisix-core (150 passed, 0 failed) and cargo check --workspace (clean). All five ProviderKey builder helpers across the workspace (aisix-gateway/src/bridge.rs:361, aisix-provider-anthropic/src/bridge.rs:553, aisix-provider-openai/src/bridge.rs:787, aisix-core/src/models/snapshot.rs:74, aisix-proxy/src/dispatch.rs:283) use serde_json::from_str, never struct-literal — so the new fields ride on #[serde(default)] and existing tests don't need to be touched. The single struct-literal site (in the new round-trip test) is the only place that sets all fields explicitly, and it's correct. Independently verified the legacy-payload-compat contract by running the test under cargo test. PASS.

• Reliability — Pure type extension; no async, no I/O, no time-sensitive code, no new error path. The schema-then-struct two-stage parse on the etcd loader path is unchanged. PASS.

• Security — No new credential flow; secret semantics unchanged. The new fields are operator-supplied attribution labels with no privileged interpretation in this PR (no dispatch site reads them). AISIX-Cloud#302 §5 places the validation responsibility on cp-api's createProviderKey — the DP is correctly second-line-of-defence-only, which matches the spec. PASS for this PR (see LOW 2 for the future hardening alignment with §5).

• Sensitive-info leakage — New fields are non-secret by construction (vendor identity, adapter shape, telemetry tags). The pre-existing Debug impl on ProviderKey already leaks secret (out of scope here), and the new fields do not widen that exposure. The new Debug impl on TelemetryTags is fine — all fields are labels intended for log/metric emission anyway. PASS.

• Breaking changes — aisix-core is a workspace-internal crate (no publish = true in Cargo.toml). New struct fields are all #[serde(default)] so the wire format adds optional fields only. additionalProperties: false on the schema continues to apply, with the three new fields explicitly enumerated. Existing on-disk KV payloads with no provider / adapter / telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes and legacy_payload_without_phase_a_fields_deserialises_with_defaults). The new pub use TelemetryTags does not collide with any existing export (verified by rg "TelemetryTags" crates/ — only the new declaration and re-exports appear). PASS.

• E2E test coverage — Adequate for the "skeleton, no consumer" contract this PR claims. The 14 new tests cover both directions:

  • Schema-level: minimal, legacy-no-phase-a, full-phase-a, byo-shape, unknown-adapter, unknown-telemetry-field, unknown-top-level, unknown-kind.
  • Struct-level: legacy-no-phase-a-defaults, full-phase-a, byo-shape, unknown-telemetry-field, unknown-adapter, round-trip-with-defaults.
    Both layers are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No cross-crate or HTTP-level e2e needed in this PR because there is no behavior under test — the dispatch wiring lands in the follow-up Phase A sub-PR per the PR description. PASS.

Independent edge-case probe (not added to the PR — purely for audit verification, see findings above)

Verified the following edge cases by spawning a temporary integration test in crates/aisix-core/tests/audit_check.rs (since deleted, working tree is clean):

Payload fragment	Serde result	Schema result	Outcome
"provider": null	Err — invalid type: null, expected a string	Err — null is not of type string	Both gate; OK
"telemetry_tags": null	Err — invalid type: null, expected struct	Err — null is not of type object	Both gate; OK
"adapter": null	Ok — None	Err — null is not one of [openai,…]	Divergence; see LOW 3
omitted (legacy)	Ok — defaults	Ok	Compat pinned
"telemetry_tags": {}	Ok — default	Ok	Compat pinned
provider = 100k chars	Ok	Ok	See LOW 2
Round-trip of TelemetryTags::default()	Emits {"featured": false} (kind/branded_provider/pk_label/byo_label all skip_serializing_if)	Schema accepts	OK

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Summary

Second sub-PR of api7/AISIX-Cloud#302 Phase A (DP-side Provider→Adapter refactor). Builds on #297 (which landed the Adapter enum).

This PR is intentionally zero-behavior-change — pure type extension on ProviderKey. Nothing in the gateway reads the new fields yet; Provider continues to drive 100% of dispatch.

What this PR does

1. Adds four new fields to aisix_core::ProviderKey:

   • provider: String — vendor identity (e.g. "deepseek", "openai"). Free-form String in this PR; closed-set validation is deferred to a later Phase A sub-PR that wires dispatch.
   • adapter: Option<Adapter> — wire-shape, pinned to the closed Adapter enum from feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297. None until a follow-up populates it.
   • telemetry_tags: TelemetryTags — attribution tags emitted alongside requests routed through this key.

2. Adds a new TelemetryTags struct with five optional fields:

   • kind: Option<String> — closed-set "catalog" | "byo" (enforced by the JSON Schema; the Rust type is Option<String> to stay forward-compatible if cp-api ships a new variant ahead of a DP rollout)
   • featured: bool — defaults to false
   • branded_provider: Option<String>
   • pk_label: Option<String>
   • byo_label: Option<String>

   TelemetryTags derives Default and is #[serde(deny_unknown_fields)] so an unknown tag from cp-api fails loudly on the DP rather than silently dropping.

3. Updates the provider_key JSON Schema with matching optional properties. additionalProperties: false is preserved both at the top level and inside telemetry_tags. adapter is constrained to the five Adapter enum values; kind is constrained to "catalog" | "byo".

4. Re-exports TelemetryTags from aisix_core::models and the crate root, mirroring Adapter.

Backward compatibility

All four new struct fields use #[serde(default)]. A pre-#302 payload like

{"display_name":"openai-prod","secret":"sk-x","api_base":"https://api.openai.com/v1"}

still deserializes — provider lands as "", adapter as None, telemetry_tags as TelemetryTags::default(). A dedicated test pins this contract (legacy_payload_without_phase_a_fields_deserialises_with_defaults).

The JSON Schema mirrors this: only display_name and secret remain required.

What this PR does NOT do

• Does not remove or modify Provider
• Does not change display_name (pre-existed on ProviderKey)
• Does not touch secret / api_base
• Does not change Hub, Bridges, dispatch, or any wire transform
• Does not change mustMarshalProviderKeyKV on the CP side (that's Phase D)
• Does not delete the wrapper crate
• Does not touch rerank.rs (A5) or OpenAiBridge (A4)

Design notes

• Option<Adapter> over Adapter::Default: keeping adapterNone-able makes "Phase A hasn't backfilled this yet" representable. Adding a default variant would lie about adapter shape for un-tagged keys.
• TelemetryTags as a struct (not HashMap<String, Value>): the catalog/byo attribution shape is a closed set known to both cp-api and the DP; a typed struct with deny_unknown_fields catches drift between the two sides at parse time. A free-form map would silently swallow typos.

Test plan

• cargo test -p aisix-core — 150 passed, 0 failed (+14 over feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297)
• cargo clippy --workspace --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean
• Legacy payload without Phase A fields deserializes (compat contract)
• Full Phase A payload (catalog shape) deserializes and round-trips
• BYO telemetry shape (branded_provider:null + byo_label) deserializes
• Unknown adapter string rejected (closed-set guard)
• Unknown telemetry_tags field rejected (deny_unknown_fields guard)
• Unknown telemetry_tags.kind value rejected at schema layer
• Schema accepts both legacy and Phase A payloads; rejects unknown top-level fields

References

• Tracking issue: api7/AISIX-Cloud#302
• Predecessor: feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297 (Adapter enum)

Summary by CodeRabbit

• New Features

  • Extended provider configuration to support telemetry tagging, adapter specification, and additional provider metadata fields while maintaining backward compatibility with existing payloads.

• Tests

  • Added comprehensive test coverage for new schema fields, legacy payload compatibility, and validation constraints.

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: ddb92b36-e041-4434-bcf7-10268b7dc3ca

📥 Commits

Reviewing files that changed from the base of the PR and between 83c48c9 and 28024f6.

📒 Files selected for processing (4)

• crates/aisix-core/src/lib.rs
• crates/aisix-core/src/models/mod.rs
• crates/aisix-core/src/models/provider_key.rs
• crates/aisix-core/src/models/schema.rs

---

📝 Walkthrough

Walkthrough

This PR extends the ProviderKey model with Phase A fields (provider, adapter, telemetry_tags) to support provider identity and telemetry attribution. A new TelemetryTags struct is introduced with serde defaults and strict field validation. The changes maintain backward compatibility so legacy payloads deserialize with zero values, and comprehensive tests verify both JSON schema validation and round-trip serialization.

Changes

Provider Key Phase A Extension

Layer / File(s)	Summary
Type definitions and public exports crates/aisix-core/src/models/provider_key.rs, crates/aisix-core/src/models/mod.rs, crates/aisix-core/src/lib.rs	New TelemetryTags struct introduced with serde defaults and deny_unknown_fields attribute. ProviderKey extended with three new fields: provider (string), adapter (optional), and telemetry_tags. Adapter imported to support the new field type. Public re-exports in mod.rs and lib.rs updated to expose TelemetryTags.
JSON schema validation and documentation crates/aisix-core/src/models/schema.rs	provider_key_schema() extended with Phase A fields as optional (preserving backward compatibility). Documentation added explaining that provider, adapter, and telemetry_tags are not yet wired to dispatch behavior. Schema tests added covering minimal payloads, legacy payloads without Phase A fields, and payloads with Phase A fields including both catalog and BYO telemetry shapes. Rejection tests verify unknown adapter values, unknown telemetry fields, and invalid kind discriminator values are caught.
Deserialization and round-trip compatibility tests crates/aisix-core/src/models/provider_key.rs	Test suite verifies legacy payloads without Phase A fields deserialize to defaults. Payloads with all Phase A fields (including telemetry tag values and BYO dual-label handling) deserialize correctly. adapter field rejects unknown adapter strings. telemetry_tags rejects unknown fields. A ProviderKey containing default Phase A fields successfully round-trips via JSON without losing semantic equality.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

---

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[moonming]

Independent audit (CLAUDE.md §8)

Cold re-read of the diff via gh pr diff 298 — no shared context with the implementation session. Reviewed against the six required angles.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. No maxLength bound on free-form telemetry fields (crates/aisix-core/src/models/schema.rs:288-307). branded_provider, pk_label, byo_label, and the top-level provider are all unbounded strings in the JSON Schema. A misbehaving (or compromised) cp-api could publish a 10MB-per-tag value and the DP would happily persist it into the snapshot. This is consistent with the pre-existing lack of maxLength on display_name/secret/api_base, so it is not a regression introduced here — but it is worth tightening in a follow-up Phase A PR (suggest 120 chars to match cache_policy.name).
   Suggested follow-up edit:

   "provider": { "type": "string", "maxLength": 120 },
   "branded_provider": { "type": ["string", "null"], "maxLength": 120 },
   "pk_label": { "type": ["string", "null"], "maxLength": 120 },
   "byo_label": { "type": ["string", "null"], "maxLength": 120 }

2. Schema does not enforce the catalog/byo mutual-exclusion shape (crates/aisix-core/src/models/schema.rs:288-307). The PR description notes that the canonical BYO shape has branded_provider:null + byo_label="…" and the catalog shape has branded_provider:"vendor" + byo_label:null, but the schema allows any combination (both set, both null, etc.). This is a deliberate Phase A skeleton choice — no dispatch reads the field yet, and adding a conditional oneOf now would foreclose what Phase A migration learns about real cp-api payloads. Worth pinning the constraint once Phase A wires the telemetry consumer.

3. TelemetryTags::Default produces all-None including kind: None, but the schema constrains kind to "catalog"|"byo" (no null allowed). A round-trip of TelemetryTags::default() via serde_json::to_value produces {} (because of skip_serializing_if = "Option::is_none"), which is schema-valid. A round-trip via serde_json::to_value with serialize_none = true (not used today) would emit kind: null and fail schema. This is a non-issue with current code, but worth a comment near the struct definition so a future PR doesn't add a null-emitting serializer.

Per-angle assessment

• Correctness: The PR claims "zero behavior change" and the diff bears it out — no dispatch site, snapshot loader, hub, or bridge reads the new fields. Verified by running cargo test --workspace (all 18 crate test suites pass, no downstream breakage). All five test helpers across the workspace that build ProviderKey go through serde_json::from_str, never struct-literal syntax, so the addition of new required-but-defaulted fields does not break any existing test. PASS.
• Reliability: Pure type extension; no async, no I/O, no time-sensitive code. PASS.
• Security: No new credential flow; secret semantics unchanged. The new fields are operator-supplied labels with no privileged interpretation in this PR. PASS for this PR (see LOW 1 for a hardening follow-up).
• Sensitive-info leakage: New fields are non-secret. The Debug impl on ProviderKey already leaks secret (pre-existing), and the new fields do not widen that exposure. PASS.
• Breaking changes: aisix-core is a workspace-internal crate (no publish = true). All in-workspace consumers reach ProviderKey via serde_json, never struct-literal construction with full field list — verified by grep. The wire format adds optional fields only; additionalProperties: false plus the existing on-disk payloads with no provider/adapter/telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes). PASS.
• E2E test coverage: Adequate for the "skeleton, no consumer" contract this PR claims. Both struct-level and schema-level legacy-payload-compat tests are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No need for cross-crate or HTTP-level e2e in this PR because there is no behavior under test — that lands in the follow-up Phase A sub-PR that wires dispatch.

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Independent third-party audit per CLAUDE.md §8 (separate from author self-review)

Cold re-read of this PR by a fresh agent with no shared context with the implementation session or with the author's own audit comment above. Reviewed against the six required angles and against the canonical wire shapes in api7/AISIX-Cloud#302 §5.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. PR title slightly misrepresents the diff. The title says "add provider/adapter/display_name fields" but display_name is pre-existing on ProviderKey (verified by git show main:crates/aisix-core/src/models/provider_key.rs). The diff only adds provider, adapter, and telemetry_tags. The body is accurate ("Adds four new fields…" actually says 4 but lists 3 — display_name is correctly absent from the body's enumeration). The body explicitly says "Does not change display_name (pre-existed on ProviderKey)". Non-blocking — squash-merge commit title is what would land on main, suggest tightening to feat(core): add provider/adapter/telemetry_tags fields to ProviderKey (Phase A skeleton).

2. No maxLength on any of the new string fields (crates/aisix-core/src/models/schema.rs:288-307). Independently verified: a 100,000-char provider / pk_label / byo_label / branded_provider passes both schema validation and serde deserialisation. Consistent with the pre-existing lack of maxLength on display_name / secret / api_base, so not a regression — but AISIX-Cloud#302 §5 specifies display_name length 1-64 with sluggified-form ≥ 3. The DP currently enforces neither, leaving cp-api as the sole gatekeeper for label length. Worth tightening in a follow-up alongside the §5 validation work cp-api is committing to.

3. adapter:null is silently accepted by serde but rejected by schema. Independently verified:

   • Struct path: serde_json::from_str of {"adapter": null, …} → adapter = None, Ok(()).
   • Schema path: validate_provider_key of the same → rejected ("null is not one of [openai,…]").
   • Because the etcd loader runs schema validation before struct deserialisation, a wire payload with adapter:null is rejected at the schema gate, so this divergence has no observable impact in this PR. But it is worth a one-line code comment near the adapter field saying "schema is the gatekeeper for null-vs-absent; the serde Option only handles the absent case". Otherwise a future PR that builds a ProviderKey from a serde_json::Valuebypassing the schema (e.g. a unit test reading a fixture, or a CLI tool) could see adapter:None semantics for a payload that the schema would have rejected.

Per-angle assessment

• Correctness — PR claims "zero behavior change" and the diff bears it out. No dispatch site, snapshot loader, hub, or bridge reads the new fields. Independently verified by running cargo test -p aisix-core (150 passed, 0 failed) and cargo check --workspace (clean). All five ProviderKey builder helpers across the workspace (aisix-gateway/src/bridge.rs:361, aisix-provider-anthropic/src/bridge.rs:553, aisix-provider-openai/src/bridge.rs:787, aisix-core/src/models/snapshot.rs:74, aisix-proxy/src/dispatch.rs:283) use serde_json::from_str, never struct-literal — so the new fields ride on #[serde(default)] and existing tests don't need to be touched. The single struct-literal site (in the new round-trip test) is the only place that sets all fields explicitly, and it's correct. Independently verified the legacy-payload-compat contract by running the test under cargo test. PASS.

• Reliability — Pure type extension; no async, no I/O, no time-sensitive code, no new error path. The schema-then-struct two-stage parse on the etcd loader path is unchanged. PASS.

• Security — No new credential flow; secret semantics unchanged. The new fields are operator-supplied attribution labels with no privileged interpretation in this PR (no dispatch site reads them). AISIX-Cloud#302 §5 places the validation responsibility on cp-api's createProviderKey — the DP is correctly second-line-of-defence-only, which matches the spec. PASS for this PR (see LOW 2 for the future hardening alignment with §5).

• Sensitive-info leakage — New fields are non-secret by construction (vendor identity, adapter shape, telemetry tags). The pre-existing Debug impl on ProviderKey already leaks secret (out of scope here), and the new fields do not widen that exposure. The new Debug impl on TelemetryTags is fine — all fields are labels intended for log/metric emission anyway. PASS.

• Breaking changes — aisix-core is a workspace-internal crate (no publish = true in Cargo.toml). New struct fields are all #[serde(default)] so the wire format adds optional fields only. additionalProperties: false on the schema continues to apply, with the three new fields explicitly enumerated. Existing on-disk KV payloads with no provider / adapter / telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes and legacy_payload_without_phase_a_fields_deserialises_with_defaults). The new pub use TelemetryTags does not collide with any existing export (verified by rg "TelemetryTags" crates/ — only the new declaration and re-exports appear). PASS.

• E2E test coverage — Adequate for the "skeleton, no consumer" contract this PR claims. The 14 new tests cover both directions:

  • Schema-level: minimal, legacy-no-phase-a, full-phase-a, byo-shape, unknown-adapter, unknown-telemetry-field, unknown-top-level, unknown-kind.
  • Struct-level: legacy-no-phase-a-defaults, full-phase-a, byo-shape, unknown-telemetry-field, unknown-adapter, round-trip-with-defaults.
    Both layers are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No cross-crate or HTTP-level e2e needed in this PR because there is no behavior under test — the dispatch wiring lands in the follow-up Phase A sub-PR per the PR description. PASS.

Independent edge-case probe (not added to the PR — purely for audit verification, see findings above)

Verified the following edge cases by spawning a temporary integration test in crates/aisix-core/tests/audit_check.rs (since deleted, working tree is clean):

Payload fragment	Serde result	Schema result	Outcome
"provider": null	Err — invalid type: null, expected a string	Err — null is not of type string	Both gate; OK
"telemetry_tags": null	Err — invalid type: null, expected struct	Err — null is not of type object	Both gate; OK
"adapter": null	Ok — None	Err — null is not one of [openai,…]	Divergence; see LOW 3
omitted (legacy)	Ok — defaults	Ok	Compat pinned
"telemetry_tags": {}	Ok — default	Ok	Compat pinned
provider = 100k chars	Ok	Ok	See LOW 2
Round-trip of TelemetryTags::default()	Emits {"featured": false} (kind/branded_provider/pk_label/byo_label all skip_serializing_if)	Schema accepts	OK

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Summary

Second sub-PR of api7/AISIX-Cloud#302 Phase A (DP-side Provider→Adapter refactor). Builds on #297 (which landed the Adapter enum).

This PR is intentionally zero-behavior-change — pure type extension on ProviderKey. Nothing in the gateway reads the new fields yet; Provider continues to drive 100% of dispatch.

What this PR does

1. Adds four new fields to aisix_core::ProviderKey:

   • provider: String — vendor identity (e.g. "deepseek", "openai"). Free-form String in this PR; closed-set validation is deferred to a later Phase A sub-PR that wires dispatch.
   • adapter: Option<Adapter> — wire-shape, pinned to the closed Adapter enum from feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297. None until a follow-up populates it.
   • telemetry_tags: TelemetryTags — attribution tags emitted alongside requests routed through this key.

2. Adds a new TelemetryTags struct with five optional fields:

   • kind: Option<String> — closed-set "catalog" | "byo" (enforced by the JSON Schema; the Rust type is Option<String> to stay forward-compatible if cp-api ships a new variant ahead of a DP rollout)
   • featured: bool — defaults to false
   • branded_provider: Option<String>
   • pk_label: Option<String>
   • byo_label: Option<String>

   TelemetryTags derives Default and is #[serde(deny_unknown_fields)] so an unknown tag from cp-api fails loudly on the DP rather than silently dropping.

3. Updates the provider_key JSON Schema with matching optional properties. additionalProperties: false is preserved both at the top level and inside telemetry_tags. adapter is constrained to the five Adapter enum values; kind is constrained to "catalog" | "byo".

4. Re-exports TelemetryTags from aisix_core::models and the crate root, mirroring Adapter.

Backward compatibility

All four new struct fields use #[serde(default)]. A pre-#302 payload like

{"display_name":"openai-prod","secret":"sk-x","api_base":"https://api.openai.com/v1"}

still deserializes — provider lands as "", adapter as None, telemetry_tags as TelemetryTags::default(). A dedicated test pins this contract (legacy_payload_without_phase_a_fields_deserialises_with_defaults).

The JSON Schema mirrors this: only display_name and secret remain required.

What this PR does NOT do

• Does not remove or modify Provider
• Does not change display_name (pre-existed on ProviderKey)
• Does not touch secret / api_base
• Does not change Hub, Bridges, dispatch, or any wire transform
• Does not change mustMarshalProviderKeyKV on the CP side (that's Phase D)
• Does not delete the wrapper crate
• Does not touch rerank.rs (A5) or OpenAiBridge (A4)

Design notes

• Option<Adapter> over Adapter::Default: keeping adapterNone-able makes "Phase A hasn't backfilled this yet" representable. Adding a default variant would lie about adapter shape for un-tagged keys.
• TelemetryTags as a struct (not HashMap<String, Value>): the catalog/byo attribution shape is a closed set known to both cp-api and the DP; a typed struct with deny_unknown_fields catches drift between the two sides at parse time. A free-form map would silently swallow typos.

Test plan

• cargo test -p aisix-core — 150 passed, 0 failed (+14 over feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297)
• cargo clippy --workspace --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean
• Legacy payload without Phase A fields deserializes (compat contract)
• Full Phase A payload (catalog shape) deserializes and round-trips
• BYO telemetry shape (branded_provider:null + byo_label) deserializes
• Unknown adapter string rejected (closed-set guard)
• Unknown telemetry_tags field rejected (deny_unknown_fields guard)
• Unknown telemetry_tags.kind value rejected at schema layer
• Schema accepts both legacy and Phase A payloads; rejects unknown top-level fields

References

• Tracking issue: api7/AISIX-Cloud#302
• Predecessor: feat(core): add Adapter enum (skeleton for issue #302 Phase A) #297 (Adapter enum)

Summary by CodeRabbit

• New Features

  • Extended provider configuration to support telemetry tagging, adapter specification, and additional provider metadata fields while maintaining backward compatibility with existing payloads.

• Tests

  • Added comprehensive test coverage for new schema fields, legacy payload compatibility, and validation constraints.

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: ddb92b36-e041-4434-bcf7-10268b7dc3ca

📥 Commits

Reviewing files that changed from the base of the PR and between 83c48c9 and 28024f6.

📒 Files selected for processing (4)

• crates/aisix-core/src/lib.rs
• crates/aisix-core/src/models/mod.rs
• crates/aisix-core/src/models/provider_key.rs
• crates/aisix-core/src/models/schema.rs

---

📝 Walkthrough

Walkthrough

This PR extends the ProviderKey model with Phase A fields (provider, adapter, telemetry_tags) to support provider identity and telemetry attribution. A new TelemetryTags struct is introduced with serde defaults and strict field validation. The changes maintain backward compatibility so legacy payloads deserialize with zero values, and comprehensive tests verify both JSON schema validation and round-trip serialization.

Changes

Provider Key Phase A Extension

Layer / File(s)	Summary
Type definitions and public exports crates/aisix-core/src/models/provider_key.rs, crates/aisix-core/src/models/mod.rs, crates/aisix-core/src/lib.rs	New TelemetryTags struct introduced with serde defaults and deny_unknown_fields attribute. ProviderKey extended with three new fields: provider (string), adapter (optional), and telemetry_tags. Adapter imported to support the new field type. Public re-exports in mod.rs and lib.rs updated to expose TelemetryTags.
JSON schema validation and documentation crates/aisix-core/src/models/schema.rs	provider_key_schema() extended with Phase A fields as optional (preserving backward compatibility). Documentation added explaining that provider, adapter, and telemetry_tags are not yet wired to dispatch behavior. Schema tests added covering minimal payloads, legacy payloads without Phase A fields, and payloads with Phase A fields including both catalog and BYO telemetry shapes. Rejection tests verify unknown adapter values, unknown telemetry fields, and invalid kind discriminator values are caught.
Deserialization and round-trip compatibility tests crates/aisix-core/src/models/provider_key.rs	Test suite verifies legacy payloads without Phase A fields deserialize to defaults. Payloads with all Phase A fields (including telemetry tag values and BYO dual-label handling) deserialize correctly. adapter field rejects unknown adapter strings. telemetry_tags rejects unknown fields. A ProviderKey containing default Phase A fields successfully round-trips via JSON without losing semantic equality.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

---

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[moonming]

Independent audit (CLAUDE.md §8)

Cold re-read of the diff via gh pr diff 298 — no shared context with the implementation session. Reviewed against the six required angles.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. No maxLength bound on free-form telemetry fields (crates/aisix-core/src/models/schema.rs:288-307). branded_provider, pk_label, byo_label, and the top-level provider are all unbounded strings in the JSON Schema. A misbehaving (or compromised) cp-api could publish a 10MB-per-tag value and the DP would happily persist it into the snapshot. This is consistent with the pre-existing lack of maxLength on display_name/secret/api_base, so it is not a regression introduced here — but it is worth tightening in a follow-up Phase A PR (suggest 120 chars to match cache_policy.name).
   Suggested follow-up edit:

   "provider": { "type": "string", "maxLength": 120 },
   "branded_provider": { "type": ["string", "null"], "maxLength": 120 },
   "pk_label": { "type": ["string", "null"], "maxLength": 120 },
   "byo_label": { "type": ["string", "null"], "maxLength": 120 }

2. Schema does not enforce the catalog/byo mutual-exclusion shape (crates/aisix-core/src/models/schema.rs:288-307). The PR description notes that the canonical BYO shape has branded_provider:null + byo_label="…" and the catalog shape has branded_provider:"vendor" + byo_label:null, but the schema allows any combination (both set, both null, etc.). This is a deliberate Phase A skeleton choice — no dispatch reads the field yet, and adding a conditional oneOf now would foreclose what Phase A migration learns about real cp-api payloads. Worth pinning the constraint once Phase A wires the telemetry consumer.

3. TelemetryTags::Default produces all-None including kind: None, but the schema constrains kind to "catalog"|"byo" (no null allowed). A round-trip of TelemetryTags::default() via serde_json::to_value produces {} (because of skip_serializing_if = "Option::is_none"), which is schema-valid. A round-trip via serde_json::to_value with serialize_none = true (not used today) would emit kind: null and fail schema. This is a non-issue with current code, but worth a comment near the struct definition so a future PR doesn't add a null-emitting serializer.

Per-angle assessment

• Correctness: The PR claims "zero behavior change" and the diff bears it out — no dispatch site, snapshot loader, hub, or bridge reads the new fields. Verified by running cargo test --workspace (all 18 crate test suites pass, no downstream breakage). All five test helpers across the workspace that build ProviderKey go through serde_json::from_str, never struct-literal syntax, so the addition of new required-but-defaulted fields does not break any existing test. PASS.
• Reliability: Pure type extension; no async, no I/O, no time-sensitive code. PASS.
• Security: No new credential flow; secret semantics unchanged. The new fields are operator-supplied labels with no privileged interpretation in this PR. PASS for this PR (see LOW 1 for a hardening follow-up).
• Sensitive-info leakage: New fields are non-secret. The Debug impl on ProviderKey already leaks secret (pre-existing), and the new fields do not widen that exposure. PASS.
• Breaking changes: aisix-core is a workspace-internal crate (no publish = true). All in-workspace consumers reach ProviderKey via serde_json, never struct-literal construction with full field list — verified by grep. The wire format adds optional fields only; additionalProperties: false plus the existing on-disk payloads with no provider/adapter/telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes). PASS.
• E2E test coverage: Adequate for the "skeleton, no consumer" contract this PR claims. Both struct-level and schema-level legacy-payload-compat tests are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No need for cross-crate or HTTP-level e2e in this PR because there is no behavior under test — that lands in the follow-up Phase A sub-PR that wires dispatch.

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.

[moonming]

Independent third-party audit per CLAUDE.md §8 (separate from author self-review)

Cold re-read of this PR by a fresh agent with no shared context with the implementation session or with the author's own audit comment above. Reviewed against the six required angles and against the canonical wire shapes in api7/AISIX-Cloud#302 §5.

Findings

HIGH — none.

MEDIUM — none.

LOW

1. PR title slightly misrepresents the diff. The title says "add provider/adapter/display_name fields" but display_name is pre-existing on ProviderKey (verified by git show main:crates/aisix-core/src/models/provider_key.rs). The diff only adds provider, adapter, and telemetry_tags. The body is accurate ("Adds four new fields…" actually says 4 but lists 3 — display_name is correctly absent from the body's enumeration). The body explicitly says "Does not change display_name (pre-existed on ProviderKey)". Non-blocking — squash-merge commit title is what would land on main, suggest tightening to feat(core): add provider/adapter/telemetry_tags fields to ProviderKey (Phase A skeleton).

2. No maxLength on any of the new string fields (crates/aisix-core/src/models/schema.rs:288-307). Independently verified: a 100,000-char provider / pk_label / byo_label / branded_provider passes both schema validation and serde deserialisation. Consistent with the pre-existing lack of maxLength on display_name / secret / api_base, so not a regression — but AISIX-Cloud#302 §5 specifies display_name length 1-64 with sluggified-form ≥ 3. The DP currently enforces neither, leaving cp-api as the sole gatekeeper for label length. Worth tightening in a follow-up alongside the §5 validation work cp-api is committing to.

3. adapter:null is silently accepted by serde but rejected by schema. Independently verified:

   • Struct path: serde_json::from_str of {"adapter": null, …} → adapter = None, Ok(()).
   • Schema path: validate_provider_key of the same → rejected ("null is not one of [openai,…]").
   • Because the etcd loader runs schema validation before struct deserialisation, a wire payload with adapter:null is rejected at the schema gate, so this divergence has no observable impact in this PR. But it is worth a one-line code comment near the adapter field saying "schema is the gatekeeper for null-vs-absent; the serde Option only handles the absent case". Otherwise a future PR that builds a ProviderKey from a serde_json::Valuebypassing the schema (e.g. a unit test reading a fixture, or a CLI tool) could see adapter:None semantics for a payload that the schema would have rejected.

Per-angle assessment

• Correctness — PR claims "zero behavior change" and the diff bears it out. No dispatch site, snapshot loader, hub, or bridge reads the new fields. Independently verified by running cargo test -p aisix-core (150 passed, 0 failed) and cargo check --workspace (clean). All five ProviderKey builder helpers across the workspace (aisix-gateway/src/bridge.rs:361, aisix-provider-anthropic/src/bridge.rs:553, aisix-provider-openai/src/bridge.rs:787, aisix-core/src/models/snapshot.rs:74, aisix-proxy/src/dispatch.rs:283) use serde_json::from_str, never struct-literal — so the new fields ride on #[serde(default)] and existing tests don't need to be touched. The single struct-literal site (in the new round-trip test) is the only place that sets all fields explicitly, and it's correct. Independently verified the legacy-payload-compat contract by running the test under cargo test. PASS.

• Reliability — Pure type extension; no async, no I/O, no time-sensitive code, no new error path. The schema-then-struct two-stage parse on the etcd loader path is unchanged. PASS.

• Security — No new credential flow; secret semantics unchanged. The new fields are operator-supplied attribution labels with no privileged interpretation in this PR (no dispatch site reads them). AISIX-Cloud#302 §5 places the validation responsibility on cp-api's createProviderKey — the DP is correctly second-line-of-defence-only, which matches the spec. PASS for this PR (see LOW 2 for the future hardening alignment with §5).

• Sensitive-info leakage — New fields are non-secret by construction (vendor identity, adapter shape, telemetry tags). The pre-existing Debug impl on ProviderKey already leaks secret (out of scope here), and the new fields do not widen that exposure. The new Debug impl on TelemetryTags is fine — all fields are labels intended for log/metric emission anyway. PASS.

• Breaking changes — aisix-core is a workspace-internal crate (no publish = true in Cargo.toml). New struct fields are all #[serde(default)] so the wire format adds optional fields only. additionalProperties: false on the schema continues to apply, with the three new fields explicitly enumerated. Existing on-disk KV payloads with no provider / adapter / telemetry_tags keep validating (covered by provider_key_legacy_payload_without_phase_a_fields_passes and legacy_payload_without_phase_a_fields_deserialises_with_defaults). The new pub use TelemetryTags does not collide with any existing export (verified by rg "TelemetryTags" crates/ — only the new declaration and re-exports appear). PASS.

• E2E test coverage — Adequate for the "skeleton, no consumer" contract this PR claims. The 14 new tests cover both directions:

  • Schema-level: minimal, legacy-no-phase-a, full-phase-a, byo-shape, unknown-adapter, unknown-telemetry-field, unknown-top-level, unknown-kind.
  • Struct-level: legacy-no-phase-a-defaults, full-phase-a, byo-shape, unknown-telemetry-field, unknown-adapter, round-trip-with-defaults.
    Both layers are pinned. Round-trip test guards against accidental future serialize_none = true regressions. No cross-crate or HTTP-level e2e needed in this PR because there is no behavior under test — the dispatch wiring lands in the follow-up Phase A sub-PR per the PR description. PASS.

Independent edge-case probe (not added to the PR — purely for audit verification, see findings above)

Verified the following edge cases by spawning a temporary integration test in crates/aisix-core/tests/audit_check.rs (since deleted, working tree is clean):

Payload fragment	Serde result	Schema result	Outcome
"provider": null	Err — invalid type: null, expected a string	Err — null is not of type string	Both gate; OK
"telemetry_tags": null	Err — invalid type: null, expected struct	Err — null is not of type object	Both gate; OK
"adapter": null	Ok — None	Err — null is not one of [openai,…]	Divergence; see LOW 3
omitted (legacy)	Ok — defaults	Ok	Compat pinned
"telemetry_tags": {}	Ok — default	Ok	Compat pinned
provider = 100k chars	Ok	Ok	See LOW 2
Round-trip of TelemetryTags::default()	Emits {"featured": false} (kind/branded_provider/pk_label/byo_label all skip_serializing_if)	Schema accepts	OK

Merge gate

No HIGH or MEDIUM findings. The three LOW notes are documented for follow-up Phase A work and do not block merge.

PASS — recommend merge after CI.