Skip to content

AiSummaryConfigPage - selecting & CV generation - #14

Closed
wojciech-dron wants to merge 3 commits into
masterfrom
cv-generation-pipeline
Closed

wojciech-dron wants to merge 3 commits into
masterfrom
cv-generation-pipeline

Conversation

@wojciech-dron

Copy link
Copy Markdown
Owner

No description provided.

@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 Security concerns

Information exposure:
SelectCvTemplateForOffer adds ex.Message into a chat item returned to the UI, which may leak internal details (provider errors, configuration, stack-adjacent info). Consider logging full exception details server-side and returning a generic user-facing error message.

⚡ Recommended focus areas for review

Filtering Bug

The default FilterPredicate is set to a non-null function that always returns false, so OnInitialized never assigns the intended default predicate. As a result, enabling filtering will likely hide all items unless every caller provides its own FilterPredicate.

/// </summary>
[Parameter]
public Func<TItem, string, bool> FilterPredicate { get; set; } = (item, filter) => false;

/// <summary>
///     Whether filtering/search is enabled.
/// </summary>
[Parameter]
public bool CanFilter { get; set; }

/// <summary>
///     Optional label displayed above the component.
/// </summary>
[Parameter]
public string? Label { get; set; }

/// <summary>
///     Placeholder text for the filter input.
/// </summary>
[Parameter]
public string FilterPlaceholder { get; set; } = "Search...";

/// <summary>
///     Text shown when no items match the filter.
/// </summary>
[Parameter]
public string EmptyText { get; set; } = "No items found";

/// <summary>
///     The id for the component elements.
/// </summary>
[Parameter]
public string? Id { get; set; }

/// <summary>
///     Additional CSS classes to apply.
/// </summary>
[Parameter]
public string? Class { get; set; }

/// <summary>
///     Additional CSS styles to apply.
/// </summary>
[Parameter]
public string? Style { get; set; }

private Func<TItem, string, bool> DefaultFilterPredicate =>
    (item, filter) => StringSelector(item).Contains(filter, StringComparison.OrdinalIgnoreCase);

protected override void OnInitialized()
{
    FilterPredicate ??= DefaultFilterPredicate;
}
Wrong Mapping

The test summarize flow appears to pass selected CV markdown content as the offer content, while still using form.CvContent as CV content. This likely swaps the meaning of inputs for SummarizeOfferContent.Request and can also lead to null-forgiveness issues if no CV content is resolved.

var aiModel = !string.IsNullOrEmpty(form.SmartAiModel) ? form.SmartAiModel : form.DefaultAiModel;
var content =
    availableCvs.FirstOrDefault(x => x.Id == selectedCvId)?.MarkdownContent ??
    SelectedCv?.MarkdownContent;

var request = new SummarizeOfferContent.Request(
    CvContent: form.CvContent,
    OfferContent: content!,
    UserRequirementsForOffer: form.UserRequirements ?? "",
    ProviderName: aiModel);
Prompt Size

The agent prompt embeds the full markdown of all CV templates. This can easily exceed token limits and/or degrade model performance; consider truncation, summarization, or sending only metadata + a short excerpt per template.

var templatesDescription = string.Join("\n\n-----------------------------------\n\n",
    templates.Select(t =>
        $"--- Template ID: {t.Id}, Name: {t.Name} ---\n{t.MarkdownContent}"));

var agent = new ChatCompletionAgent
{
    Name = "CvAssistant",
    Kernel = kernel,
    Instructions =
        $"""
         You are an expert recruiter assistant that selects the best CV template for a job offer.
         Your task is to analyze a job offer description and a list of CV templates, then decide which template is the best match.

         The most important criteria is language match - the CV template language should match the language of the offer description.
         After language match, consider the relevance of skills, experience, and keywords between the CV content and the offer.

         You MUST respond with the numeric ID of the best matching template.
         No explanation, no other text - just the number.

         Available CV templates:
         {templatesDescription}
         """,
    LoggerFactory = loggerFactory,
};

@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix swapped request inputs

The request arguments are swapped: OfferContent is being set to CV markdown while
CvContent still uses the deprecated form.CvContent. This will produce incorrect
summaries and can also throw via content! when no CV is selected. Build cvContent
from the selected CV and pass form.TestOfferContent as the offer content, with a
safe null/empty check.

JobScraper.Web/Features/AiSummary/AiSummaryConfigPage.razor.cs [171-181]

 var aiModel = !string.IsNullOrEmpty(form.SmartAiModel) ? form.SmartAiModel : form.DefaultAiModel;
-var content =
+
+var cvContent =
     availableCvs.FirstOrDefault(x => x.Id == selectedCvId)?.MarkdownContent ??
     SelectedCv?.MarkdownContent;
 
+if (string.IsNullOrWhiteSpace(cvContent))
+{
+    toasts.PushMessage("Select a CV template first.", ToastType.Warning);
+    isWorking = false;
+    return;
+}
+
 var request = new SummarizeOfferContent.Request(
-    CvContent: form.CvContent,
-    OfferContent: content!,
+    CvContent: cvContent,
+    OfferContent: form.TestOfferContent!,
     UserRequirementsForOffer: form.UserRequirements ?? "",
     ProviderName: aiModel);
Suggestion importance[1-10]: 9

__

Why: In SummarizeOfferContent.Request, OfferContent is currently populated from selected CV markdown (content) while CvContent uses form.CvContent, which is inconsistent with the UI changes (CV content input removed) and likely produces wrong summaries. The suggested swap plus a null/empty guard prevents incorrect behavior and avoids a potential crash from content!.

High
Scope config to current user

AiSummaryConfig is keyed by Owner, but the page loads the first row without
filtering and creates new rows with a constant "default" owner. This can
overwrite/duplicate configs across users and likely fails on primary-key collisions.
Load and create configs using the actual dbContext.CurrentUserName key.

JobScraper.Web/Features/AiSummary/AiSummaryConfigPage.razor.cs [50-58]

+var owner = dbContext.CurrentUserName;
+
 form = await dbContext.AiSummaryConfigs
+        .Where(x => x.Owner == owner)
         .Select(AiSummaryViewModel.Projection)
         .FirstOrDefaultAsync()
  ?? new AiSummaryViewModel
     {
-        Owner = "default",
+        Owner = owner,
         DefaultAiModel = AiProvidersConfig.MainProvider,
         AiSummaryEnabled = true,
     };
Suggestion importance[1-10]: 8

__

Why: AiSummaryConfig is keyed by Owner, but the page loads FirstOrDefaultAsync() without filtering and initializes new configs with a constant Owner = "default", which can cause cross-user config reads/writes and primary key collisions. Filtering/creating by dbContext.CurrentUserName aligns the page behavior with the entity key and avoids data corruption.

Medium
Make filtering work by default

Filtering is effectively broken because FilterPredicate defaults to a function that
always returns false, and OnInitialized never replaces it (it’s never null). This
makes searches return zero matches and can hide all items when the user types. Make
FilterPredicate nullable (or default it to DefaultFilterPredicate) and use a
coalesce at call-site.

JobScraper.Web/Blazor/Components/ButtonSelect/ButtonSelect.razor [74-125]

 [Parameter]
-public Func<TItem, string, bool> FilterPredicate { get; set; } = (item, filter) => false;
+public Func<TItem, string, bool>? FilterPredicate { get; set; }
 
 ...
 
-protected override void OnInitialized()
-{
-    FilterPredicate ??= DefaultFilterPredicate;
-}
+private IEnumerable<TItem> FilteredItems => Items
+    .Where(item => !CanFilter
+        || string.IsNullOrWhiteSpace(SearchPhrase)
+        || (FilterPredicate ?? DefaultFilterPredicate)(item, SearchPhrase));
Suggestion importance[1-10]: 6

__

Why: FilterPredicate defaults to a lambda that always returns false, and OnInitialized won’t replace it because it’s never null, so typing in the search box will hide all items. Making FilterPredicate nullable (or defaulting it to DefaultFilterPredicate at call-site) fixes filtering behavior for all usages that don’t explicitly pass a predicate.

Low

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