Skip to content

AI modifying - #8

Merged
wojciech-dron merged 6 commits into
masterfrom
cv-modify
Mar 2, 2026
Merged

wojciech-dron merged 6 commits into
masterfrom
cv-modify

Conversation

@wojciech-dron

Copy link
Copy Markdown
Owner

No description provided.

@github-actions

github-actions Bot commented Mar 2, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Lifecycle Bug

The component initializes Monaco models in OnInitializedAsync, but the @ref for the editor is typically not populated until after first render. This can cause the initial models/theme setup to be skipped (because _diffEditor is null) and never retried, leaving the editor without the expected content/models. Consider moving initialization to OnAfterRenderAsync (firstRender) and/or re-initializing when the reference becomes available.

protected override async Task OnInitializedAsync()
{
    await Global.SetTheme(jsRuntime, "vs-dark");

    await InitModels();
}

public async Task SetModels(string original, string modified)
{
    await SetOriginalModel(original);
    await SetModifiedModel(modified);
}

public async Task SetOriginalModel(string value)
{
    if (_diffEditor is null)
        return;

    await _diffEditor.OriginalEditor.SetValue(value);
}

public async Task SetModifiedModel(string value)
{
    if (_diffEditor is null)
        return;

    await _diffEditor.ModifiedEditor.SetValue(value);
}

private async Task InitModels()
{
    if (_diffEditor is null)
        return;

    var originalModel = await Global.GetModel(jsRuntime, $"{Id}-original") ??
        await Global.CreateModel(jsRuntime, InitOriginalContent, "markdown", $"{Id}-original");

    var modifiedModel = await Global.GetModel(jsRuntime, $"{Id}-modified") ??
        await Global.CreateModel(jsRuntime, InitModifiedContent, "markdown", $"{Id}-modified");

    await _diffEditor.SetModel(new DiffEditorModel
    {
        Original = originalModel,
        Modified = modifiedModel,
    });
}
Runtime IO

Loading the image via File.ReadAllBytes in a static field initializer relies on a relative path and executes at type-load time. This can fail depending on working directory and hosting model (and will throw early, potentially breaking page/component initialization). Consider embedding the asset, using an injected file provider, or resolving via AppContext.BaseDirectory/content root and handling missing file gracefully.

public static class TestData
{
    public static readonly byte[] Image = File.ReadAllBytes("Features/Cv/Components/image.jpg");

    #region BaseCv
    public const string BaseCv =
Retry Logic

The retry loop uses ShouldRetry(finalContent, retryCount++) with MaxRetries = 2, which effectively allows an extra iteration compared to what the name suggests (and can also append duplicate chat history across retries). Consider making retry counting explicit (e.g., increment before check or use >=) and/or isolating chat history per attempt if you only want the final attempt’s messages.

    string? finalContent = null;
    var retryCount = 0;
    var chatHistory = new List<ChatItem>();
    do
    {
        var chat = PrepareAgentsChat(request);
        chat.AddChatMessage(offerMessage);

        if (request.OfferSummary is not null)
        {
            var summaryMessage = new ChatMessageContent(AuthorRole.User, $"offerSummary: {request.OfferSummary}");
            chat.AddChatMessage(summaryMessage);
        }

        var asyncResponse = chat.InvokeAsync(cancellationToken);

        await foreach (var response in asyncResponse)
        {
            chatHistory.Add(ChatItem.From(response));
            finalContent = response.Content;
        }
    } while (ShouldRetry(finalContent, retryCount++));

    if (finalContent is null || !finalContent.Contains(DoneSignal))
        return new Response(false, "", chatHistory);

    var adjustedCv = finalContent.Replace(DoneSignal, "").Trim();

    return new Response(true, adjustedCv, chatHistory);
}

private static bool ShouldRetry(string? lastContent, int retryCount)
{
    if (retryCount > MaxRetries) // accept content after max retries
        return false;

    if (string.IsNullOrEmpty(lastContent))
        return true;

    if (lastContent.Contains(FailSignal, StringComparison.OrdinalIgnoreCase))
        return true;

    if (lastContent.Length < DoneSignal.Length + 50) // CV should be significantly longer than just the signal
        return true;

    return false;
}

@github-actions

github-actions Bot commented Mar 2, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent startup crash from missing file

Avoid reading files during type initialization, because a missing/relative path will
throw and can crash the app on first access to TestData. Load the image lazily and
resolve the path from AppContext.BaseDirectory, returning an empty array when the
file is not found.

JobScraper.Web/Features/Cv/Components/TestData.cs [9]

-public static readonly byte[] Image = File.ReadAllBytes("Features/Cv/Components/image.jpg");
+private static readonly Lazy<byte[]> _image = new(() =>
+{
+    var path = Path.Combine(AppContext.BaseDirectory, "Features", "Cv", "Components", "image.jpg");
+    return File.Exists(path) ? File.ReadAllBytes(path) : [];
+});
 
+public static byte[] Image => _image.Value;
+
Suggestion importance[1-10]: 7

__

Why: File.ReadAllBytes(...) in a static readonly initializer can throw (missing file/relative path), causing failures as soon as TestData is accessed. Lazy-loading and resolving from AppContext.BaseDirectory meaningfully improves runtime robustness for this dev/test helper.

Medium
Avoid out-of-range and async issues

Guard the RemoveAt call to avoid an exception when chatHistory is empty (e.g.,
failed/short agent responses). Also await the editor update to ensure the UI is
updated deterministically and avoid firing-and-forgetting a task.

JobScraper.Web/Features/Cv/PrepareCvForOfferModal.razor [158-160]

-chatHistory.RemoveAt(chatHistory.Count - 1); // remove last item because it is CV content
+if (chatHistory.Count > 0)
+    chatHistory.RemoveAt(chatHistory.Count - 1); // remove last item because it is CV content
+
 ModifiedContent = result.AdjustedCvContent;
-_ =_diffEditor.SetModifiedModel(ModifiedContent);
 
+if (_diffEditor is not null)
+    await _diffEditor.SetModifiedModel(ModifiedContent);
+
Suggestion importance[1-10]: 6

__

Why: chatHistory.RemoveAt(chatHistory.Count - 1) can throw if chatHistory is unexpectedly empty, and fire-and-forget _diffEditor.SetModifiedModel(...) can lead to nondeterministic UI updates. Guarding the removal and awaiting the editor update is a correct, low-risk stability improvement.

Low

@wojciech-dron
wojciech-dron merged commit 3eed97a into master Mar 2, 2026
1 check passed
@wojciech-dron wojciech-dron changed the title Cv AI modifying AI modifying Mar 4, 2026
@wojciech-dron
wojciech-dron deleted the cv-modify branch March 9, 2026 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant