Skip to content

fix(knowledge): resolve a shared knowledge base's embedding model under its owning tenant - #3913

Open
Yi-111-a wants to merge 1 commit into
Tencent:mainfrom
Yi-111-a:fix/shared-kb-embedding-model-tenant
Open

Yi-111-a wants to merge 1 commit into
Tencent:mainfrom
Yi-111-a:fix/shared-kb-embedding-model-tenant

Conversation

@Yi-111-a

Copy link
Copy Markdown

Description

A knowledge base shared into a shared space is written by the tenant that owns it, but it is parsed with the viewer's tenant in ctx. Every embedding-model resolution in the processing pipeline therefore looked the model row up in a tenant that never had it, and each document failed with Model not found:

[document_process=…] model.go:180[GetModelByID] | Model not found
[document_process=…] processChunks get embedding model failed

The result is that a shared knowledge base is unsearchable for everyone but its owner — every document it ingests ends in parse_status=failed with enable_status=disabled, so the shared space silently has nothing to retrieve.

The search side already gets this right: knowledgeBaseService.GetQueryEmbedding resolves a shared knowledge base's model under kb.TenantID. Only the processing pipeline was still resolving by ctx, so a shared knowledge base indexed and queried through different code paths.

This adds resolveKBEmbeddingModel, which applies the same branch the search path uses, and routes all five processing-pipeline call sites through it:

call site stage
processChunks chunk persistence + indexing
ProcessSummaryGeneration document summary
processQuestionGenerationForKnowledge per-document question generation
processQuestionGenerationForChunks per-chunk question generation
updateChunkVector chunk vector refresh after an edit

A ctx carrying no tenant still falls through to the plain lookup rather than panicking in the new branch: that error is the model service's to report, exactly as it was before.

Scoping notes

  • knowledgeBaseService.ProcessKBDelete also resolves a model by ctx, but it is a different service, it groups by the knowledge row's own EmbeddingModelID, and it already treats a resolution failure as non-fatal (logger.Warnf + continue) — so it does not produce the reported breakage. Left alone to keep this diff to one concern.
  • knowledge_clone_move.go resolves against a destination knowledge base the calling tenant owns, so ctx and kb.TenantID agree there by construction.

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 💥 Breaking change
  • 📚 Documentation update
  • 🎨 Refactor
  • ⚡ Performance improvement
  • 🧪 Test
  • 🔧 Configuration / Build / CI

Related Issue

Fixes #1998

Testing

New regression coverage in internal/application/service/knowledge_process_shared_kb_embedding_test.go, reusing the existing processChunks collaborators so the real code path runs:

  • TestProcessChunksIndexesSharedKnowledgeBaseUnderOwningTenant — a knowledge base owned by tenant 10000, parsed with tenant 10001 in ctx, against a model service that only knows the owner's row. Asserts the model is resolved under the owning tenant, the viewer's tenant is never used for the lookup, and the document is chunked and indexed.
  • TestProcessChunksKeepsCtxLookupForOwnKnowledgeBase — the ordinary owned path still uses the plain ctx lookup, so behaviour is unchanged without cross-tenant sharing.
  • TestResolveKBEmbeddingModel — the branch itself: shared → owner tenant, owned → ctx, a ctx with no tenant still falls through to the plain lookup instead of panicking, and an owner-side provider failure still surfaces.

Commands run (all green):

go build ./internal/application/service/
go vet  ./internal/application/service/
gofmt -l internal/                                   # clean
go test ./internal/application/service/ -run 'SharedKnowledgeBase|TestResolveKBEmbeddingModel|KeepsCtxLookup' -v

The new tests were confirmed to be real regressions: reverting only the five call sites to the previous ctx-tenant lookup makes TestProcessChunksIndexesSharedKnowledgeBaseUnderOwningTenant fail with "a shared knowledge base must resolve its embedding model under the owning tenant", and the fix is what turns it green.

The full package suite (go test ./internal/application/service/) passes except TestSkillPythonVerifier, which also fails on the unmodified main in this environment and is unrelated to this change (skill Python verifier, no shared-knowledge-base involvement).

Checklist

  • git diff --check origin/main...HEAD passes
  • Changed source files are formatted
  • Targeted tests for the changed packages/components pass
  • Diff-scoped lint passes where applicable (for Go: golangci-lint run --new-from-rev=origin/main ./...) — golangci-lint is not available in this environment; go vet and gofmt are clean
  • Full-repository checks were run, or any unrelated/environment-dependent failures are documented above
  • Self-reviewed the code
  • Added/updated tests covering the change
  • Updated related documentation (README, website-docs/, Swagger annotations, etc.) — no user-facing behaviour or API surface changes
  • Breaking changes are clearly called out in the description above

…g tenant

A knowledge base shared into a shared space is written by the owning tenant
but parsed with the viewer's tenant in ctx, so the plain ctx-tenant model
lookup searched a tenant that never had the model row. Every document in a
shared knowledge base then failed with "Model not found" and stayed
unindexed, so it was unsearchable for anyone but its owner.

Resolve under kb.TenantID when it differs from ctx, the same branch the
search path already uses in knowledgeBaseService.GetQueryEmbedding, and route
all five processing-pipeline call sites through it so a shared knowledge base
embeds and queries with one provider and vector space. A ctx carrying no
tenant still falls through to the plain lookup: that error belongs to the
model service, as it was before.

Fixes Tencent#1998

This branch has not been deployed

No deployments
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.

[Bug]: 跨租户共享场景下 GetEmbeddingModel 使用错误导致 "Model not found"

1 participant