Conversation
975a4ec to
674c713
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The updated nav.adoc contains invalid/incorrect list and link syntax (missing *, incorrect nesting levels, and malformed external link attribute lists) that will likely break or misrender the documentation navigation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the Antora documentation navigation (nav.adoc) to use the new grouped nav structure with subheadings and reorganized sections.
Changes:
- Introduces titled nav groups (for example, “Getting started”, “Domain modeling”) and restructures list nesting.
- Adds new “Upgrading Timefold Solver” entries (including release notes) and an “Additional resources” group.
- Reorders existing sections to fit the new hierarchy.
File summaries
| File | Description |
|---|---|
| docs/src/modules/ROOT/nav.adoc | Reworks the Antora nav structure to use group headings and updated link organization. |
Review details
Suppressed comments (5)
docs/src/modules/ROOT/nav.adoc:48
- The '.Deploying to the Timefold Platform' section starts its items at
**level without a parent*, which breaks the nav list structure; change these items to*level.
.Deploying to the Timefold Platform
** xref:deploying-to-platform/introduction.adoc[Overview]
** xref:deploying-to-platform/guide.adoc[Guide]
** xref:deploying-to-platform/model-metadata.adoc[leveloffset=+1]
** xref:deploying-to-platform/metrics.adoc[leveloffset=+1]
docs/src/modules/ROOT/nav.adoc:63
- The '.Responding to change' section starts its items at
**level without a parent*, which breaks the nav list structure; change these items to*level.
.Responding to change
** xref:responding-to-change/continuous-planning.adoc[leveloffset=+1]
** xref:responding-to-change/real-time-planning.adoc[leveloffset=+1]
** xref:responding-to-change/non-disruptive-replanning.adoc[leveloffset=+1]
** xref:responding-to-change/recommendation-api.adoc[leveloffset=+1]
docs/src/modules/ROOT/nav.adoc:67
- The '.Example use cases' section starts its items at
**level without a parent*, which breaks the nav list structure; change these items to*level.
.Example use cases
** xref:quickstart/quarkus-vehicle-routing/quarkus-vehicle-routing-quickstart.adoc[Vehicle routing (guide)]
** https://github.com/TimefoldAI/timefold-quickstarts[More examples on GitHub^]
docs/src/modules/ROOT/nav.adoc:83
- The '.Commercial editions' section starts its items at
**level without a parent*, which breaks the nav list structure; change these entries to*level.
.Commercial editions
** xref:commercial-editions/commercial-editions.adoc[Overview]
** xref:commercial-editions/installation.adoc[Installation]
** xref:commercial-editions/performance-improvements.adoc[leveloffset=+1]
** xref:constraints-and-score/understanding-the-score.adoc[Score analysis]
docs/src/modules/ROOT/nav.adoc:54
- The '.Optimization algorithms' section uses
**/***nesting without a top-level*parent item, which will mis-nest or invalidate the nav list; shift the first level to*and nest subitems under it.
.Optimization algorithms
** xref:optimization-algorithms/overview.adoc[Overview]
** xref:optimization-algorithms/construction-heuristics.adoc[leveloffset=+1]
** xref:optimization-algorithms/local-search.adoc[leveloffset=+1]
** xref:optimization-algorithms/exhaustive-search.adoc[leveloffset=+1]
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
674c713 to
b9c5abb
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The updated nav file includes syntax that likely prevents the Introduction item from showing and may render stray [leveloffset=+1] text for external links.
Review details
Suppressed comments (2)
docs/src/modules/ROOT/nav.adoc:73
- The external links use a double-bracket syntax (
...[Text][leveloffset=+1]) that AsciiDoc will treat as stray text and can break or pollute the rendered nav.
* https://github.com/TimefoldAI/timefold-solver/releases[New and noteworthy][leveloffset=+1]
* xref:upgrading-timefold-solver/overview.adoc[leveloffset=+1]
* xref:upgrading-timefold-solver/upgrade-from-v1.adoc[leveloffset=+1]
* https://docs.timefold.ai/timefold-solver/1.x/upgrading-timefold-solver/upgrade-from-optaplanner[Upgrading from OptaPlanner][leveloffset=+1]
docs/src/modules/ROOT/nav.adoc:1
- The introduction entry is no longer a list item, which likely prevents it from appearing in the Antora navigation.
xref:introduction.adoc[leveloffset=+1]
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
b9c5abb to
0428d50
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The nav includes external links with two bracketed attribute lists (e.g. ...[Label][leveloffset=+1]), which is invalid AsciiDoc macro syntax and may render incorrectly in the sidebar.
Review details
Suppressed comments (2)
docs/src/modules/ROOT/nav.adoc:70
- These external-link nav entries use two bracketed attribute lists, but AsciiDoc link macros accept only a single attribute list so the trailing "[leveloffset=+1]" will likely render as literal text or be ignored.
* https://github.com/TimefoldAI/timefold-solver/releases[New and noteworthy][leveloffset=+1]
docs/src/modules/ROOT/nav.adoc:73
- This external-link nav entry uses two bracketed attribute lists, but AsciiDoc link macros accept only a single attribute list so the trailing "[leveloffset=+1]" will likely render as literal text or be ignored.
* https://docs.timefold.ai/timefold-solver/1.x/upgrading-timefold-solver/upgrade-from-optaplanner[Upgrading from OptaPlanner][leveloffset=+1]
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate findings affect URL preservation and the correctness and consistency of the migration guidance.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (6)
Previously missed (1) — in code that hasn't changed since the last review.
docs/src/modules/ROOT/pages/upgrading-timefold-solver/overview.adoc:18
- Deleting the old
backwards-compatibility.adocpage removes its public URL, so existing bookmarks and external links now return 404; preserve it by adding:page-aliases: upgrading-timefold-solver/backwards-compatibility.adocto this document's header.
docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-from-optaplanner.adoc:69
- “Unsecure” is not the established adjective for this security warning and reads incorrectly; use “insecure”.
* The unsecure module `persistence-xstream` is removed, because of old, unresolved CVEs in XStream.
docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-from-optaplanner.adoc:14
Javais incorrectly lowercased in this user-facing instruction, and the list is missing a serial comma; capitalize it and make the sentence grammatically clear.
Run the command below to upgrade your java, build and other code automatically.
docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-from-optaplanner.adoc:17
- This page is now exposed in the current 2.x navigation and its command resolves
{timefold-solver-version}, but the manual section still describes a Timefold Solver 1.x migration and then points users to the 2.x recipe, leaving the documented upgrade path internally contradictory; either keep this page under the 1.x documentation or rewrite it for the OptaPlanner-to-2.x migration.
NOTE: The script below upgrades your OptaPlanner project to the latest Timefold Solver version, which is {timefold-solver-version}.
This is a significant jump and might require additional migrations. Do check out our other guide for more details on how to the xref:upgrading-timefold-solver/upgrade-from-v1.adoc[upgrade to 2.x range.]
docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-from-optaplanner.adoc:26
- The
ToLatestrecipe shown here does not migrate OptaPlanner coordinates or imports: the repository implementation has noorg.optaplanner/optaplannertransformations, so running this command on an OptaPlanner project leaves the advertised upgrade incomplete; add a dedicated OptaPlanner recipe or remove this automatic path.
mvn org.openrewrite.maven:rewrite-maven-plugin:{rewrite-maven-plugin-version}:run -Drewrite.recipeArtifactCoordinates=ai.timefold.solver:timefold-solver-migration:{timefold-solver-version} -Drewrite.activeRecipes=ai.timefold.solver.migration.ToLatest
docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-from-optaplanner.adoc:71
- Backticks make
ScoreConverterlook like a concrete replacement type, but this repository exposes score-specific classes such asHardSoftScoreConverterand no genericScoreConverter, so the migration instruction is not actionable.
Use JPA's `ScoreConverter` instead.
- Files reviewed: 7/7 changed files
- Comments generated: 5
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Correct the Antora section syntax and preserve the legacy page URL and fragment before approval.
Review details
Suppressed comments (10)
Previously missed (1) — in code that hasn't changed since the last review.
docs/src/modules/ROOT/pages/upgrading-timefold-solver/overview.adoc:18
- Moving the content out of
backwards-compatibility.adocwithout an alias removes that established page URL and its#backwardsCompatibilityfragment, so existing bookmarks and links break; preserve the old path with:page-aliases:and retain the explicit anchor.
docs/src/modules/ROOT/nav.adoc:3
- Antora section titles require a period followed by a space, so
.Getting startedis not parsed as a section heading and the new navigation grouping is lost; write. Getting startedinstead.
.Getting started
docs/src/modules/ROOT/nav.adoc:11
- Antora section titles require a period followed by a space, so
.Build with Timefoldis not parsed as a section heading and the new navigation grouping is lost; write. Build with Timefoldinstead.
.Build with Timefold
docs/src/modules/ROOT/nav.adoc:55
- Antora section titles require a period followed by a space, so
.Deploying to the Timefold Platformis not parsed as a section heading and the new navigation grouping is lost; write. Deploying to the Timefold Platforminstead.
.Deploying to the Timefold Platform
docs/src/modules/ROOT/nav.adoc:61
- Antora section titles require a period followed by a space, so
.Optimization algorithmsis not parsed as a section heading and the new navigation grouping is lost; write. Optimization algorithmsinstead.
.Optimization algorithms
docs/src/modules/ROOT/nav.adoc:70
- Antora section titles require a period followed by a space, so
.Upgrading Timefold Solveris not parsed as a section heading and the new navigation grouping is lost; write. Upgrading Timefold Solverinstead.
.Upgrading Timefold Solver
docs/src/modules/ROOT/nav.adoc:80
- Antora section titles require a period followed by a space, so
.Commercial editionsis not parsed as a section heading and the new navigation grouping is lost; write. Commercial editionsinstead.
.Commercial editions
docs/src/modules/ROOT/nav.adoc:94
- Antora section titles require a period followed by a space, so
.Additional resourcesis not parsed as a section heading and the new navigation grouping is lost; write. Additional resourcesinstead.
.Additional resources
docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-from-optaplanner.adoc:14
- Capitalize the programming language name as
Javahere, matching the proper-noun usage in the manual upgrade list below.
Run the command below to upgrade your java, build and other code automatically.
docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-from-optaplanner.adoc:69
- Use
insecurerather thanunsecurehere; the module is unsafe because of unresolved CVEs.
* The unsecure module `persistence-xstream` is removed, because of old, unresolved CVEs in XStream.
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved aggregate-document inclusion and backward-compatibility issues must be addressed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (8)
Previously missed (1) — in code that hasn't changed since the last review.
docs/src/modules/ROOT/pages/upgrading-timefold-solver/overview.adoc:18
- Deleting
backwards-compatibility.adocand moving its content here removes both the oldupgrading-timefold-solver/backwards-compatibilityURL and its explicit#backwardsCompatibilityfragment, so add a:page-aliases:entry for the old page and retain that anchor on this section.
docs/src/modules/ROOT/nav.adoc:70
- The new page is not included by
.optimization-algorithms.adoc, which is what the root guide includes, so removing this section from move-selector-reference causes Custom moves to disappear from the aggregate user guide and PDF; add aninclude::custom-moves.adoc[leveloffset=+1]there.
* xref:deploying-to-platform/guide.adoc[Guide]
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:87
- The snippet declares
MoveIteratorFactorywith one type parameter and rawMoveiterators, but the implementation below uses the two-parameter API, so the shown interface and example cannot compile together; mirror the actualMoveIteratorFactory<Solution_, Move_ extends Move<Solution_>>signatures.
public interface MoveIteratorFactory<Solution_> {
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:180
- The example uses
Randomas the overridden parameter, while the current API requiresRandomGenerator, so the@Overrideat line 178 will fail; useRandomGeneratorhere.
public Iterator<MyChangeMove> createRandomMoveIterator(ScoreDirector<MyPlanningSolution> scoreDirector,
Random workingRandom) {
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:118
- The comment is missing
isinif it too expensive; change it toif it is too expensive.
// if it too expensive to calculate or unknown
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:197
- The comment is missing
isinthere at least one value; change it tothere is at least one value.
// This assumes there at least one value that can be assigned to the selected entity
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:2
- This new page is not included by
optimization-algorithms/.optimization-algorithms.adoc, so the single-page user guide built from.index.adocomits the custom-move content even though the nav exposes it; add this page to the optimization aggregate.
== Custom moves
docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-from-optaplanner.adoc:69
- The adjective
unsecureis incorrect here; useinsecure.
* The unsecure module `persistence-xstream` is removed, because of old, unresolved CVEs in XStream.
- Files reviewed: 9/9 changed files
- Comments generated: 3
- Review effort level: Lite
| @@ -0,0 +1,241 @@ | |||
| [#customMovesIntroduction] | |||
| ---- | ||
| public interface MoveListFactory<Solution_> { | ||
|
|
||
| List<Move> createMoveList(Solution_ solution); |
There was a problem hiding this comment.
🟡 Changes recommended
Final review comments identify documentation accuracy, compatibility, aggregation, and navigation issues that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (7)
Previously missed (2) — in code that hasn't changed since the last review.
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:95
- This example shows the obsolete one-parameter/raw
MoveIteratorFactorysignature instead of the currentMoveIteratorFactory<Solution_, Move_ extends Move<Solution_>>andIterator<Move_>contract, so the sample is not an accurate implementation template.
This issue also appears on line 179 of the same file.
docs/src/modules/ROOT/pages/upgrading-timefold-solver/overview.adoc:18
- Deleting
upgrading-timefold-solver/backwards-compatibility.adocwithout aliasing it from this replacement page breaks existing links to the published page; add the deleted path to:page-aliases:.
docs/src/modules/ROOT/nav.adoc:64
- Adding this page only to
nav.adocleaves it out of thepages/.index.adoc→optimization-algorithms/.optimization-algorithms.adocsingle-page aggregation, so the root user guide will omit the entire Custom moves section; add an include forcustom-moves.adocto that hidden aggregator as well.
*** xref:optimization-algorithms/custom-moves.adoc[Move definition]
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:38
- This snippet no longer matches
MoveListFactory.createMoveList, whose current return type isList<? extends Move<Solution_>>; as written it teaches a raw and inaccurate declaration.
public interface MoveListFactory<Solution_> {
List<Move> createMoveList(Solution_ solution);
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:180
- The sample implementation still accepts
Random, but the currentMoveIteratorFactorycontract requiresRandomGenerator, so this@Overridecannot compile; change the parameter type.
public Iterator<MyChangeMove> createRandomMoveIterator(ScoreDirector<MyPlanningSolution> scoreDirector,
Random workingRandom) {
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:1
- Moving this section out of
move-selector-reference.adocremoves the previously published#customMovesIntroduction,#generatingCustomMoves,#moveListFactory, and#moveIteratorFactorytargets, so existing deep links will 404; retain compatibility anchors or redirects in the old page.
[#customMovesIntroduction]
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:24
- Because this file is now a standalone page with a level-one document title,
=== Generating custom movesskips the required level-two section and leaves its====children at the wrong depth, so change this section to==and the two factory headings to===.
=== Generating custom moves
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
| @@ -0,0 +1,241 @@ | |||
| [#customMovesIntroduction] | |||
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved navigation, compatibility-link, aggregator, and API-example issues remain.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (10)
Previously missed (2) — in code that hasn't changed since the last review.
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:93
- These iterator signatures use raw
Iterator<Move>andRandom, but the current interface has aMove_ extends Move<Solution_>type parameter and requiresIterator<Move_>plusRandomGenerator, so this interface example is no longer accurate.
This issue also appears on line 179 of the same file.
docs/src/modules/ROOT/pages/upgrading-timefold-solver/overview.adoc:18
- Moving the contents out of
backwards-compatibility.adocremoves its public URL and the#backwardsCompatibilityanchor, so existing links will no longer resolve; add a page alias in this document's header and preserve the old anchor.
docs/src/modules/ROOT/nav.adoc:1
- The old navigation entry for
introduction.adocis removed without a replacement, so the introductory page is no longer discoverable from the sidebar; retain it under the new Getting started section.
.Getting started
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:1
- This new page is not included by the
optimization-algorithms/.optimization-algorithms.adocsingle-page aggregator, so it will appear in the navigation but be missing from single-page builds; add it to that aggregator.
[#customMovesIntroduction]
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:38
- This example uses the obsolete raw
List<Move>return type, whereas the currentMoveListFactory<Solution_>contract requiresList<? extends Move<Solution_>>, so readers cannot use the snippet as the current interface signature.
List<Move> createMoveList(Solution_ solution);
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:180
- The sample implementation still takes
Random, so it cannot override the currentMoveIteratorFactorymethod that takesRandomGenerator; useRandomGeneratorhere.
public Iterator<MyChangeMove> createRandomMoveIterator(ScoreDirector<MyPlanningSolution> scoreDirector,
Random workingRandom) {
docs/src/modules/ROOT/pages/optimization-algorithms/custom-moves.adoc:1
- Moving these sections out of
move-selector-reference.adocremoves the existingcustomMovesIntroduction,generatingCustomMoves,moveListFactory, andmoveIteratorFactoryanchors, so existing deep links no longer navigate to their intended content; retain compatibility anchors/stubs or provide redirects to the new page.
[#customMovesIntroduction]
docs/src/modules/ROOT/pages/upgrading-timefold-solver/overview.adoc:18
- Deleting
upgrading-timefold-solver/backwards-compatibility.adocremoves its old URL, so existing bookmarks and external links will 404; add:page-aliases: upgrading-timefold-solver/backwards-compatibility.adocto this page's document header.
== Backwards compatibility
docs/src/modules/ROOT/pages/upgrading-timefold-solver/overview.adoc:18
- The deleted page also declared
[#backwardsCompatibility], but this moved heading does not, so links targeting#backwardsCompatibilitylose their anchor; retain that explicit anchor above the heading.
== Backwards compatibility
docs/src/modules/ROOT/pages/upgrading-timefold-solver/upgrade-from-optaplanner.adoc:56
- The page sends readers to the 2.x upgrade recipe at line 17, but this manual recipe says the target is Timefold Solver 1.x and lists Java 17 as the minimum, so current OptaPlanner users can follow the wrong compatibility requirements; update it for 2.x or explicitly separate the 1.x and 2.x paths.
Timefold Solver 1.x is backward compatible with OptaPlanner 8.x,
except for the following changes:
* Java 17 is the minimum, and Java 21 and 25 are also supported.
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
| * Moves | ||
| ** xref:optimization-algorithms/move-selector-reference.adoc[leveloffset=+1] | ||
| ** Custom Moves | ||
| *** xref:optimization-algorithms/custom-moves.adoc[Move definition] |
A new nav structure with subheadings has been implemented.
This adjusts the nav.adoc file to make use of that.