Skip to content

Commit e803d3b

Browse files
authored
Update the ICommandPredictor to provide more feedback and also make feedback easier to be corelated (PowerShell#14649)
1 parent 61fea6c commit e803d3b

6 files changed

Lines changed: 214 additions & 61 deletions

File tree

src/System.Management.Automation/engine/Subsystem/CommandPrediction/CommandPrediction.cs

Lines changed: 56 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,22 @@ public sealed class PredictionResult
2727
/// </summary>
2828
public string Name { get; }
2929

30+
/// <summary>
31+
/// Gets the mini-session id that represents a specific invocation to the <see cref="ICommandPredictor.GetSuggestion"/> API of the predictor.
32+
/// When it's not specified, it's considered by a client that the predictor doesn't expect feedback.
33+
/// </summary>
34+
public uint? Session { get; }
35+
3036
/// <summary>
3137
/// Gets the suggestions.
3238
/// </summary>
3339
public IReadOnlyList<PredictiveSuggestion> Suggestions { get; }
3440

35-
internal PredictionResult(Guid id, string name, List<PredictiveSuggestion> suggestions)
41+
internal PredictionResult(Guid id, string name, uint? session, List<PredictiveSuggestion> suggestions)
3642
{
3743
Id = id;
3844
Name = name;
45+
Session = session;
3946
Suggestions = suggestions;
4047
}
4148
}
@@ -48,22 +55,24 @@ public static class CommandPrediction
4855
/// <summary>
4956
/// Collect the predictive suggestions from registered predictors using the default timeout.
5057
/// </summary>
58+
/// <param name="client">Represents the client that initiates the call.</param>
5159
/// <param name="ast">The <see cref="Ast"/> object from parsing the current command line input.</param>
5260
/// <param name="astTokens">The <see cref="Token"/> objects from parsing the current command line input.</param>
5361
/// <returns>A list of <see cref="PredictionResult"/> objects.</returns>
54-
public static Task<List<PredictionResult>?> PredictInput(Ast ast, Token[] astTokens)
62+
public static Task<List<PredictionResult>?> PredictInput(string client, Ast ast, Token[] astTokens)
5563
{
56-
return PredictInput(ast, astTokens, millisecondsTimeout: 20);
64+
return PredictInput(client, ast, astTokens, millisecondsTimeout: 20);
5765
}
5866

5967
/// <summary>
6068
/// Collect the predictive suggestions from registered predictors using the specified timeout.
6169
/// </summary>
70+
/// <param name="client">Represents the client that initiates the call.</param>
6271
/// <param name="ast">The <see cref="Ast"/> object from parsing the current command line input.</param>
6372
/// <param name="astTokens">The <see cref="Token"/> objects from parsing the current command line input.</param>
6473
/// <param name="millisecondsTimeout">The milliseconds to timeout.</param>
6574
/// <returns>A list of <see cref="PredictionResult"/> objects.</returns>
66-
public static async Task<List<PredictionResult>?> PredictInput(Ast ast, Token[] astTokens, int millisecondsTimeout)
75+
public static async Task<List<PredictionResult>?> PredictInput(string client, Ast ast, Token[] astTokens, int millisecondsTimeout)
6776
{
6877
Requires.Condition(millisecondsTimeout > 0, nameof(millisecondsTimeout));
6978

@@ -85,8 +94,8 @@ public static class CommandPrediction
8594
state =>
8695
{
8796
var predictor = (ICommandPredictor)state!;
88-
List<PredictiveSuggestion>? texts = predictor.GetSuggestion(context, cancellationSource.Token);
89-
return texts?.Count > 0 ? new PredictionResult(predictor.Id, predictor.Name, texts) : null;
97+
SuggestionPackage pkg = predictor.GetSuggestion(client, context, cancellationSource.Token);
98+
return pkg.SuggestionEntries?.Count > 0 ? new PredictionResult(predictor.Id, predictor.Name, pkg.Session, pkg.SuggestionEntries) : null;
9099
},
91100
predictor,
92101
cancellationSource.Token,
@@ -99,27 +108,28 @@ await Task.WhenAny(
99108
Task.Delay(millisecondsTimeout, cancellationSource.Token)).ConfigureAwait(false);
100109
cancellationSource.Cancel();
101110

102-
var results = new List<PredictionResult>(predictors.Count);
111+
var resultList = new List<PredictionResult>(predictors.Count);
103112
foreach (Task<PredictionResult?> task in tasks)
104113
{
105114
if (task.IsCompletedSuccessfully)
106115
{
107116
PredictionResult? result = task.Result;
108117
if (result != null)
109118
{
110-
results.Add(result);
119+
resultList.Add(result);
111120
}
112121
}
113122
}
114123

115-
return results;
124+
return resultList;
116125
}
117126

118127
/// <summary>
119128
/// Allow registered predictors to do early processing when a command line is accepted.
120129
/// </summary>
130+
/// <param name="client">Represents the client that initiates the call.</param>
121131
/// <param name="history">History command lines provided as references for prediction.</param>
122-
public static void OnCommandLineAccepted(IReadOnlyList<string> history)
132+
public static void OnCommandLineAccepted(string client, IReadOnlyList<string> history)
123133
{
124134
Requires.NotNull(history, nameof(history));
125135

@@ -134,19 +144,51 @@ public static void OnCommandLineAccepted(IReadOnlyList<string> history)
134144
if (predictor.SupportEarlyProcessing)
135145
{
136146
ThreadPool.QueueUserWorkItem<ICommandPredictor>(
137-
state => state.StartEarlyProcessing(history),
147+
state => state.StartEarlyProcessing(client, history),
148+
predictor,
149+
preferLocal: false);
150+
}
151+
}
152+
}
153+
154+
/// <summary>
155+
/// Send feedback to a predictor when one or more suggestions from it were displayed to the user.
156+
/// </summary>
157+
/// <param name="client">Represents the client that initiates the call.</param>
158+
/// <param name="predictorId">The identifier of the predictor whose prediction result was accepted.</param>
159+
/// <param name="session">The mini-session where the displayed suggestions came from.</param>
160+
/// <param name="countOrIndex">
161+
/// When the value is greater than 0, it's the number of displayed suggestions from the list returned in <paramref name="session"/>, starting from the index 0.
162+
/// When the value is less than or equal to 0, it means a single suggestion from the list got displayed, and the index is the absolute value.
163+
/// </param>
164+
public static void OnSuggestionDisplayed(string client, Guid predictorId, uint session, int countOrIndex)
165+
{
166+
var predictors = SubsystemManager.GetSubsystems<ICommandPredictor>();
167+
if (predictors.Count == 0)
168+
{
169+
return;
170+
}
171+
172+
foreach (ICommandPredictor predictor in predictors)
173+
{
174+
if (predictor.AcceptFeedback && predictor.Id == predictorId)
175+
{
176+
ThreadPool.QueueUserWorkItem<ICommandPredictor>(
177+
state => state.OnSuggestionDisplayed(client, session, countOrIndex),
138178
predictor,
139179
preferLocal: false);
140180
}
141181
}
142182
}
143183

144184
/// <summary>
145-
/// Send feedback to predictors about their last suggestions.
185+
/// Send feedback to a predictor when a suggestion from it was accepted.
146186
/// </summary>
187+
/// <param name="client">Represents the client that initiates the call.</param>
147188
/// <param name="predictorId">The identifier of the predictor whose prediction result was accepted.</param>
189+
/// <param name="session">The mini-session where the accepted suggestion came from.</param>
148190
/// <param name="suggestionText">The accepted suggestion text.</param>
149-
public static void OnSuggestionAccepted(Guid predictorId, string suggestionText)
191+
public static void OnSuggestionAccepted(string client, Guid predictorId, uint session, string suggestionText)
150192
{
151193
Requires.NotNullOrEmpty(suggestionText, nameof(suggestionText));
152194

@@ -161,7 +203,7 @@ public static void OnSuggestionAccepted(Guid predictorId, string suggestionText)
161203
if (predictor.AcceptFeedback && predictor.Id == predictorId)
162204
{
163205
ThreadPool.QueueUserWorkItem<ICommandPredictor>(
164-
state => state.OnSuggestionAccepted(suggestionText),
206+
state => state.OnSuggestionAccepted(client, session, suggestionText),
165207
predictor,
166208
preferLocal: false);
167209
}

src/System.Management.Automation/engine/Subsystem/CommandPrediction/ICommandPredictor.cs

Lines changed: 67 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,22 +40,37 @@ public interface ICommandPredictor : ISubsystem
4040
/// A command line was accepted to execute.
4141
/// The predictor can start processing early as needed with the latest history.
4242
/// </summary>
43+
/// <param name="clientId">Represents the client that initiates the call.</param>
4344
/// <param name="history">History command lines provided as references for prediction.</param>
44-
void StartEarlyProcessing(IReadOnlyList<string> history);
45+
void StartEarlyProcessing(string clientId, IReadOnlyList<string> history);
4546

4647
/// <summary>
47-
/// The suggestion given by the predictor was accepted.
48+
/// Get the predictive suggestions. It indicates the start of a suggestion rendering session.
4849
/// </summary>
49-
/// <param name="acceptedSuggestion">The accepted suggestion text.</param>
50-
void OnSuggestionAccepted(string acceptedSuggestion);
50+
/// <param name="clientId">Represents the client that initiates the call.</param>
51+
/// <param name="context">The <see cref="PredictionContext"/> object to be used for prediction.</param>
52+
/// <param name="cancellationToken">The cancellation token to cancel the prediction.</param>
53+
/// <returns>An instance of <see cref="SuggestionPackage"/>.</returns>
54+
SuggestionPackage GetSuggestion(string clientId, PredictionContext context, CancellationToken cancellationToken);
5155

5256
/// <summary>
53-
/// Get the predictive suggestions.
57+
/// One or more suggestions provided by the predictor were displayed to the user.
5458
/// </summary>
55-
/// <param name="context">The <see cref="PredictionContext"/> object to be used for prediction.</param>
56-
/// <param name="cancellationToken">The cancellation token to cancel the prediction.</param>
57-
/// <returns>A list of predictive suggestions.</returns>
58-
List<PredictiveSuggestion>? GetSuggestion(PredictionContext context, CancellationToken cancellationToken);
59+
/// <param name="clientId">Represents the client that initiates the call.</param>
60+
/// <param name="session">The mini-session where the displayed suggestions came from.</param>
61+
/// <param name="countOrIndex">
62+
/// When the value is greater than 0, it's the number of displayed suggestions from the list returned in <paramref name="session"/>, starting from the index 0.
63+
/// When the value is less than or equal to 0, it means a single suggestion from the list got displayed, and the index is the absolute value.
64+
/// </param>
65+
void OnSuggestionDisplayed(string clientId, uint session, int countOrIndex);
66+
67+
/// <summary>
68+
/// The suggestion provided by the predictor was accepted.
69+
/// </summary>
70+
/// <param name="clientId">Represents the client that initiates the call.</param>
71+
/// <param name="session">Represents the mini-session where the accepted suggestion came from.</param>
72+
/// <param name="acceptedSuggestion">The accepted suggestion text.</param>
73+
void OnSuggestionAccepted(string clientId, uint session, string acceptedSuggestion);
5974
}
6075

6176
/// <summary>
@@ -160,4 +175,47 @@ public PredictiveSuggestion(string suggestion, string? toolTip)
160175
ToolTip = toolTip;
161176
}
162177
}
178+
179+
/// <summary>
180+
/// A package returned from <see cref="ICommandPredictor.GetSuggestion"/>.
181+
/// </summary>
182+
public struct SuggestionPackage
183+
{
184+
/// <summary>
185+
/// Gets the mini-session that represents a specific invocation to <see cref="ICommandPredictor.GetSuggestion"/>.
186+
/// When it's not specified, it's considered by a client that the predictor doesn't expect feedback.
187+
/// </summary>
188+
public uint? Session { get; }
189+
190+
/// <summary>
191+
/// Gets the suggestion entries returned from that mini-session.
192+
/// </summary>
193+
public List<PredictiveSuggestion>? SuggestionEntries { get; }
194+
195+
/// <summary>
196+
/// Initializes a new instance of the <see cref="SuggestionPackage"/> struct without providing a session id.
197+
/// Note that, when a session id is not specified, it's considered by a client that the predictor doesn't expect feedback.
198+
/// </summary>
199+
/// <param name="suggestionEntries">The suggestions to return.</param>
200+
public SuggestionPackage(List<PredictiveSuggestion> suggestionEntries)
201+
{
202+
Requires.NotNullOrEmpty(suggestionEntries, nameof(suggestionEntries));
203+
204+
Session = null;
205+
SuggestionEntries = suggestionEntries;
206+
}
207+
208+
/// <summary>
209+
/// Initializes a new instance of the <see cref="SuggestionPackage"/> struct with the mini-session id and the suggestions.
210+
/// </summary>
211+
/// <param name="session">The mini-session where suggestions came from.</param>
212+
/// <param name="suggestionEntries">The suggestions to return.</param>
213+
public SuggestionPackage(uint session, List<PredictiveSuggestion> suggestionEntries)
214+
{
215+
Requires.NotNullOrEmpty(suggestionEntries, nameof(suggestionEntries));
216+
217+
Session = session;
218+
SuggestionEntries = suggestionEntries;
219+
}
220+
}
163221
}

src/System.Management.Automation/engine/Utils.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2312,6 +2312,14 @@ internal static void NotNullOrEmpty(string value, string paramName)
23122312
}
23132313
}
23142314

2315+
internal static void NotNullOrEmpty(ICollection value, string paramName)
2316+
{
2317+
if (value == null || value.Count == 0)
2318+
{
2319+
throw new ArgumentNullException(paramName);
2320+
}
2321+
}
2322+
23152323
internal static void Condition([DoesNotReturnIf(false)] bool precondition, string paramName)
23162324
{
23172325
if (!precondition)

0 commit comments

Comments
 (0)