8000 Add suggestions when features are incorrectly specified by mabartos · Pull Request #48963 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Add suggestions when features are incorrectly specified - #48963

Merged
vmuzikar merged 2 commits into
keycloak:mainfrom
mabartos:KC-48962
May 18, 2026
Merged

Add suggestions when features are incorrectly specified#48963
vmuzikar merged 2 commits into
keycloak:mainfrom
mabartos:KC-48962

Conversation

@mabartos
@mabartos mabartos commented May 13, 2026 •
edited
Loading
Copy link
Copy Markdown
Member

OLD

Screenshot From 2026-05-13 14-39-39

NEW

Screenshot From 2026-05-13 14-41-33

@mabartos
mabartos force-pushed the KC-48962 branch 2 times, most recently from 1b194c6 to 2413f1c Compare May 13, 2026 13:28
@mabartos mabartos self-assigned this May 13, 2026
@mabartos
mabartos marked this pull request as ready for review May 13, 2026 16:43
@mabartos
mabartos requested review from a team as code owners May 13, 2026 16:43
@mabartos
mabartos commented May 13, 2026
Copy link
Copy Markdown
Member Author

@shawkins @Pepo48 Could you please check it? Thanks!

@mabartos
mabartos requested review from Pepo48 and shawkins May 13, 2026 19:06
@shawkins
shawkins commented May 13, 2026
Copy link
Copy Markdown
Contributor

I think the changes make sense, but not sure why we'll have two different behaviors for unknown options or unknown features. With options we show the top matches regardless of similarity score, here you'll show the top matches only if they meet a threshold, otherwise you'll show everything.

I think the reasoning for the former is that the user can do a help command or search the docs if they don't see what their looking for - is there something different about features such that it's more helpful to show all of them?

Pepo48
Pepo48 requested changes May 13, 2026
View reviewed changes
@Pepo48 Pepo48 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.

@mabartos thanks for the PR. I added a couple of comments below.

Comment thread quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/SimilarityUtil.java Resolved
Comment thread quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/SimilarityUtil.java Resolved
Comment thread quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/SimilarityUtil.java Resolved
Comment thread quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java Resolved
Comment thread quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java Resolved
Comment thread quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/SimilarityUtil.java Resolved
@mabartos
mabartos commented May 14, 2026
Copy link
Copy Markdown
Member Author

I think the changes make sense, but not sure why we'll have two different behaviors for unknown options or unknown features. With options we show the top matches regardless of similarity score, here you'll show the top matches only if they meet a threshold, otherwise you'll show everything.
I think the reasoning for the former is that the user can do a help command or search the docs if they don't see what their looking for - is there something different about features such that it's more helpful to show all of them?

@shawkins @Pepo48 Thanks for the review!

First of all, I wanted to have behavior changes rather low for outputs - it means that if users specify feature incorrectly, the full list is shown, and they can see the possible features (some people can be used to that 😛 ). I find showing the full list of features as the worst case for the output, but IMHO it's still better than hiding it, and pointing to the docs, or running some "big help" command (if we have sth like "./kc.sh help features", I'd be ok with that).

That's the difference for options and features - right now, we have ~50 features that could be listed, so it's not VERY bad, so we can still show them in the output.
In case:

  • A) we consider having a threshold and fallback of listing all options, we naturally cannot list them all as there's >140 options + disabled ones.
  • B) we consider having the same current behavior as for options (no threshold) - the output for the limited count of features would be very strange. Features names are usually much shorter than the option names, and the cosine similarity does not work as nice as for options -> without threshold, there would be some misleading suggestions as:
    • "foobar" -> "par"
    • "dokcer" -> "kerberos"
    • "no-feature" -> "ipa-tuura-federation", "resource-indicators", "client-auth-federated"
      As they're sharing some bigrams.

Considering the case B), I'd still rather list all features, than suggest some nonsense for features 😁
@shawkins @Pepo48 But I'm leaving the decision up to you, as you're the code owners of that.

mabartos and others added 2 commits May 14, 2026 08:59
@mabartos
Add suggestions when features are incorrect 8000 ly specified
ca100e2 
Closes keycloak#48962

Signed-off-by: Martin Bartoš <mabartos@redhat.com>
@mabartos @Pepo48
Address review comments
5d7c842 
Co-authored-by: Peter Zaoral <pepo48@gmail.com>
Signed-off-by: Martin Bartoš <mabartos@redhat.com>
@keycloak-github-bot keycloak-github-bot Bot mentioned this pull request May 14, 2026
Closed
shawkins
shawkins approved these changes May 15, 2026
View reviewed changes
@shawkins shawkins 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.

LGTM. In general what we seem to be needing is #41003 - I've updated that to reference features as well.

@mabartos
mabartos requested a review from Pepo48 May 15, 2026 11:59
Pepo48
Pepo48 approved these changes May 15, 2026
View reviewed changes
@Pepo48 Pepo48 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.

I assume the fact that the suggestions mechanism only goes through "disableable" features, meaning the essential features (e.g. hostname) will never be suggested, is deliberate design choice, right?

If so, then LGTM. Thanks, Martin!

@vmuzikar
vmuzikar merged commit a4d6c21 into keycloak:main May 18, 2026
90 checks passed
@stianst stianst mentioned this pull request Jun 1, 2026
Closed
sashyo pushed a commit to tide-foundation/keycloak-IGA that referenced this pull request Jul 20, 2026
@mabartos @Pepo48 @sashyo
Add suggestions when features are incorrectly specified (keycloak#48963)
3fb678e 
* Add suggestions when features are incorrectly specified

Closes keycloak#48962

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

* Address review comments

Co-authored-by: Peter Zaoral <pepo48@gmail.com>
Signed-off-by: Martin Bartoš <mabartos@redhat.com>

---------

Signed-off-by: Martin Bartoš <mabartos@redhat.com>
Co-authored-by: Peter Zaoral <pepo48@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shawkins shawkins shawkins approved these changes
@Pepo48 Pepo48 Pepo48 approved these changes

Assignees

@mabartos mabartos

Labels

flaky-test team/cloud-native

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add suggestions when features are incorrectly specified

4 participants

@mabartos @shawkins @Pepo48 @vmuzikar
0