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.

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
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

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

namespace AdvancedPaste.Helpers;

[JsonSerializable(typeof(PersistedCache))]
[JsonSerializable(typeof(LogEvent))]
[JsonSourceGenerationOptions(UseStringEnumConverter = true)]
public sealed partial class AdvancedPasteJsonSerializerContext : JsonSerializerContext
{
}
54 changes: 54 additions & 0 deletions src/modules/AdvancedPaste/AdvancedPaste/Helpers/LogEvent.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

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

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

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?

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal struct INPUT

internal static int Size
{
get { return Marshal.SizeOf(typeof(INPUT)); }
get { return Marshal.SizeOf<INPUT>(); }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

namespace AdvancedPaste.Settings
{
internal sealed class UserSettings : IUserSettings, IDisposable
internal sealed partial class UserSettings : IUserSettings, IDisposable
{
private readonly SettingsUtils _settingsUtils;
private readonly TaskScheduler _taskScheduler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,15 @@ public sealed class PersistedCache : ISettingsConfig
{
public record class CacheItem(CacheKey CacheKey, CacheValue CacheValue);

private static readonly JsonSerializerOptions SerializerOptions = new()
{
Converters =
{
new JsonStringEnumConverter(),
},
};

public static PersistedCache FromJsonString(string json) => JsonSerializer.Deserialize<PersistedCache>(json, SerializerOptions);
public static PersistedCache FromJsonString(string json) => JsonSerializer.Deserialize<PersistedCache>(json, AdvancedPasteJsonSerializerContext.Default.PersistedCache);

public string Version { get; init; }

public List<CacheItem> Items { get; init; } = [];

public string GetModuleName() => Constants.AdvancedPasteModuleName;

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

public override string ToString() => ToJsonString();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,8 @@ 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 { telemetryEvent.CacheUsed, telemetryEvent.IsSavedQuery, telemetryEvent.PromptTokens, telemetryEvent.CompletionTokens, telemetryEvent.ModelName, telemetryEvent.ActionChain };
Logger.LogDebug($"{nameof(TransformClipboardAsync)} complete; {JsonSerializer.Serialize(logEvent)}");
var logEvent = new LogEvent(telemetryEvent);
Logger.LogDebug($"{nameof(TransformClipboardAsync)} complete; {logEvent.ToJsonString()}");
}

private Kernel CreateKernel()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ 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 { telemetryEvent.PromptTokens, telemetryEvent.CompletionTokens, telemetryEvent.ModelName };
Logger.LogDebug($"{nameof(TransformTextAsync)} complete; {JsonSerializer.Serialize(logEvent)}");
Logger.LogDebug($"{nameof(TransformTextAsync)} complete; {logEvent.ToJsonString()}");

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