feat(catalog): expose upstream zh descriptions via locale-aware API#98
Merged
Conversation
Upstream catalog/index.json already ships per-entry description_zh
alongside description (en); the ingest path now stores them in a new
descriptions jsonb column on capability_items and capability_versions,
mirroring the JSONB locale-map pattern item_categories already uses.
API endpoints resolve description per request locale (?lang= query
> Accept-Language > en) and surface the raw descriptions map so
clients can re-resolve on locale switch without re-fetching.
- migration: add descriptions jsonb DEFAULT '{}' on items + versions
- ingest: catalogEntry.DescriptionZh + integral-replacement write
- handlers/locale.go: ResolveLocale + PickDescription + list helpers
- buildItemResponse + bypass list/search/recommend paths wired up
- search SELECT columns include descriptions for vector/hybrid paths
- test fixtures (sqlite CREATE TABLE) updated; 24 new unit tests pass
Resolved description field stays backwards compatible — callers
that omit Accept-Language continue to see the English string.
…anizations cmd/migrate/main.go references uuid.New() at line 1261 in backfillOrganizations but never imported the package. Caught when the staging Docker build attempted RUN go build ./cmd/migrate and failed; local go build ./internal/... had been masking it.
📝 WalkthroughWalkthroughThis PR implements multilingual description support for catalog items. It adds a ChangesCatalog Internationalization (i18n) for Item Descriptions
Sequence Diagram(s)sequenceDiagram
participant Client
participant APIHandler
participant Database
participant ResolveLocale
participant PickDescription
Client->>APIHandler: GET /item/123?lang=zh<br/>(or Accept-Language: zh-CN)
APIHandler->>Database: fetch item (description + descriptions JSON)
Database-->>APIHandler: CapabilityItem{description: "...", descriptions: {"en": "...", "zh": "..."}}
APIHandler->>ResolveLocale: extract from context
ResolveLocale-->>APIHandler: "zh"
APIHandler->>PickDescription: descriptions JSON + "zh"
PickDescription-->>APIHandler: "中文描述"
APIHandler-->>Client: ItemResponse{description: "中文描述", descriptions: {...}}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
server/internal/handlers/locale_test.go (2)
25-87: ⚡ Quick winAdd tests for query parameter normalization and unsupported query locale.
The test suite covers
Accept-Languagenormalization (e.g.,zh-CN→zh) but doesn't verify that query parameters undergo the same normalization. Additionally, there's no test confirming that an unsupported query locale (e.g.,?lang=ja) falls back toen.🧪 Suggested test additions
func TestResolveLocale_QueryParamNormalizes(t *testing.T) { c := newTestContext(t, "/items?lang=zh-CN", "") if got := ResolveLocale(c); got != "zh" { t.Errorf("?lang=zh-CN should normalize to zh: got %q", got) } } func TestResolveLocale_QueryParamUnsupportedFallsBack(t *testing.T) { c := newTestContext(t, "/items?lang=ja", "") if got := ResolveLocale(c); got != "en" { t.Errorf("?lang=ja should fall back to en: got %q", got) } }These tests confirm that query parameters respect the same normalization and fallback rules as headers, preventing regressions in the API contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/internal/handlers/locale_test.go` around lines 25 - 87, Add two unit tests to verify query parameter locale normalization and fallback: implement TestResolveLocale_QueryParamNormalizes and TestResolveLocale_QueryParamUnsupportedFallsBack using newTestContext to create requests like "/items?lang=zh-CN" and "/items?lang=ja" and assert ResolveLocale(c) returns "zh" for the normalized zh-CN case and "en" for the unsupported ja case; this ensures ResolveLocale handles query param normalization and fallback the same way as Accept-Language header handling.
89-128: ⚡ Quick winAdd tests for the list resolution functions.
ResolveItemListLocaleandResolveCapabilityItemPointersLocaleare used by list, search, and recommendation endpoints to rewrite descriptions in-place, but they lack test coverage. Testing these functions ensures the in-place mutation logic and nil-pointer handling work correctly.🧪 Suggested test additions
func TestResolveItemListLocale(t *testing.T) { items := []models.CapabilityItem{ { Description: "fallback", Descriptions: datatypes.JSON([]byte(`{"en":"Hello","zh":"你好"}`)), }, { Description: "another fallback", Descriptions: datatypes.JSON([]byte(`{"en":"Goodbye"}`)), }, } c := newTestContext(t, "/items", "zh-CN") ResolveItemListLocale(c, items) if items[0].Description != "你好" { t.Errorf("items[0].Description: got %q, want 你好", items[0].Description) } if items[1].Description != "Goodbye" { t.Errorf("items[1].Description: got %q, want Goodbye (fallback to en)", items[1].Description) } } func TestResolveCapabilityItemPointersLocale(t *testing.T) { item1 := &models.CapabilityItem{ Description: "fallback", Descriptions: datatypes.JSON([]byte(`{"en":"Hello","zh":"你好"}`)), } item2 := &models.CapabilityItem{ Description: "another fallback", Descriptions: datatypes.JSON([]byte(`{"en":"Goodbye"}`)), } items := []*models.CapabilityItem{item1, nil, item2} c := newTestContext(t, "/items", "zh-CN") ResolveCapabilityItemPointersLocale(c, items) if item1.Description != "你好" { t.Errorf("item1.Description: got %q, want 你好", item1.Description) } if item2.Description != "Goodbye" { t.Errorf("item2.Description: got %q, want Goodbye (fallback to en)", item2.Description) } }These tests verify that in-place mutation works correctly, nil pointers are handled gracefully, and locale fallback logic is applied consistently across list endpoints.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/internal/handlers/locale_test.go` around lines 89 - 128, Add unit tests for ResolveItemListLocale and ResolveCapabilityItemPointersLocale that exercise in-place mutation, nil-pointer handling, and locale fallback: create sample CapabilityItem values with Description and Descriptions (datatypes.JSON) covering cases where the requested locale exists, falls back to "en", and where a pointer in the slice is nil; call ResolveItemListLocale(c, items) for a []models.CapabilityItem and ResolveCapabilityItemPointersLocale(c, itemsPtrs) for []*models.CapabilityItem using newTestContext(t, "/items", "zh-CN"); assert that items' Description fields are updated to the expected localized string ("你好" when zh present, "Goodbye" when only en present) and that nil entries are skipped without panics.server/internal/handlers/locale.go (1)
17-39: ⚡ Quick winConsider respecting Accept-Language q-value fallback preferences.
The current implementation honors only the first tag in
Accept-Language, ignoring quality values and secondary preferences. A user sendingAccept-Language: ja,zh;q=0.9would receiveen(fallback) instead of their second preferencezh. While the current behavior is clearly documented and reasonable for a first iteration, a more user-friendly approach would iterate through the tag list until a supported locale is found.♻️ Optional enhancement to respect fallback preferences
func ResolveLocale(c *gin.Context) string { if c == nil { return DefaultLocale } if q := strings.TrimSpace(c.Query("lang")); q != "" { return normalizeLocale(q) } if h := strings.TrimSpace(c.GetHeader("Accept-Language")); h != "" { - // Accept-Language: zh-CN,en;q=0.9 → take "zh-CN" (first tag), ignore q values. - first := strings.SplitN(h, ",", 2)[0] - first = strings.SplitN(first, ";", 2)[0] - return normalizeLocale(first) + // Accept-Language: ja,zh-CN,en;q=0.9 → try each tag until we find a supported one. + tags := strings.Split(h, ",") + for _, tag := range tags { + tag = strings.TrimSpace(strings.SplitN(tag, ";", 2)[0]) + if normalized := normalizeLocale(tag); normalized != DefaultLocale || tag == "" || strings.HasPrefix(strings.ToLower(tag), "en") { + return normalized + } + } } return DefaultLocale }This allows unsupported first-choice languages (e.g.,
ja) to fall through to supported secondary choices (e.g.,zh), improving UX for multilingual users.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/internal/handlers/locale.go` around lines 17 - 39, ResolveLocale currently only takes the first tag from Accept-Language and ignores q-values; update ResolveLocale to parse the Accept-Language header into slices of language tags with their q weights, sort them by descending q (default q=1 when absent), then iterate the sorted tags calling normalizeLocale(tag) and return the first normalized value that represents a supported locale (i.e., not falling back to DefaultLocale), otherwise continue to the next tag and finally return DefaultLocale; you can reuse normalizeLocale and/or add a small helper like isSupportedLocale to detect real-supported results rather than trusting the first raw tag.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/cmd/migrate/main_test.go`:
- Line 48: The test DB schema created by newMigrateTestDB is missing the
descriptions column on capability_versions while capability_items was updated;
update newMigrateTestDB to add "descriptions TEXT NOT NULL DEFAULT '{}'" to the
CREATE TABLE for capability_versions so the test schema matches the production
change to capability_items.descriptions, ensuring migration tests exercise the
same column shape for version rows.
---
Nitpick comments:
In `@server/internal/handlers/locale_test.go`:
- Around line 25-87: Add two unit tests to verify query parameter locale
normalization and fallback: implement TestResolveLocale_QueryParamNormalizes and
TestResolveLocale_QueryParamUnsupportedFallsBack using newTestContext to create
requests like "/items?lang=zh-CN" and "/items?lang=ja" and assert
ResolveLocale(c) returns "zh" for the normalized zh-CN case and "en" for the
unsupported ja case; this ensures ResolveLocale handles query param
normalization and fallback the same way as Accept-Language header handling.
- Around line 89-128: Add unit tests for ResolveItemListLocale and
ResolveCapabilityItemPointersLocale that exercise in-place mutation, nil-pointer
handling, and locale fallback: create sample CapabilityItem values with
Description and Descriptions (datatypes.JSON) covering cases where the requested
locale exists, falls back to "en", and where a pointer in the slice is nil; call
ResolveItemListLocale(c, items) for a []models.CapabilityItem and
ResolveCapabilityItemPointersLocale(c, itemsPtrs) for []*models.CapabilityItem
using newTestContext(t, "/items", "zh-CN"); assert that items' Description
fields are updated to the expected localized string ("你好" when zh present,
"Goodbye" when only en present) and that nil entries are skipped without panics.
In `@server/internal/handlers/locale.go`:
- Around line 17-39: ResolveLocale currently only takes the first tag from
Accept-Language and ignores q-values; update ResolveLocale to parse the
Accept-Language header into slices of language tags with their q weights, sort
them by descending q (default q=1 when absent), then iterate the sorted tags
calling normalizeLocale(tag) and return the first normalized value that
represents a supported locale (i.e., not falling back to DefaultLocale),
otherwise continue to the next tag and finally return DefaultLocale; you can
reuse normalizeLocale and/or add a small helper like isSupportedLocale to detect
real-supported results rather than trusting the first raw tag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68004327-1c28-4e75-9b50-d409783750ce
📒 Files selected for processing (16)
docs/CATALOG_INGEST.mdserver/cmd/migrate/main.goserver/cmd/migrate/main_test.goserver/internal/handlers/capability_item.goserver/internal/handlers/capability_registry.goserver/internal/handlers/locale.goserver/internal/handlers/locale_test.goserver/internal/handlers/recommend.goserver/internal/handlers/registry_test.goserver/internal/handlers/search.goserver/internal/models/models.goserver/internal/services/catalog_ingest_service.goserver/internal/services/catalog_ingest_service_test.goserver/internal/services/scan_service_test.goserver/internal/services/search_service.goserver/migrations/20260522000000_add_descriptions_jsonb_to_items.sql
| item_type TEXT NOT NULL, | ||
| name TEXT NOT NULL, | ||
| description TEXT, | ||
| descriptions TEXT NOT NULL DEFAULT '{}', |
There was a problem hiding this comment.
Keep test schema parity for capability_versions.descriptions.
Line 48 updates capability_items, but newMigrateTestDB still creates capability_versions without descriptions. That drift can hide/introduce migration-path failures when version rows are persisted through model paths.
🧪 Suggested schema parity patch
`CREATE TABLE IF NOT EXISTS capability_versions (
id TEXT PRIMARY KEY,
item_id TEXT NOT NULL,
revision INTEGER NOT NULL,
+ descriptions TEXT NOT NULL DEFAULT '{}',
content TEXT NOT NULL,
content_md5 TEXT DEFAULT '',
metadata TEXT DEFAULT '{}',
commit_msg TEXT,
created_by TEXT NOT NULL,
created_at DATETIME
)`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| descriptions TEXT NOT NULL DEFAULT '{}', | |
| `CREATE TABLE IF NOT EXISTS capability_versions ( | |
| id TEXT PRIMARY KEY, | |
| item_id TEXT NOT NULL, | |
| revision INTEGER NOT NULL, | |
| descriptions TEXT NOT NULL DEFAULT '{}', | |
| content TEXT NOT NULL, | |
| content_md5 TEXT DEFAULT '', | |
| metadata TEXT DEFAULT '{}', | |
| commit_msg TEXT, | |
| created_by TEXT NOT NULL, | |
| created_at DATETIME | |
| )`, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/cmd/migrate/main_test.go` at line 48, The test DB schema created by
newMigrateTestDB is missing the descriptions column on capability_versions while
capability_items was updated; update newMigrateTestDB to add "descriptions TEXT
NOT NULL DEFAULT '{}'" to the CREATE TABLE for capability_versions so the test
schema matches the production change to capability_items.descriptions, ensuring
migration tests exercise the same column shape for version rows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Upstream
catalog/index.jsonalready ships per-entrydescription_zhalongsidedescription(≈99.8% coverage); the ingest path now stores both in a newdescriptions jsonbcolumn oncapability_itemsandcapability_versions, mirroring the JSONB locale-map patternitem_categoriesalready uses.API endpoints resolve
descriptionper request locale (?lang=query >Accept-Language>en) and surface the rawdescriptionsmap so clients can re-resolve on locale switch without re-fetching.What changed
20260522000000_…):descriptions jsonb NOT NULL DEFAULT '{}'on items + versionscatalog_ingest_service.go):catalogEntry.DescriptionZhfield +buildDescriptionsJSON/descriptionsJSONEqualhelpers; integral-replacement semantics (upstream is the source of truth — drop a zh translation upstream and the next ingest clears it from DB)handlers/locale.go):ResolveLocale(?lang= > Accept-Language > en) +PickDescription(fallback chain) + list/search bypass helpersItemResponse.Descriptions+buildItemResponse(c, …)signature; list (ListItems/ListMyItems), recommend (GetTrending/GetNewAndNoteworthy), search (SemanticSearch/HybridSearch/FindSimilar) all wired upSELECTcolumns now includedescriptionsdescriptiononly — no vector-space driftCREATE TABLEin 3 test files) updated; 24 new unit tests (9 ingest + 15 locale)cmd/migrate/main.goimportsgoogle/uuid(caught by Docker build — localgo build ./internal/...was masking it)Backward compatibility
description textcolumn kept as the resolved-English fallback. Callers that omitAccept-Languagecontinue to see the English string (zero API contract change). Frontend client (PR zgsm-sangfor/opencode#512) consumes the newdescriptionsmap directly.Test plan
go test ./internal/handlers/ ./internal/services/(24 new tests, plus existing pass; 2 preexisting failures inmarketplace_test/usage_testunrelated to this change)metadataUpdated=3315(≈ every row got zh backfill, exactly as expected for first-time i18n ingest), failed=0, incomplete=0curl -H 'Accept-Language: zh' /api/itemsreturns Chinesedescription;curl /api/items(no header) returns English (back-compat verified)models.MemoryFileswag parse error — separate issue)Summary by CodeRabbit
Release Notes
New Features
Documentation