Repository navigation
fix: let a folder turn explanation off, or on with its own provider - #256
Merged
Merged
Conversation
A folder's settings did not decide whether its jobs were explained. Turning explanation off for a folder was not possible. The box at the top of the folder's section, "Enable AI Error Explanation", only opens the section: left unticked, which it is by default, the folder sets nothing and its jobs follow the global switch. So did a folder stored with it unticked, which every folder saved before 216 is. Builds in such a folder were still sent to the provider whenever the global switch was on. A folder with a provider of its own, on the other hand, had no Explain Error button while the global switch was off, and the console page's endpoint refused with "disabled in global configuration". Only the explainError() step and automatic explanation used the folder's provider. Now: - The folder section has "Turn off AI error explanation for jobs in this folder" (disableExplanation). With it, there is no button, no automatic explanation, and explainError() does nothing. Explanations builds already have can still be read. - The box that opens the section is called "Set AI error explanation for this folder", which is what it does. Its stored meaning is unchanged, so no folder changes behaviour on upgrade. - The nearest folder that decides wins: one that turns explanation off says no, one with a provider of its own says yes. Otherwise the global switch applies. - The console and Stages pages, and their endpoints, ask the same question for the build in front of them, so a folder with its own provider gets the button while the global switch is off, and the provider name shown is the folder's. - Looking up the folder provider now passes over a folder whose section is unused, like the enabled check already did. Before, such a folder with a provider stored by an older form stopped the search and the global provider was used even under a parent folder with its own.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A folder's settings did not decide whether its jobs are explained.
explainError()and automatic explanation in the same folder used the folder's provider.Now:
explainError()does nothing. Explanations builds already have can still be read.Before and after
The folder's section
A failed build in that folder, global switch on
A failed build in a folder with its own provider, global switch off
explainError()in the same folder works.mobile-team-model, is in the footer).The screenshots use a local stand-in provider with canned answers.
Things a reviewer should know
enableExplanationkeeps its stored meaning: "the section is in use". Every folder whose Configure page was saved before 216 has it stored asfalse, so giving that value the meaning "off" would have switched those folders off. Turning off is a new field,disableExplanation.ExplainErrorFolderProperty.findFolderDecision.ExplainErrorFolderProperty.isFolderExplanationEnabledis unused and kept as it was.Related Issues
None; found while reviewing what to fix next.
Testing done
FolderExplanationSwitchTest, 10 new tests: a folder turning explanation off while it is on globally (explainer, console page and endpoint,explainError(), automatic explanation); a folder with its own provider while it is off globally (console page and endpoint, Graph View endpoint); the nearest folder deciding in both directions; folders stored before keeping the global switch; an unused section passing the parent's provider on; the switch saved through the folder form.ExplainErrorFolderPropertyTest,ErrorExplainerTest,ConsolePageDecoratorTest,ConsoleExplainErrorActionTest,GraphViewExplainErrorActionTest,PipelineGraphViewDecoratorTest,AutoExplainJobOverrideTest,AutoExplainLimitsTest,ErrorRunListenerTest,ExplainCardStatesTest,ConsoleToolbarTest,CasCTest,ConfigMigrationTest,ExplainErrorStepTest,ExplanationReuseTest,UsageTrackingTest,QuotaEnforcerTest.mvn verifywas not run locally.By hand, on a local Jenkins 2.528.3 with a stand-in provider that counts its requests, the same folders before and after:
main)Submitter checklist
Was generative AI tooling used to co-author this PR?
Assisted-by: Claude Code (claude-opus-5-5)