fix: preserve models with colliding flattened Go names - #8236
Shubham Padkonde (Shubham-Padkonde) wants to merge 10 commits into
Conversation
|
Shubham Padkonde (@Shubham-Padkonde) your behaviour is disruptive, opening about 30 PRs on the repos in a couple of hours time, most likely code that was AI generated and not tested/reviewed, will only clog our review pipeline. Please stop opening additional pull requests and work on the comments added to existing ones. We have turned on pull requests limits on this repository, and will proceed to blocking you at the organization level if additional pull requests are opened on other repositories before the already opened ones are reviewed and closed/or merged. |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The cyclic model migration path can still lose colliding models.
Review effort: Lite
Findings: None
What changed in this PR
Fixes Go model loss caused by colliding flattened names by preserving models with deterministic suffixes.
Changes:
- Adds collision-safe, stable model naming.
- Adds regression coverage for collisions and ordering.
- Documents the fix in the changelog.
| File | Summary |
|---|---|
src/Kiota.Builder/Refiners/GoRefiner.cs |
Preserves colliding models during flattening; cyclic migration still lacks collision-safe allocation. |
tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs |
Adds collision, suffix, and insertion-order regression tests. |
CHANGELOG.md |
Documents the Go generation fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Shubham Padkonde (@Shubham-Padkonde) this PR is conflicting, would you mind handling the conflicts when you have a minute please? |
|
This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged. |
|
Conflicts have been resolved. A maintainer will take a look shortly. |
|
Conflicts have been resolved. A maintainer will take a look shortly. |
|
This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged. |
|
Conflicts have been resolved. A maintainer will take a look shortly. |
|
This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged. |
|
Conflicts have been resolved. A maintainer will take a look shortly. |
|
This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged. |
|
Conflicts have been resolved. A maintainer will take a look shortly. |
Fixes #7823.
Go flattens inline model classes into their top-level package. When two distinct models acquire the same flattened name,
AddClasskeeps one and leaves references to the other pointing at a model that later refinement never visits. For Pipedrive's/oauth/tokenand/oauth/token/, that produces an interface embedding the unrefinedIAdditionalDataHolderand the generated client does not compile.Choose an available name before moving a model, retaining both objects and their existing references. Visit children in ordinal name order so suffix allocation is stable. Existing names are preserved where there is no collision.
Tests cover preserving both refined interfaces, existing numeric suffixes, multiple collisions, and reversed insertion order. The original preservation test fails before the change.
External validation uses the two token paths extracted from Pipedrive's upstream OpenAPI description:
undefined: IAdditionalDataHolder.Developed with Codex assistance. This does not claim validation of the full Pipedrive SDK or any live OAuth calls.
Additional validation:
The focused retry of GetsGithubFromApisGuruWithExactMatchAsync also timed out during its external HttpClient download, after its search result was found; that test does not exercise Go refinement. The full suite is therefore not reported as fully green.