Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AOT] Clean up some AOT issues in Advanced Paste module #36297

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
+80 −17
Open

[AOT] Clean up some AOT issues in Advanced Paste module #36297

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ee1dba5
init
Dec 11, 2024
93fe40d
Do not update package version in this PR
Dec 11, 2024
4d428c8
Downgrade reverseMarkdown version
Dec 11, 2024
14a1bb6
Clean up some unnessesary change
Dec 11, 2024
046ab9a
Clean up unnessary change
Dec 11, 2024
839dba7
We can not mark AdvancedPaste is AOT compatible. We need to clean up …
Dec 13, 2024
5d814e0
merge main
Dec 19, 2024
ff532d0
Merge main and clean up new AOT build issue
Dec 19, 2024
dcd9603
Update LogEvent define.
Dec 24, 2024
fbef327
Add Kernal to spell check allow list.
Dec 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 36 additions & 3 deletions src/modules/AdvancedPaste/AdvancedPaste/Helpers/LogEvent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,50 @@

using System.Text.Json;
using AdvancedPaste.Models.KernelQueryCache;
using AdvancedPaste.Telemetry;

namespace AdvancedPaste.Helpers
{
public class LogEvent
{
public LogEvent(object message)
public LogEvent(bool cacheUsed, bool isSavedQuery, int promptTokens, int completionTokens, string modelName, string actionChain)
{
this.message = message;
CacheUsed = cacheUsed;
IsSavedQuery = isSavedQuery;
PromptTokens = promptTokens;
CompletionTokens = completionTokens;
ModelName = modelName;
ActionChain = actionChain;
}

private object message;
public LogEvent(AdvancedPasteSemanticKernelFormatEvent semanticKernalFormatEvent)
Fixed Show fixed Hide fixed
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public LogEvent(AdvancedPasteSemanticKernelFormatEvent semanticKernalFormatEvent)
public LogEvent(AdvancedPasteSemanticKernelFormatEvent semanticKernelFormatEvent)

{
CacheUsed = semanticKernalFormatEvent.CacheUsed;
Fixed Show fixed Hide fixed
IsSavedQuery = semanticKernalFormatEvent.IsSavedQuery;
Fixed Show fixed Hide fixed
PromptTokens = semanticKernalFormatEvent.PromptTokens;
Fixed Show fixed Hide fixed
CompletionTokens = semanticKernalFormatEvent.CompletionTokens;
Fixed Show fixed Hide fixed
ModelName = semanticKernalFormatEvent.ModelName;
ActionChain = semanticKernalFormatEvent.ActionChain;
}

public LogEvent(AdvancedPasteGenerateCustomFormatEvent generateCustomFormatEvent)
{
PromptTokens = generateCustomFormatEvent.PromptTokens;
CompletionTokens = generateCustomFormatEvent.CompletionTokens;
ModelName = generateCustomFormatEvent.ModelName;
}

public bool IsSavedQuery { get; set; }

public bool CacheUsed { get; set; }

public int PromptTokens { get; set; }

public int CompletionTokens { get; set; }

public string ModelName { get; set; }

public string ActionChain { get; set; }

public string ToJsonString() => JsonSerializer.Serialize(this, AdvancedPasteJsonSerializerContext.Default.PersistedCache);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public string ToJsonString() => JsonSerializer.Serialize(this, AdvancedPasteJsonSerializerContext.Default.PersistedCache);
public string ToJsonString() => JsonSerializer.Serialize(this, AdvancedPasteJsonSerializerContext.Default.LogEvent);

Also, I believe the LogEvent class should be more defined.

Since we know telemetryEvent.CacheUsed, telemetryEvent.IsSavedQuery, telemetryEvent.PromptTokens, telemetryEvent.CompletionTokens, telemetryEvent.ModelName, telemetryEvent.ActionChain are the properties passed into the anonymous object, defining them in the class will make it cleaner and less prone to potential issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me consider how to make this interface easy to use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snickler
I've updated the LogEvent define. Could you please take a look?

}
Expand Down
2 changes: 1 addition & 1 deletion src/modules/AdvancedPaste/AdvancedPaste/Services/KernelServiceBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ private void LogResult(bool cacheUsed, bool isSavedQuery, IEnumerable<ActionChai
{
AdvancedPasteSemanticKernelFormatEvent telemetryEvent = new(cacheUsed, isSavedQuery, usage.PromptTokens, usage.CompletionTokens, ModelName, AdvancedPasteSemanticKernelFormatEvent.FormatActionChain(actionChain));
PowerToysTelemetry.Log.WriteEvent(telemetryEvent);
var logEvent = new LogEvent(new { telemetryEvent.CacheUsed, telemetryEvent.IsSavedQuery, telemetryEvent.PromptTokens, telemetryEvent.CompletionTokens, telemetryEvent.ModelName, telemetryEvent.ActionChain });
var logEvent = new LogEvent(telemetryEvent);
Logger.LogDebug($"{nameof(TransformClipboardAsync)} complete; {logEvent.ToJsonString()}");
}

Expand Down
2 changes: 1 addition & 1 deletion src/modules/AdvancedPaste/AdvancedPaste/Services/OpenAI/CustomTextTransformService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ public async Task<string> TransformTextAsync(string prompt, string inputText)
var usage = response.Usage;
AdvancedPasteGenerateCustomFormatEvent telemetryEvent = new(usage.PromptTokens, usage.CompletionTokens, ModelName);
PowerToysTelemetry.Log.WriteEvent(telemetryEvent);
var logEvent = new LogEvent(telemetryEvent);

var logEvent = new LogEvent(new { telemetryEvent.PromptTokens, telemetryEvent.CompletionTokens, telemetryEvent.ModelName });
Logger.LogDebug($"{nameof(TransformTextAsync)} complete; {logEvent.ToJsonString()}");

return response.Choices[0].Text;
Expand Down
Loading