Skip to content

refactor(go-models): route Bedrock ListModels through ParseListModel (#15853)#15956

Open
hunnyboy1217 wants to merge 1 commit into
infiniflow:mainfrom
hunnyboy1217:feat/go-listmodels-bedrock-15853
Open

refactor(go-models): route Bedrock ListModels through ParseListModel (#15853)#15956
hunnyboy1217 wants to merge 1 commit into
infiniflow:mainfrom
hunnyboy1217:feat/go-listmodels-bedrock-15853

Conversation

@hunnyboy1217

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Part of #15853 (provider model-list refactor).

Bedrock's ListModels already signs (SigV4) and calls the AWS ListFoundationModels control-plane endpoint, but it hand-built bare ListModelResponse{Name} values. This maps the AWS modelSummaries catalog into ModelList and enriches through the shared ParseListModel helper, consistent with the other migrated providers.

The empty-modelId guard is preserved (filtered before ParseListModel) so a malformed AWS response never leaks a blank entry to the UI dropdown. Output is unchanged today (Bedrock model ids are not yet in all_models.json, so enrichment is a no-op), but Bedrock now benefits automatically once its models are added to the registry.

Drive-by fix

Shared gitee_test.go DSModelList -> ModelList compile fix (renamed in #15900); auto-resolves against the sibling #15853 PRs.

Type of change

  • Refactoring

How has this been tested?

  • go build ./internal/entity/models/... — clean
  • go vet ./internal/entity/models/... — clean
  • Existing TestBedrockListModelsParsesCatalog passes unchanged (names + empty-id filtering preserved); TestBedrock*RequiresAPIKey pass locally.

…nfiniflow#15853)

Bedrock's ListModels already signs and calls the AWS ListFoundationModels
control-plane endpoint, but it hand-built bare ListModelResponse values. Map the
AWS modelSummaries catalog into ModelList and enrich through the shared
ParseListModel helper (issue infiniflow#15853), consistent with the other migrated
providers. The empty-modelId guard is preserved (filtered before ParseListModel)
so a malformed AWS response never leaks a blank entry to the UI.

Also includes the shared gitee_test.go DSModelList->ModelList compile fix
(infiniflow#15900 rename); it auto-resolves against the sibling infiniflow#15853 PRs.

Existing TestBedrockListModelsParsesCatalog passes unchanged (names preserved;
enrichment only adds fields). go build and go vet are clean.
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0f8a206d-142c-4a42-893e-def11846163e

📥 Commits

Reviewing files that changed from the base of the PR and between bde2b1f and 77316d6.

📒 Files selected for processing (2)
  • internal/entity/models/bedrock.go
  • internal/entity/models/gitee_test.go

📝 Walkthrough

Walkthrough

This PR consolidates model catalog parsing by refactoring Bedrock's ListModels to construct a ModelList and route through the shared ParseListModel helper instead of hand-building response objects. The corresponding test mock type is updated to match the new parsing structure.

Changes

Model list parsing consolidation

Layer / File(s) Summary
Bedrock list models refactoring
internal/entity/models/bedrock.go
BedrockModel.ListModels now builds a ModelList with DSModel entries (filtering empty modelIds) and returns ParseListModel(modelList) instead of directly constructing and returning []ListModelResponse.
Test mock response type alignment
internal/entity/models/gitee_test.go
Gitee test /models mock server response variable type changed from DSModelList to ModelList to match the refactored parsing pattern.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • infiniflow/ragflow#15940: Adds dimension handling propagation through ModelList and ParseListModel, building directly on this PR's model list parsing consolidation.
  • infiniflow/ragflow#15885: Adds DeepSeek alias-mapping tests to the Gitee ListModels test file that this PR modifies.

Suggested labels

☯️ refactor, size:XS

Poem

🐰 Bedrock now sings a simpler tune,
Through ParseListModel's shared cocoon,
No more hand-built response arrays—
Just ModelList, the cleaner way!
Tests align with graceful sway.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main refactoring: routing Bedrock's ListModels through the ParseListModel helper function.
Description check ✅ Passed The description fully addresses the template with clear problem statement, refactoring type selected, and comprehensive testing details.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. ☯️ refactor Pull request that refactor/refine code labels Jun 11, 2026

@Haruko386 Haruko386 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@hunnyboy1217 please show your output in RAGFlow CLI for your code tests

@Haruko386 Haruko386 removed the ☯️ refactor Pull request that refactor/refine code label Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants