Skip to content

Feat: search Volpiano in the MEI for manuscripts without chant records - #1020

Open
CThierrin wants to merge 1 commit into
mainfrom
feat/volpiano-search-from-mei
Open

CThierrin wants to merge 1 commit into
mainfrom
feat/volpiano-search-from-mei

Conversation

@CThierrin

@CThierrin CThierrin commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Volpiano search queries chant records imported from Cantus Database, so it returns nothing for a manuscript that only has MEI (e.g. Liber Usualis). For those manuscripts, offer Volpiano under notation search instead: the query is converted to pitch names and matched against the existing OMR n-gram index. Octave, accidentals, clefs and barlines are ignored, since the index does not record them.

The fallback applies only when a manuscript has no chant records and has pitch search enabled; other manuscripts keep the chant-based search.

Resolves Issue #1018

Summary by CodeRabbit

  • New Features
    • Added Volpiano notation search for manuscripts without loaded chant records.
    • Search matches pitch names while ignoring octave, accidentals, clefs, and barlines. Short melodies are always searched; longer melodies are matched within a five-neume span.
    • Added in-app help explaining Volpiano notation search.
  • Bug Fixes
    • Invalid Volpiano queries now return a clear error instead of proceeding as a search.

Volpiano search queries chant records imported from Cantus Database, so it
returns nothing for a manuscript that only has MEI (e.g. Liber Usualis).
For those manuscripts, offer Volpiano under notation search instead: the
query is converted to pitch names and matched against the existing OMR
n-gram index. Octave, accidentals, clefs and barlines are ignored, since
the index does not record them.

The fallback applies only when a manuscript has no chant records and has
pitch search enabled; other manuscripts keep the chant-based search.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 8779eb0b-a1b9-4138-8a75-a53925a8146f
📥 Commits

Reviewing files that changed from the base of the PR and between 3066178 and 65f77db.

📒 Files selected for processing: 9
  • app/public/cantusdata/helpers/search_utils.py
  • app/public/cantusdata/test/core/helpers/test_search_utils.py
  • app/public/cantusdata/test/core/views/test_search_notation_view.py
  • app/public/cantusdata/views/search_notation.py
  • nginx/app/src/js/manuscript-detail/ManuscriptDetailPageView.js
  • nginx/app/src/js/search/chant-search/ChantSearchProvider.js
  • nginx/app/src/js/search/omr-search/OMRSearchProvider.js
  • nginx/app/src/js/search/omr-search/VolpianoHelpView.js
  • nginx/app/src/js/search/omr-search/volpiano-help.template.html
📝 Walkthrough

The change adds Volpiano-to-pitch-name conversion for notation queries. It also enables a Volpiano search field for manuscripts with unloaded chants when the pitch-search plugin is active, and provides search guidance.

Changes: Volpiano notation search

Layer / File(s) Summary
Convert Volpiano queries
app/public/cantusdata/helpers/search_utils.py, app/public/cantusdata/views/search_notation.py, app/public/cantusdata/test/core/helpers/test_search_utils.py, app/public/cantusdata/test/core/views/test_search_notation_view.py
The conversion maps Volpiano note characters to pitch names and skips configured notation characters. The query handler preserves the input case for conversion and raises NotationSearchException if conversion returns no notes or encounters an unrecognized character. Tests cover conversion, invalid input, and equivalent Volpiano and pitch-name search results.
Enable Volpiano search in the manuscript interface
nginx/app/src/js/manuscript-detail/ManuscriptDetailPageView.js, nginx/app/src/js/search/chant-search/ChantSearchProvider.js, nginx/app/src/js/search/omr-search/*
When chants are not loaded and the pitch-search plugin is active, the manuscript page excludes volpiano and volpiano_literal from chant search and enables the Volpiano field in OMR search. The OMR provider displays help text for that field.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 65f77

Volpiano search appears ready to merge after normal checks; no material issue was established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 8 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main change: adding Volpiano search in MEI data for manuscripts without chant records.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 8 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@etosphere etosphere left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! The converter is clean and well-tested, and excludedFields is a nice, small way to change what a manuscript offers. However, I have a question for this approach:

If I understand it correctly, your Volpiano search here becomes a Pitch search with a different input format and work differently compared with on other manuscripts. If no one ask for searching the MEI by using Volpiano, I'd suggest simply grey out the search fields that are not functioning well for a given manuscript (which can be achieved using your excludedFields in this PR).

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.

3 participants