Skip to content

Text-to-vector Test Refactoring#4419

Draft
nicolo-rinaldi wants to merge 5 commits into
apache:mainfrom
SeaseLtd:text-to-vector-test-refactoring
Draft

Text-to-vector Test Refactoring#4419
nicolo-rinaldi wants to merge 5 commits into
apache:mainfrom
SeaseLtd:text-to-vector-test-refactoring

Conversation

@nicolo-rinaldi
Copy link
Copy Markdown
Contributor

@nicolo-rinaldi nicolo-rinaldi commented May 12, 2026

Description

It turned out that, after the refactoring, some functions and classes were not clear enough. Those have been change to address this issue.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide
  • I have added a changelog entry for my change

@github-actions github-actions Bot added documentation Improvements or additions to documentation dependencies Dependency upgrades tool:build tests labels May 12, 2026
alessandrobenedetti
alessandrobenedetti approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@alessandrobenedetti alessandrobenedetti left a comment

Choose a reason for hiding this comment

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

left a couple of minors, once fixed we can merge!

Comment thread gradle/libs.versions.toml
androidx-lifecycle-runtimeCompose = { module = "org.jetbrains.androidx.lifecycle:lifecycle-runtime-compose", version.ref = "androidx-lifecycle" }
androidx-lifecycle-viewmodelCompose = { module = "org.jetbrains.androidx.lifecycle:lifecycle-viewmodel-compose", version.ref = "androidx-lifecycle" }
androidx-lifecycle-viewModelNav3 = { module = "org.jetbrains.androidx.lifecycle:lifecycle-viewmodel-navigation3", version.ref = "androidx-lifecycle" }
androidx-lifecycle-viewmodelCompose = { module = "org.jetbrains.androidx.lifecycle:lifecycle-viewmodel-compose", version.ref = "androidx-lifecycle" }
Copy link
Copy Markdown
Contributor

@alessandrobenedetti alessandrobenedetti May 12, 2026

Choose a reason for hiding this comment

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

suspicious, probably a mistake?

Copy link
Copy Markdown
Contributor Author

@nicolo-rinaldi nicolo-rinaldi May 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

No, this came out from ./gradlew tidy. My take is that the capital M changes the alphabetical order. The change has not been made by me personally

Comment thread solr/solr-ref-guide/modules/query-guide/pages/text-to-vector.adoc
----

Documentation Indexing time
Copy link
Copy Markdown
Contributor

@alessandrobenedetti alessandrobenedetti May 12, 2026

Choose a reason for hiding this comment

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

was this originally a typo?

Copy link
Copy Markdown
Contributor Author

@nicolo-rinaldi nicolo-rinaldi May 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I think yes (https://solr.apache.org/guide/solr/latest/query-guide/text-to-vector.html#:~:text=Documentation%20Indexing%20time), I changed it here to discuss

aruggero
aruggero reviewed May 14, 2026
View reviewed changes
Comment thread ...test/org/apache/solr/languagemodels/store/rest/TestTextToVectorModelStoreInitialization.java Outdated
Copy link
Copy Markdown
Contributor

@aruggero aruggero May 14, 2026

Choose a reason for hiding this comment

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

@nicolo-rinaldi managedModelStore or managedTextToVectorModelStore as for the document enrichment?

aruggero
aruggero reviewed May 14, 2026
View reviewed changes
Comment thread solr/modules/language-models/src/test/org/apache/solr/languagemodels/TestLanguageModelBase.java Outdated
Copy link
Copy Markdown
Contributor

@aruggero aruggero May 14, 2026

Choose a reason for hiding this comment

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

@nicolo-rinaldi "textToVectorModelStore"?

@nicolo-rinaldi nicolo-rinaldi force-pushed the text-to-vector-test-refactoring branch from 3df8074 to 7ea679b Compare May 14, 2026 10:56
alessandrobenedetti
alessandrobenedetti approved these changes May 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@alessandrobenedetti alessandrobenedetti left a comment

Choose a reason for hiding this comment

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

I'll proceed to merge now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alessandrobenedetti alessandrobenedetti alessandrobenedetti approved these changes

+1 more reviewer

@aruggero aruggero aruggero left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

dependencies Dependency upgrades documentation Improvements or additions to documentation tests tool:build

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nicolo-rinaldi @alessandrobenedetti @aruggero