Skip to content

Move hydrate resolve model into a common place#1289

Merged
lramos15 merged 1 commit intomainfrom
lramos15/dramatic-gayal
Oct 10, 2025
Merged

Move hydrate resolve model into a common place#1289
lramos15 merged 1 commit intomainfrom
lramos15/dramatic-gayal

Conversation

@lramos15
Copy link
Member

No description provided.

Copilot AI review requested due to automatic review settings October 10, 2025 19:01
@lramos15 lramos15 enabled auto-merge October 10, 2025 19:01
@lramos15 lramos15 self-assigned this Oct 10, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Centralizes model hydration within the initial fetch loop instead of during subsequent resolution calls.

  • Removes per-call hydration in resolution methods, relying on hydration performed when populating _familyMap.
  • Adjusts loops to avoid redundant hydration and simplifies conditional checks for null or type mismatches.

Comment on lines +266 to +267
for (let model of data) {
model = await this._hydrateResolvedModel(model);
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

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

[nitpick] Reassigning the loop variable inside a for-of (let model of data) obscures the distinction between the raw API response and the hydrated model. Prefer using a distinct variable (e.g. for (const rawModel of data) { const model = await this._hydrateResolvedModel(rawModel); ... }) to improve clarity and prevent accidental use of the unhydrated object.

Suggested change
for (let model of data) {
model = await this._hydrateResolvedModel(model);
for (const rawModel of data) {
const model = await this._hydrateResolvedModel(rawModel);

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

not a bad suggestion

@vs-code-engineering vs-code-engineering bot added this to the October 2025 milestone Oct 10, 2025
@lramos15 lramos15 added this pull request to the merge queue Oct 10, 2025
Merged via the queue into main with commit 0f9ff43 Oct 10, 2025
16 checks passed
@lramos15 lramos15 deleted the lramos15/dramatic-gayal branch October 10, 2025 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants