From 73cccc3e9264a3086e0844128afed82a226ad3dc Mon Sep 17 00:00:00 2001 From: Olivier Cots Date: Wed, 2 Sep 2026 12:32:59 +0200 Subject: [PATCH] fix(Descriptions): keep `Available` exhaustive, let the hint carry its list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #553. Its fix routed the similarity-filtered matches into `candidates`, which `Exceptions/display.jl:158` labels `Available` -- so the two message fields effectively swapped content: a 12-entry catalog was reported as holding 5 descriptions, with nothing saying the list had been filtered, while `Hint Try one of the closest matches:` still ended on a colon with nothing after it. Two lines put each field back on its own job: - `candidates` is always the catalog again -- every entry, or the first `max_show` followed by the `… and N more` marker; - the closest matches are joined into `suggestion`, which the renderer already splits on newlines (`_print_pipe_field`), so `display.jl` needs no change. This also makes #553's marker reachable. It is built from `all_candidates`, which was discarded whenever `similar_descs` was non-empty, and `_find_similar_descriptions` returns empty only when the request shares no symbol with any description (Jaccard, filtered on > 0). The marker could therefore only ever render for a catalog above `max_show` *and* a wholly unknown request -- never in the case #553 was filed about. Tests: new non-regression testset for #557 covering the branch #553 could not reach (catalog > max_show *with* similar matches), the exhaustive small-catalog path, a self-contained hint, and an end-to-end check that the rendered message shows the matches after the `Hint` label. The #553 testset asserted the behaviour this commit fixes (`candidates` equal to the closest matches) and was corrected. Verified failing without the source change. Also refreshed the `complete` docstring, whose example still showed a message format several releases old. Closes #557. Co-Authored-By: Claude Opus 5 --- BREAKING.md | 22 +++++ CHANGELOG.md | 42 ++++++++++ Project.toml | 2 +- src/Descriptions/complete.jl | 51 ++++++++---- test/suite/descriptions/test_complete.jl | 100 +++++++++++++++++++++-- 5 files changed, 194 insertions(+), 23 deletions(-) diff --git a/BREAKING.md b/BREAKING.md index 855cde76..afc03adf 100644 --- a/BREAKING.md +++ b/BREAKING.md @@ -3,6 +3,28 @@ This document outlines all breaking changes introduced in CTBase v0.18.0-beta compared to v0.17.4. Use this guide to migrate your code and understand the impact of these changes. +## Non-breaking note (0.30.4-beta) + +- **Descriptions — `AmbiguousDescription` message fields**: `candidates` again + always carries the catalog (exhaustive, or truncated with a `… and N more` + marker), and the `"Try one of the closest matches:"` hint lists those matches + itself instead of ending on a colon. Fixes + [#557](https://github.com/control-toolbox/CTBase.jl/issues/557), a follow-up to + [#553](https://github.com/control-toolbox/CTBase.jl/issues/553). **No breaking + change**: error-message content only; `complete`'s resolution behaviour, its + signatures and return values, and the `AmbiguousDescription` field names and + types are all unchanged. The one observable difference for a caller inspecting + the exception is that `suggestion` is now a multi-line string on the + closest-matches path, and `candidates` is the catalog rather than a filtered + subset — which is what it was before 0.30.3-beta. No migration required. + +## Non-breaking note (0.30.3-beta) + +- **Descriptions — truncation marker and closest-matches hint**: raised the + shown-candidate ceiling from 10 to 20 and added a `… and N more` marker. **No + breaking change**: display only. See the 0.30.4-beta note above for the + correction to how the two fields are filled. + ## Non-breaking note (0.30.2-beta) - **Strategies — `parameter` documentation anchors**: split the docstrings for diff --git a/CHANGELOG.md b/CHANGELOG.md index 7465c5e3..003100cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,48 @@ All notable changes to CTBase will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.30.4-beta] - unreleased + +### 🐛 Bug Fixes + +**`Descriptions.complete` — `Available` stays exhaustive, the hint carries its own list** +([#557](https://github.com/control-toolbox/CTBase.jl/issues/557), follow-up to +[#553](https://github.com/control-toolbox/CTBase.jl/issues/553)). + +- `candidates` is again always the **catalog** — every entry, or the first `max_show` + followed by the `… and N more` marker. 0.30.3-beta routed the similarity-filtered + matches into that field, which the display layer labels `Available`, so a 12-entry + catalog was reported as holding 5 descriptions with nothing saying so. +- The `"Try one of the closest matches:"` hint now **lists those matches** on its own + lines instead of ending on a colon. The exception renderer already supported + multi-line values, so no display change was needed. +- Side effect worth noting: the truncation marker added in 0.30.3-beta was until now + unreachable in practice. It is produced from `all_candidates`, which was discarded + whenever similar descriptions existed — and `_find_similar_descriptions` returns + empty only when the request shares no symbol with any description. The marker could + therefore only appear for a catalog above `max_show` *and* a wholly unknown request. + It now renders on every path. + +Also updated the `complete` docstring, whose example still showed a message format +several releases old. + +### 🧪 Tests + +- New non-regression testset for #557: `candidates` exhaustive on the small-catalog + path, marker present on the branch 0.30.3-beta could not reach (catalog > `max_show` + *with* similar matches), hint self-contained, and an end-to-end check that the + rendered message shows the matches after the `Hint` label. +- Corrected the #553 testset, which asserted the behaviour this release fixes + (`candidates` equal to the closest matches). + +### ✅ Compatibility + +- **No breaking changes**: error-message content only. `complete`'s resolution + behaviour, signatures, return values and the `AmbiguousDescription` fields are + unchanged. Code that reads `e.candidates` expecting the catalog gets the catalog + again; code that reads `e.suggestion` gets a multi-line string where it previously + got one line. No migration required. + ## [0.30.3-beta] - unreleased ### 🐛 Bug Fixes diff --git a/Project.toml b/Project.toml index 1c4bb3e5..16ea86b7 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "CTBase" uuid = "54762871-cc72-4466-b8e8-f6c8b58076cd" -version = "0.30.3-beta" +version = "0.30.4-beta" authors = ["Olivier Cots ", "Jean-Baptiste Caillau "] [deps] diff --git a/src/Descriptions/complete.jl b/src/Descriptions/complete.jl index c31ae095..28056c72 100644 --- a/src/Descriptions/complete.jl +++ b/src/Descriptions/complete.jl @@ -23,28 +23,41 @@ the first one in the catalog wins (priority is top-to-bottom). ```julia-repl julia> using CTBase -julia> D = ((:a, :b), (:a, :b, :c), (:b, :c), (:a, :c)) -(:a, :b) -(:b, :c) -(:a, :c) +julia> D = ((:a, :b), (:a, :b, :c), (:b, :c), (:a, :c)); + julia> CTBase.complete(:a; descriptions=D) (:a, :b) + julia> CTBase.complete(:a, :c; descriptions=D) (:a, :b, :c) + julia> CTBase.complete((:a, :c); descriptions=D) (:a, :b, :c) -julia> CTBase.complete(:f; descriptions=D) -ERROR: AmbiguousDescription: the description (:f,) is ambiguous / incorrect - Description: (:f,) - Valid candidates: - - (:a, :b) - - (:a, :b, :c) - - (:b, :c) - - (:a, :c) - Suggestion: Available descriptions: (:a, :b), (:a, :b, :c), (:b, :c), (:a, :c) - Context: description completion + +julia> CTBase.complete(:a, :z; descriptions=D) +ERROR: AmbiguousDescription → #complete#15, complete.jl:97 +│ +│ cannot find matching description +│ +│ Diagnostic No complete match — no description contains all symbols +│ Requested (:a, :z) +│ Available (:a, :b) +│ (:a, :b, :c) +│ (:b, :c) +│ (:a, :c) +│ +│ Context description completion +│ Hint Try one of the closest matches: +│ (:a, :c) +│ (:a, :b) +│ (:a, :b, :c) +└─ ``` +`Available` always describes the catalog — every entry, or the first `max_show` +followed by a `… and N more` marker. The closest matches, when any description +shares a symbol with the request, are listed by the `Hint` itself. + See also: [`CTBase.Descriptions._compute_similarity`](@extref), [`CTBase.Descriptions._find_similar_descriptions`](@extref), [`CTBase.Descriptions._format_description_candidates`](@extref), [`CTBase.Exceptions.AmbiguousDescription`](@extref) """ function complete(list::Symbol...; descriptions::Tuple{Vararg{Description}})::Description @@ -73,9 +86,13 @@ function complete(list::Symbol...; descriptions::Tuple{Vararg{Description}})::De similar_descs = _find_similar_descriptions(list, descriptions; max_results=5) all_candidates = _format_description_candidates(descriptions; max_show=20) - # Build contextual suggestion + # Build contextual suggestion. The closest matches are carried *by the + # hint itself*, not by `candidates`: the display layer labels + # `candidates` "Available", so putting a similarity-filtered subset + # there would claim the catalog is smaller than it is, and leave the + # hint announcing a list it never prints (issue #557). suggestion = if !isempty(similar_descs) - "Try one of the closest matches:" + string("Try one of the closest matches:\n", join(similar_descs, "\n")) elseif !isempty(all_candidates) "Choose from the available descriptions listed above" else @@ -93,7 +110,7 @@ function complete(list::Symbol...; descriptions::Tuple{Vararg{Description}})::De throw( Exceptions.AmbiguousDescription( list; - candidates=isempty(similar_descs) ? all_candidates : similar_descs, + candidates=all_candidates, suggestion=suggestion, context="description completion", diagnostic=diagnostic, diff --git a/test/suite/descriptions/test_complete.jl b/test/suite/descriptions/test_complete.jl index fcf06018..81cb0c8e 100644 --- a/test/suite/descriptions/test_complete.jl +++ b/test/suite/descriptions/test_complete.jl @@ -207,8 +207,10 @@ function test_complete() Test.@test e isa Exceptions.AmbiguousDescription Test.@test !isempty(e.candidates) Test.@test occursin("closest matches", e.suggestion) - # Should suggest descriptions containing :b (which is (:a, :b, :c)) - Test.@test any(occursin("(:a,", candidate) for candidate in e.candidates) + # The similar descriptions are listed by the hint itself; the + # candidate list stays the full catalog (issue #557). + Test.@test occursin("(:a, :b, :c)", e.suggestion) + Test.@test length(e.candidates) == 3 end end @@ -250,8 +252,9 @@ function test_complete() Test.@test length(err22.candidates) == 21 # 20 shown + marker Test.@test err22.candidates[end] == "… and 2 more" - # 2. Closest matches are not discarded: candidates == the nearest - # descriptions once similar_descs is non-empty. + # 2. Closest matches are not discarded. They are carried by the + # *hint*, not by `candidates` -- see the issue 557 testset below + # for why that distinction matters. err = try Descriptions.complete(:adnlp, :gpu; descriptions=descs) catch e @@ -259,8 +262,95 @@ function test_complete() end Test.@test err isa Exceptions.AmbiguousDescription Test.@test occursin("closest matches", err.suggestion) + Test.@test occursin("(:gpu, :exact)", err.suggestion) + Test.@test occursin("(:gpu, :krylov)", err.suggestion) Test.@test !isempty(err.candidates) - Test.@test Set(err.candidates) == Set(["(:gpu, :exact)", "(:gpu, :krylov)"]) + end + + Test.@testset "issue 557 - Available stays exhaustive, hint carries its list" begin + # ================================================================ + # NON-REGRESSION: issue #557, follow-up to #553. + # + # #553's fix put the similarity-filtered matches into `candidates`. + # The display layer labels that field "Available", so the message + # claimed a 12-entry catalog held 5 descriptions, while the hint + # ("Try one of the closest matches:") printed nothing after its + # colon. Two invariants, one per field: + # + # 1) `candidates` describes the CATALOG -- exhaustive, or truncated + # with a marker -- on every path, similar matches or not. The + # marker added by #553 was unreachable in practice: it lived on + # the branch taken only when nothing resembles the request. + # 2) `suggestion` carries the closest matches itself, so the hint + # is readable on its own line block. + # ================================================================ + + # --- 1. The branch #553 could not reach: > max_show AND a request + # that DOES resemble the catalog, so similar_descs != []. + big = () + for i in 1:22 + big = Descriptions.add(big, (:ipopt, Symbol(:cpu_, i))) + end + err_big = try + Descriptions.complete(:ipopt, :nowhere; descriptions=big) + catch e + e + end + Test.@test err_big isa Exceptions.AmbiguousDescription + # similar matches exist (every entry shares :ipopt) ... + Test.@test occursin("closest matches", err_big.suggestion) + # ... and `candidates` is still the catalog, marker included. + Test.@test length(err_big.candidates) == 21 # 20 shown + marker + Test.@test err_big.candidates[end] == "… and 2 more" + + # --- 2. Small catalog: `candidates` is exhaustive, not filtered. + descs = () + for i in 1:10 + descs = Descriptions.add(descs, (:ipopt, Symbol(:cpu_, i))) + end + descs = Descriptions.add(descs, (:gpu, :exact)) + descs = Descriptions.add(descs, (:gpu, :krylov)) + + err = try + Descriptions.complete(:adnlp, :gpu; descriptions=descs) + catch e + e + end + Test.@test err isa Exceptions.AmbiguousDescription + Test.@test length(err.candidates) == 12 + Test.@test "(:gpu, :exact)" in err.candidates + # the entries that do NOT resemble the request are listed too -- + # that is what "Available" means + Test.@test "(:ipopt, :cpu_1)" in err.candidates + Test.@test !any(occursin("more", c) for c in err.candidates) + + # --- 3. The hint is self-contained: header line, then one line per + # closest match. + lines = split(err.suggestion, '\n') + Test.@test length(lines) > 1 + Test.@test lines[1] == "Try one of the closest matches:" + Test.@test Set(lines[2:end]) == Set(["(:gpu, :exact)", "(:gpu, :krylov)"]) + + # --- 4. End to end: the rendered message shows them under Hint, + # not only in the struct. + msg = sprint(showerror, err) + hint_at = findfirst("Hint", msg) + Test.@test !isnothing(hint_at) + tail = msg[last(hint_at):end] + Test.@test occursin("(:gpu, :exact)", tail) + Test.@test occursin("(:gpu, :krylov)", tail) + + # --- 5. The no-similarity path is untouched: nothing resembles the + # request, so the hint stays a single sentence. + err_none = try + Descriptions.complete(:zzz; descriptions=descs) + catch e + e + end + Test.@test err_none isa Exceptions.AmbiguousDescription + Test.@test !occursin("closest matches", err_none.suggestion) + Test.@test !occursin('\n', err_none.suggestion) + Test.@test length(err_none.candidates) == 12 end Test.@testset "Diagnostic field verification" begin