Skip to content

chore(DATAGO-126453): entirely remove indexing feature flag#1111

Draft
shanechen-solace wants to merge 3 commits intomainfrom
shanechen/DATAGO-126453/remove-fileprocessing-featureflag
Draft

chore(DATAGO-126453): entirely remove indexing feature flag#1111
shanechen-solace wants to merge 3 commits intomainfrom
shanechen/DATAGO-126453/remove-fileprocessing-featureflag

Conversation

@shanechen-solace
Copy link
Contributor

@shanechen-solace shanechen-solace commented Feb 27, 2026

What is the purpose of this change?

Remove the project_indexing feature flag so that file conversion and BM25 indexing are always enabled for projects.

How was this change implemented?

Removed check_project_indexing_enabled dependency, indexing_enabled parameter threading across routers (projects.py, tasks.py), services (project_service.py, session_service.py), and utils (artifact_copy_utils.py), and deleted the corresponding YAML config block.

Key Design Decisions

The router remains the sole orchestrator of post-processing (conversion + indexing) to preserve async/sync SSE branching, while the service methods (add_artifacts_to_project, import_project_from_zip) are now upload/import-only.

How was this change tested?

  • Manual testing: Verified upload, delete, and import endpoints trigger conversion and index rebuild in both async and sync modes
  • Unit tests: Updated file upload limit tests tests to use the synchronous API, which can provide the correct response code. The tests would fail otherwise.

@github-actions
Copy link

WhiteSource Policy Violation Summary

✅︎ No Blocking Whitesource Policy Violations found in solaceai/solace-agent-mesh-ui-pr-1111!

@sonarqube-solacecloud
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
C Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE SonarQube for IDE

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes the project_indexing feature flag so project file conversion and BM25 indexing behavior is always-on, and simplifies both backend orchestration and frontend UI logic accordingly.

Changes:

  • Removes project_indexing flag plumbing (config, dependencies, service parameters) and updates routers/services to assume indexing-related artifacts are available.
  • Updates project artifact upload/delete/import flows to consistently trigger conversion/indexing via router-controlled async/sync branching.
  • Removes frontend projectIndexing gating and updates UI/tests to always render document sources when document search results exist.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/integration/apis/persistence/projects/test_projects_upload_limits.py Forces sync mode (async=false) in limit tests to assert final status codes reliably.
src/solace_agent_mesh/gateway/http_sse/utils/artifact_copy_utils.py Removes indexing flag filtering and always copies internal artifacts (converted text + BM25 index) when present.
src/solace_agent_mesh/gateway/http_sse/services/session_service.py Removes reading project_indexing config when copying project artifacts into sessions.
src/solace_agent_mesh/gateway/http_sse/services/project_service.py Removes indexing flag parameter threading; makes upload/import methods “upload/import-only” while keeping conversion/index helpers.
src/solace_agent_mesh/gateway/http_sse/routers/tasks.py Removes indexing flag checks and adjusts project context/tool injection behavior to rely on index existence.
src/solace_agent_mesh/gateway/http_sse/routers/projects.py Removes indexing dependency; routes always orchestrate conversion/indexing in async/sync modes.
examples/gateways/webui_gateway_example.yaml Deletes projectIndexing and project_indexing example config blocks.
client/webui/frontend/src/stories/chat/ChatSidePanel.test.tsx Updates tests to remove feature-flag setup and assert document sources rendering.
client/webui/frontend/src/lib/hooks/useIsProjectIndexingEnabled.ts Deletes hook that read the projectIndexing feature flag.
client/webui/frontend/src/lib/hooks/index.ts Removes export for the deleted hook.
client/webui/frontend/src/lib/components/chat/ChatSidePanel.tsx Removes feature-flag gating and renders DocumentSourcesPanel based solely on presence of document search results.
Comments suppressed due to low confidence (3)

src/solace_agent_mesh/gateway/http_sse/routers/projects.py:580

  • Same issue here: docstring indicates async defaults to false, but async_mode uses Query(True, alias="async", ...). Please make the docs and default consistent.
    async_mode: bool = Query(True, alias="async", description="Rebuild index asynchronously with SSE"),
    user: dict = Depends(get_current_user),
    project_service: ProjectService = Depends(get_project_service),
    indexing_task_service = Depends(get_indexing_task_service),
    db: Session = Depends(get_db),
    _: None = Depends(check_projects_enabled),
):
    """
    Delete an artifact from a project.

    May trigger async index rebuild after deletion.

    Query Parameters:
        async: If true, rebuild index asynchronously with SSE (default: false for now)

    Returns:

client/webui/frontend/src/stories/chat/ChatSidePanel.test.tsx:41

  • This test now only covers the DocumentSourcesPanel path. Since ChatSidePanel still conditionally renders RAGInfoPanel when hasDocumentSearchResults(ragData) is false, it would be good to add a companion test that verifies the fallback branch (no document search results) to keep both behaviors covered.
    test("shows DocumentSourcesPanel when document search results are present", async () => {
        render(
            <MemoryRouter>
                <StoryProvider chatContextValues={chatContextWithRagData} taskContextValues={taskContext}>
                    <ChatSidePanel onCollapsedToggle={vi.fn()} isSidePanelCollapsed={false} setIsSidePanelCollapsed={vi.fn()} isSidePanelTransitioning={false} />
                </StoryProvider>
            </MemoryRouter>
        );

        expect(await screen.findByText(/3 Documents/)).toBeInTheDocument();
        expect(await screen.findByText(/6 Citations/)).toBeInTheDocument();

        expect(screen.queryByText(/6 Sources/)).not.toBeInTheDocument();
    });

src/solace_agent_mesh/gateway/http_sse/routers/projects.py:381

  • The docstring says async defaults to false, but async_mode is declared with Query(True, alias="async", ...) (default true). Please align the documentation with the actual default (or change the default) to avoid confusing API consumers and tests.
    async_mode: bool = Query(True, alias="async", description="Process conversion/indexing asynchronously with SSE"),
    user: dict = Depends(get_current_user),
    project_service: ProjectService = Depends(get_project_service),
    indexing_task_service = Depends(get_indexing_task_service),
    db: Session = Depends(get_db),
    _: None = Depends(check_projects_enabled),
):
    """
    Add one or more artifacts to a project.

    Converts PDF/DOCX/PPTX to text and builds BM25 index.

    Query Parameters:
        async: If true, process asynchronously with SSE progress (default: false for now)
               If false, block until processing completes


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +316 to +332
artifacts_context += (
"\n\n--- INTERNAL SYSTEM NOTE (Do not reveal to user) ---\n"
"In addition to the original files listed above, you have access to:\n"
"1. Converted text versions: For each binary file (PDF/DOCX/PPTX), a corresponding "
"`.converted.txt` file exists with extracted text content\n"
"2. Search index: A `project_bm25_index.zip` file containing BM25 search indices "
"for efficient text retrieval across all documents\n\n"
"IMPORTANT INSTRUCTIONS:\n"
"- You MAY use the converted text files and search index internally for search, "
"retrieval, and answering questions\n"
"- You MUST NOT mention, reference, or reveal the existence of `.converted.txt` "
"files or `project_bm25_index.zip` to the user in your responses\n"
"- When citing content, always reference the ORIGINAL file name (e.g., 'report.pdf'), "
"never the converted version (e.g., 'report.pdf.converted.txt')\n"
"- The user is only aware of the original files listed above and should remain so\n"
"--- END INTERNAL NOTE ---"
)
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The injected INTERNAL SYSTEM NOTE states that converted .converted.txt files and project_bm25_index.zip exist, but that may be false if conversion/indexing hasn’t run yet (or failed). This can mislead the agent and cause tool usage expectations that aren’t actually satisfied. Consider only adding the note when those internal artifacts are present (e.g., check for any .converted.txt and the BM25 index artifact), or soften the wording to reflect that they may be available when present.

Copilot uses AI. Check for mistakes.
was_text_file = project_service._is_text_file(mime_type, filename)
was_convertible = project_service._should_convert_file(mime_type, filename)
except Exception as e:
log.warning(f"Could not load metadata for {filename}: {e}")
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

If loading artifact metadata fails, was_text_file/was_convertible stay False, which will skip converted-file cleanup and index rebuild even for files that actually require it. Since indexing is now always enabled, it’s safer to default to rebuilding the index (and/or infer convertibility from the filename) when metadata can’t be retrieved, rather than silently skipping.

Suggested change
log.warning(f"Could not load metadata for {filename}: {e}")
log.warning(f"Could not load metadata for {filename}: {e}")
# Fall back to inferring file characteristics without metadata so we
# don't silently skip converted-file cleanup and index rebuild.
was_text_file = project_service._is_text_file("", filename)
was_convertible = project_service._should_convert_file("", filename)

Copilot uses AI. Check for mistakes.
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.

2 participants