Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Associate boost factor data with a session id #2124

Merged
merged 2 commits into from
Dec 1, 2024

Conversation

kgilpin
Copy link
Contributor

@kgilpin kgilpin commented Nov 25, 2024

This allows multiple jobs / sessions to be using the index concurrently, applying different boost factors according to their needs.

Requirements

When searching for code and files, add a session id for the boost records.

When searching, the session id is a required argument, and it is used to select only the boost
records that are relevant to the current search session.


/include=search/src

Plan changes only in packages/search/src

Navie Plan

Add session id to search boost records in packages/search

Problem
The search system needs to differentiate between boost records from different search sessions to ensure boost records only affect search results within their intended context. This change should be implemented within the packages/search/src directory.

Analysis
We need to modify the file index and search functionality to support session-specific boost records. This involves:

  1. Adding session id to boost record storage
  2. Filtering boost records by session id during search
  3. Ensuring all boost operations include a session id

The core changes will be in the FileIndex class, which manages the SQLite database tables for file content and boost records.

Proposed Changes

  1. packages/search/src/file-index.ts:

    • Update the file_boost table schema to include a session_id column
    • Modify the boostFile method to require a session id parameter
    • Update the search SQL query to filter boost records by session id
    • Add session id parameter to search method signature
  2. packages/search/src/types.ts (new file):

    • Define interfaces for boost and search operations that include session id
    • Define types for search results and boost records
  3. packages/search/src/snippet-index.ts:

    • Update the snippet boosting functionality to include session id
    • Modify snippet search to consider session-specific boost factors
    • Update the search query to filter boost records by session id
  4. packages/search/src/index.ts:

    • Export the new session-aware interfaces and types
    • Update the public API to include session id in boost and search operations

This implementation focuses exclusively on the search package and maintains its core functionality while adding session-specific boost capabilities.

Review

✅ Logical Changes

The changes introduce a session ID to associate boost factors with specific search sessions, ensuring that boosts are session-specific and do not interfere with other sessions. Additionally, the database schema for boost tables is modified to include session_id as part of the primary key, and tests are updated to reflect these changes. Documentation is also added to explain the purpose of the session ID.

✅ Possible Bugs to Investigate

Notes

The initial review called out the lack of documentation for the session id feature.

Screen Shot 2024-11-25 at 2 28 49 PM

@kgilpin kgilpin force-pushed the feat/search-boost-session-id branch from 068ff7d to d89e438 Compare November 25, 2024 19:45
@kgilpin kgilpin marked this pull request as ready for review November 25, 2024 20:01
@kgilpin kgilpin self-assigned this Nov 25, 2024
@kgilpin kgilpin added navie enhancement New feature or request and removed navie labels Nov 25, 2024
This allows multiple jobs / sessions to be using the index concurrently,
applying different boost factors according to their needs.
@kgilpin kgilpin force-pushed the feat/search-boost-session-id branch from d89e438 to 9ccd947 Compare November 26, 2024 02:36
Base automatically changed from feat/update-appmap-file-search to main December 1, 2024 12:28
@kgilpin kgilpin merged commit d9651c7 into main Dec 1, 2024
23 checks passed
@appland-release
Copy link
Contributor

🎉 This PR is included in version @appland/search-v1.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@appland-release
Copy link
Contributor

🎉 This PR is included in version @appland/appmap-v3.175.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants