From a0786b6fe75588f834ae5b8194309d2fda62ac19 Mon Sep 17 00:00:00 2001 From: langcaiye Date: Thu, 11 Jun 2026 19:29:29 +0800 Subject: [PATCH] fix(wiki): clear stale citation chunks on deletion --- .../application/service/knowledge_delete.go | 81 ++++++++++++------- .../service/knowledge_delete_test.go | 25 ++++++ internal/middleware/kb_access.go | 4 +- internal/middleware/kb_access_test.go | 4 + 4 files changed, 85 insertions(+), 29 deletions(-) create mode 100644 internal/application/service/knowledge_delete_test.go diff --git a/internal/application/service/knowledge_delete.go b/internal/application/service/knowledge_delete.go index f38f09fed..c0ebd8ac2 100644 --- a/internal/application/service/knowledge_delete.go +++ b/internal/application/service/knowledge_delete.go @@ -141,6 +141,12 @@ func (s *knowledgeService) DeleteKnowledge(ctx context.Context, id string) error logger.Infof(ctx, "Knowledge %s has no embedding model, skipping vector store cleanup", knowledge.ID) } + // Clean wiki pages before deleting chunks so cleanup can still identify + // which chunk_refs belonged to this source document. + if kb != nil && kb.IsWikiEnabled() { + s.cleanupWikiOnKnowledgeDelete(ctx, knowledge) + } + // Delete all chunks associated with this knowledge wg.Go(func() error { if err := s.chunkService.DeleteChunksByKnowledgeID(ctx, knowledge.ID); err != nil { @@ -176,16 +182,6 @@ func (s *knowledgeService) DeleteKnowledge(ctx context.Context, id string) error return nil }) - // Clean up wiki pages that reference this knowledge. Pass the full - // knowledge object so cleanup can source title/summary from the row - // itself rather than reaching into possibly-not-yet-written wiki pages. - if kb != nil && kb.IsWikiEnabled() { - wg.Go(func() error { - s.cleanupWikiOnKnowledgeDelete(ctx, knowledge) - return nil - }) - } - if err = wg.Wait(); err != nil { return err } @@ -251,6 +247,7 @@ func (s *knowledgeService) cleanupWikiOnKnowledgeDelete(ctx context.Context, kno logger.Warnf(ctx, "wiki cleanup: failed to list pages by source ref %s: %v", knowledgeID, err) pages = nil } + sourceChunkRefs := s.wikiChunkRefsForKnowledge(ctx, knowledge) // Prefer the on-disk summary if the summary page already exists (it's // richer than the raw user-provided description). Leave docSummary @@ -279,6 +276,7 @@ func (s *knowledgeService) cleanupWikiOnKnowledgeDelete(ctx context.Context, kno } } else { page.SourceRefs = remaining + page.ChunkRefs = removeChunkRefs(page.ChunkRefs, sourceChunkRefs) if err := s.wikiService.UpdatePageMeta(ctx, page); err != nil { logger.Warnf(ctx, "wiki cleanup: failed to update source refs for page %s: %v", page.Slug, err) } else { @@ -314,6 +312,25 @@ func (s *knowledgeService) cleanupWikiOnKnowledgeDelete(ctx context.Context, kno knowledgeID, len(allAffectedSlugs), allAffectedSlugs) } +func (s *knowledgeService) wikiChunkRefsForKnowledge(ctx context.Context, knowledge *types.Knowledge) map[string]bool { + if knowledge == nil || s.chunkRepo == nil { + return nil + } + chunks, err := s.chunkRepo.ListChunksByKnowledgeID(ctx, knowledge.TenantID, knowledge.ID) + if err != nil { + logger.Warnf(ctx, "wiki cleanup: failed to list chunks for knowledge %s: %v", knowledge.ID, err) + return nil + } + refs := make(map[string]bool, len(chunks)) + for _, chunk := range chunks { + if chunk == nil || chunk.ID == "" { + continue + } + refs[chunk.ID] = true + } + return refs +} + // markKnowledgeDeletedForWiki writes a short-TTL tombstone so any wiki_ingest // task still running or queued for this knowledge can short-circuit before // resurrecting a page with a stale source_ref. No-op when Redis is absent. @@ -391,6 +408,20 @@ func removeSourceRef(refs types.StringArray, knowledgeID string) types.StringArr return result } +func removeChunkRefs(refs types.StringArray, removed map[string]bool) types.StringArray { + if len(refs) == 0 || len(removed) == 0 { + return refs + } + result := make(types.StringArray, 0, len(refs)) + for _, ref := range refs { + if removed[ref] { + continue + } + result = append(result, ref) + } + return result +} + // DeleteKnowledgeList deletes a knowledge entry and all related resources func (s *knowledgeService) DeleteKnowledgeList(ctx context.Context, ids []string) error { if len(ids) == 0 { @@ -505,7 +536,16 @@ func (s *knowledgeService) DeleteKnowledgeList(ctx context.Context, ids []string return nil }) - // 3. Delete all chunks associated with this knowledge + // 3. Clean wiki pages before deleting chunks so cleanup can still identify + // which chunk_refs belonged to each source document. + for _, knowledge := range knowledgeList { + kb, _ := s.kbService.GetKnowledgeBaseByID(ctx, knowledge.KnowledgeBaseID) + if kb != nil && kb.IsWikiEnabled() { + s.cleanupWikiOnKnowledgeDelete(ctx, knowledge) + } + } + + // 4. Delete all chunks associated with this knowledge wg.Go(func() error { if err := s.chunkService.DeleteByKnowledgeList(ctx, ids); err != nil { logger.GetLogger(ctx).WithField("error", err).Errorf("DeleteKnowledge delete chunks failed") @@ -514,7 +554,7 @@ func (s *knowledgeService) DeleteKnowledgeList(ctx context.Context, ids []string return nil }) - // 4. Delete the physical file and extracted images if they exist + // 5. Delete the physical file and extracted images if they exist wg.Go(func() error { storageAdjust := int64(0) for _, knowledge := range knowledgeList { @@ -558,25 +598,10 @@ func (s *knowledgeService) DeleteKnowledgeList(ctx context.Context, ids []string return nil }) - // Clean up wiki pages that reference deleted knowledge. cleanup needs - // the full knowledge object (Title / Description) so the retract prompt - // can describe the vanished document even when wiki pages haven't been - // ingested yet — which is common in the batch-delete-shortly-after-upload - // flow. - wg.Go(func() error { - for _, knowledge := range knowledgeList { - kb, _ := s.kbService.GetKnowledgeBaseByID(ctx, knowledge.KnowledgeBaseID) - if kb != nil && kb.IsWikiEnabled() { - s.cleanupWikiOnKnowledgeDelete(ctx, knowledge) - } - } - return nil - }) - if err = wg.Wait(); err != nil { return err } - // 5. Delete the knowledge entry itself from the database + // 6. Delete the knowledge entry itself from the database return s.repo.DeleteKnowledgeList(ctx, tenantInfo.ID, ids) } diff --git a/internal/application/service/knowledge_delete_test.go b/internal/application/service/knowledge_delete_test.go new file mode 100644 index 000000000..55cd695a5 --- /dev/null +++ b/internal/application/service/knowledge_delete_test.go @@ -0,0 +1,25 @@ +package service + +import ( + "testing" + + "github.com/Tencent/WeKnora/internal/types" + "github.com/stretchr/testify/require" +) + +func TestRemoveChunkRefs(t *testing.T) { + got := removeChunkRefs( + types.StringArray{"chunk-a", "chunk-b", "chunk-c"}, + map[string]bool{"chunk-b": true}, + ) + + require.Equal(t, types.StringArray{"chunk-a", "chunk-c"}, got) +} + +func TestRemoveChunkRefsNoRemovedSet(t *testing.T) { + refs := types.StringArray{"chunk-a", "chunk-b"} + + got := removeChunkRefs(refs, nil) + + require.Equal(t, refs, got) +} diff --git a/internal/middleware/kb_access.go b/internal/middleware/kb_access.go index 1d560ff62..95ae514ed 100644 --- a/internal/middleware/kb_access.go +++ b/internal/middleware/kb_access.go @@ -3,6 +3,7 @@ package middleware import ( "context" stderrors "errors" + "strings" apprepo "github.com/Tencent/WeKnora/internal/application/repository" "github.com/Tencent/WeKnora/internal/config" @@ -190,7 +191,8 @@ func KBIDFromChunkIDParam(param string, chunkService ChunkLookup) KBIDResolver { // before this refactor is useful. func isResourceNotFound(err error) bool { return stderrors.Is(err, apprepo.ErrKnowledgeBaseNotFound) || - stderrors.Is(err, ErrResourceNotFound) + stderrors.Is(err, ErrResourceNotFound) || + strings.TrimSpace(err.Error()) == "chunk not found" } // RequireKBAccess returns a gin.HandlerFunc that resolves KB access diff --git a/internal/middleware/kb_access_test.go b/internal/middleware/kb_access_test.go index baccf60cb..3cf878b55 100644 --- a/internal/middleware/kb_access_test.go +++ b/internal/middleware/kb_access_test.go @@ -245,6 +245,10 @@ func TestRequireKBAccess_NotFound_Aborts(t *testing.T) { require.False(t, ok, "no access should be stashed on failure") } +func TestIsResourceNotFound_ChunkNotFound(t *testing.T) { + require.True(t, isResourceNotFound(errors.New("chunk not found"))) +} + func TestRequireKBAccess_SharedKB_RewritesTenantContext(t *testing.T) { share := &stubKBShareForGuard{ permission: map[string]types.OrgMemberRole{"kb-shared": types.OrgRoleEditor},