Improve social card UX, docs coverage, and metadata snippets

[PrzemyslawKlys]

Summary

• split social card labeling into two distinct roles:
  • top-right chip = category badge (HOME/DOCS/API/etc.)
  • bottom-left chip = route label (/, /docs, api, ...)
• remove duplicate badge rendering that caused the misaligned/awkward "PAGES" look
• add deterministic style variants for card palettes by content type (api, docs, editorial, default)
• pass route labels from both site pages and API docs generation paths
• improve social-card generation coverage for docs entry routes (docs landing + first-level docs pages)
• improve snippet extraction used by SEO/social descriptions to preserve spacing and decode HTML entities

Validation

• dotnet build .\\PowerForge.Web\\PowerForge.Web.csproj -c Release
• dotnet build .\\PowerForge.Web.Cli\\PowerForge.Web.Cli.csproj -c Release
• Built TestimoX website with this engine and validated:
  • OG/Twitter tags emitted for main pages
  • docs routes now receive generated social cards
  • card layout no longer duplicates top/bottom badge text
• Built IntelligenceX website with this engine and validated:
  • doctor passed in pipeline run
  • generated cards for home/docs/api are aligned and distinct
  • docs card description spacing improved

Notes

• This PR intentionally focuses on website/social-card behavior only.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[claude]

Code Review

Overall this is a clean, well-scoped PR. The separation of top-right badge (category) from bottom-left chip (route label) is a good UX improvement. A few items worth addressing before merge:

---

Likely Test Regression

WebSiteSocialCardsTests.Build_DoesNotAutoGenerateSocialCards_ForDocsPages_UnlessOverridden (line 263) will now fail. That test creates a docs page at docs/guide (two path segments) and asserts the default image is used, not a generated card. With ShouldGenerateSocialCardForDocsEntry, any docs page with segments <= 2 now does get a generated card:

var segments = normalized
    .Split('/', StringSplitOptions.RemoveEmptyEntries)
    .Length;
return segments <= 2;   // "docs/guide" → 2 → true

The new behavior is intentional per the PR description, but the test documents the old contract. It needs to be updated (or replaced with a test that verifies a deeper path, e.g., docs/getting-started/step-one, does not get a generated card).

---

Footer Label Inconsistency Between Site and API Cards

Site cards go through BuildSocialRouteDisplayLabel, which always returns a leading slash ("/" or "/docs/getting-started").

API cards use NormalizeApiRouteLabel, which explicitly adds a / then immediately strips it:

if (!value.StartsWith("/", StringComparison.Ordinal))
    value = "/" + value;
return value.Trim('/');   // strips the slash we just added → "api", not "/api"

As a result, API cards will render api while site cards render /docs/.... This looks like an unintended inconsistency now that routeLabel is directly visible on the card.

---

ClassifyStyle — Overly Broad Substring Matching

if (combined.Contains("doc", StringComparison.Ordinal))
    return "docs";
if (combined.Contains("post", StringComparison.Ordinal))
    return "editorial";

"doc" matches docker, docstring, /vendor/doc-util, etc. "post" matches /compose/repost, /postman-collection, /ghost-post-webhook, etc. The badge values are controlled (DOCS, API, HOME…) so false positives are unlikely there, but footer labels are derived from arbitrary URL paths. Consider checking against the badge value first and only falling back to route-path matching for well-known prefixes:

// Compare badge first (already normalised to uppercase)
if (string.Equals(badge, "API", StringComparison.OrdinalIgnoreCase)) return "api";
if (string.Equals(badge, "DOCS", StringComparison.OrdinalIgnoreCase)) return "docs";
// route-path fallback using segment prefix rather than raw substring

---

BuildSnippet — Silent Divergence from StripTags Helper

The old BuildSnippet called StripTags() (which replaces tags with ""). The new version bypasses it and replaces tags with " " to prevent word-concatenation. That's the right fix, but StripTags() still exists and is used for TOC heading extraction. A one-line comment in BuildSnippet would prevent the next reader from "fixing" it back to use StripTags():

// Replace with a space (not empty) so adjacent words don't concatenate across tags.
var withoutTags = StripTagsRegex.Replace(html, " ");

---

Minor: Redundant Explicit Badge Mappings

In ResolveSocialBadge:

if (string.Equals(collection, "pages", StringComparison.OrdinalIgnoreCase))
    return "PAGES";
if (string.Equals(collection, "docs", StringComparison.OrdinalIgnoreCase))
    return "DOCS";
if (string.Equals(collection, "blog", StringComparison.OrdinalIgnoreCase))
    return "BLOG";
return collection.ToUpperInvariant();

The first three branches produce exactly what collection.ToUpperInvariant() would produce, so they're redundant. The HOME branch at the top is the only case that cannot be expressed via ToUpperInvariant(). Not a bug, but worth simplifying if you're touching this area again.

---

File Length

WebSiteBuilder.DataAndDiagnostics.cs is now 864 lines, over the 800-line guideline in AGENTS.md. This may have been pre-existing — flagging it in case a split was already planned.

---

Summary of items to act on:

1. Update Build_DoesNotAutoGenerateCards_ForDocsPages_UnlessOverridden to reflect the new docs-entry scope (required — test will fail).
2. Fix NormalizeApiRouteLabel to return a leading-slash format consistent with BuildSocialRouteDisplayLabel, or vice versa (correctness issue).
3. Consider narrowing ClassifyStyle substring checks (low risk but worth hardening).
4. Add a comment to BuildSnippet explaining the space-replacement (nice to have).

[PrzemyslawKlys]

Added a follow-up commit with the 3 requested enhancements:

• per-collection social card style/variant presets (Social.GeneratedCardStylesByCollection, Social.GeneratedCardVariantsByCollection)
• per-page overrides (meta.social_card_badge, meta.social_card_route, meta.social_card_style, meta.social_card_variant)
• layout variants in renderer (hero/standard/compact) with default heuristics (home -> hero, docs/editorial -> compact)

Also improved snippet extraction for cleaner docs/blog social descriptions and updated Docs/PowerForge.Web.ContentSpec.md with examples.

[claude]

PR Review: Improve social card UX, docs coverage, and metadata snippets

Overall this is a well-structured PR with clear intent, good documentation, and consistent use of the existing code style. The precedence chain (per-page meta → collection config → global config → heuristics) is logical and well-documented. A few things worth looking at:

---

Code duplication (DRY violation)

InferSocialCardStyle (WebSiteBuilder.SocialCards.cs) and ClassifyStyle (WebSocialCardGenerator.cs) are nearly identical:

// WebSiteBuilder.SocialCards.cs
var combined = string.Concat(badge ?? string.Empty, " ", route ?? string.Empty).ToLowerInvariant();
if (combined.Contains("api", StringComparison.Ordinal)) return "api";
// ...

// WebSocialCardGenerator.cs – same logic, duplicated
var combined = string.Concat(badge ?? string.Empty, " ", footerLabel ?? string.Empty).ToLowerInvariant();
if (combined.Contains("api", StringComparison.Ordinal)) return "api";

Same duplication exists between InferSocialCardVariant and ClassifyVariant. Because the site builder always passes a resolved style key that NormalizeStyle can map, ClassifyStyle/ClassifyVariant in the generator should never fire in practice – but their presence makes the logic harder to follow and maintain. Consider either removing them (trusting the upstream resolver) or extracting them into a shared utility.

---

TryResolveCollectionCardPreset – redundant linear scan

// First attempt: case-sensitive TryGetValue (fast path)
if (map.TryGetValue(collection, out var direct) && ...) { ... }

// Second attempt: OrdinalIgnoreCase linear scan (O(n))
foreach (var kvp in map) { if (string.Equals(kvp.Key, collection, OrdinalIgnoreCase)) ... }

Since the JSON config keys are lowercase ("docs", "blog", etc.), the fallback loop will only matter if callers pass mixed-case collection names. A cleaner alternative: create the dictionary with StringComparer.OrdinalIgnoreCase during deserialization, making both attempts unnecessary. Or keep just the loop. Doing both is confusing.

---

Hardcoded palette indices in SelectPalette

var candidates = styleKey switch
{
    "api"       => new[] { 0, 2 },
    "docs"      => new[] { 2, 0 },
    "editorial" => new[] { 3, 1 },
    _           => new[] { 1, 0 }
};

These indices are coupled to the order of entries in Palettes. If a palette is added, removed, or reordered, the visual style mapping silently breaks. Named constants or at minimum a comment identifying which palette each index refers to would reduce fragility.

Additionally, these small arrays are heap-allocated on every RenderPng call. Using static readonly fields would avoid the repeated allocation.

---

footerRectWidth estimation inconsistency

var footerTextWidth = EstimateTextWidth(TrimSingleLine(primaryFooterLabel, 64), footerFontSize, glyphFactor: 0.52);

The width estimate trims to 64 chars, but safeFooter is trimmed at 56 chars for the hero variant. The pill can end up slightly wider than the rendered text on hero cards. Minor visual issue, but the trim lengths should be aligned.

---

API docs generator: hardcoded badge/style

In WebApiDocsGenerator.SocialCards.cs, badge, style, and variant are hardcoded:

"API",        // badge – hardcoded
routeLabel,
...,
"api",        // styleKey – hardcoded
"standard"    // variantKey – hardcoded

Fine as a default, but it means API doc pages cannot benefit from GeneratedCardStyle/GeneratedCardVariant site config, unlike regular pages. Plumbing overrides through options would be the natural place if this is ever needed.

---

SnippetParagraphRegex – timeout fallback is silent

private static readonly Regex SnippetParagraphRegex =
    new("<p\\b[^>]*>(?<text>[\\s\\S]*?)</p>", ..., RegexTimeout);

The pattern is correct for spanning newlines. The existing RegexTimeout provides a safety net, but if it fires (e.g. very large <p> blocks with embedded code), BuildSnippet will silently fall back to the full HTML path. A warning log on RegexMatchTimeoutException would make this easier to diagnose.

---

Minor nit: redundant null-forgiveness operators

if (!string.IsNullOrWhiteSpace(routeOverride))
    return routeOverride!.Trim();  // ! suppressor is not needed here

The compiler already narrows the type after the non-null/whitespace guard. Removing the ! operators would reduce noise.

---

Positives

• Precedence chain is clean and well-documented; the four-level fallback is easy to follow.
• BuildSnippet improvement (paragraph extraction + HtmlDecode) is a meaningful quality upgrade for SEO/social descriptions.
• ShouldGenerateSocialCardForDocsEntry correctly gates generation to the landing page and first-level routes, keeping generation costs bounded.
• Documentation in ContentSpec.md is thorough: precedence table, JSON config examples, and front-matter snippets are all included.
• EscapeXml is consistently applied to all user-supplied text inserted into SVG output.

Overall the PR is in good shape. The duplication and magic palette indices are the items worth addressing before merge; the rest are minor.

[PrzemyslawKlys]

Added follow-up social-card fitting fixes based on live IntelligenceX card regressions.\n\n### What changed\n- improved title/description wrapping for long CamelCase/API identifiers (removes awkward mid-token hyphen splits like Resu-lt)\n- added semantic badge/footer normalization in renderer so cards detect label intent better\n - long badge-like labels such as IntelligenceX API Reference collapse to API\n - footer labels normalize to route-like chips (/api, /docs, /showcase)\n - very long route labels are abbreviated (/api/.../type-slug)\n- capped footer chip max width to prevent oversized bottom pills\n- in API social generation, separated route key (hash/slug) from display label\n - keeps stable unique filenames while rendering concise footer chip labels\n\n### Validation\n- dotnet test PowerForge.Tests ... targeted social card tests passed (3/3)\n- rebuilt IntelligenceX website using this engine branch (POWERFORGE_ROOT override)\n- visually confirmed improved output for:\n - �pi-intelligencex-analysis-analysiscatalogvalidationresult-a427e35b22.png\n - showcase-ee2d6e3175.png\n - docs-getting-started-c23af94cf5.png\n\n### Note\n- existing test WebSiteSocialCardsTests.Build_DoesNotAutoGenerateCards_ForDocsPages_UnlessOverridden is already stale vs current docs-entry social-card behavior and still fails when run in the broad social suite (pre-existing mismatch).

[claude]

PR Review: Improve social card UX, docs coverage, and metadata snippets

Overall this is a well-structured PR with clear intent and good defensive coding (XML escaping, regex timeouts, path sanitization). The changes are focused and the documentation updates are thorough. A few items worth discussing:

---

Code Duplication

InferSocialCardStyle / ClassifyStyle and InferSocialCardVariant / ClassifyVariant are nearly identical.

WebSiteBuilder.SocialCards.cs has InferSocialCardStyle and InferSocialCardVariant, while WebSocialCardGenerator.cs has ClassifyStyle and ClassifyVariant — both doing the same string-matching logic independently. Any future change (e.g., adding a new style) needs to be kept in sync in two places. Consider extracting these to a shared static helper, or having the WebSiteBuilder side pass the already-resolved style/variant into RenderPng, skipping the fallback classification in the generator entirely (since callers now always resolve these before calling).

---

Potential False Positives in Substring Matching

ContainsToken does a plain IndexOf (substring) search, not a word-boundary check:

private static bool ContainsToken(string value, string token)
    => value.IndexOf(token, StringComparison.OrdinalIgnoreCase) >= 0;

This means:

• "apiary" matches "api"
• "podcast" matches "doc" (via LooksLikeDocsLabel)
• "document-processor" matches "doc"

Similarly InferSocialCardStyle / ClassifyStyle check combined.Contains("doc", ...) and combined.Contains("api", ...) on the concatenated badge + route string, which could produce unexpected style assignments. For example, a page at /advocate would trigger api matching (since "api" is a substring of "advocate" when lowercased)... actually no, "advocate" does not contain "api". But /apocalypse would match "api" since it contains "api". These edge cases may be rare in practice, but worth noting.

---

SelectPalette — Hardcoded Palette Indices Without Bounds Guard

var candidates = styleKey switch
{
    "api" => new[] { 0, 2 },
    "docs" => new[] { 2, 0 },
    "editorial" => new[] { 3, 1 },
    _ => new[] { 1, 0 }
};
var candidate = candidates[hash[0] % candidates.Length];
return Palettes[candidate % Palettes.Length];

The % Palettes.Length at the end provides a safety net, but it silently changes the intended palette mapping if the Palettes array ever has fewer than 4 entries. Index 3 mod 3 = 0, so editorial would accidentally fall back to index 0 (same as api) if a palette is removed. A Debug assertion or comment noting the dependency would help future maintainers.

---

API Docs Cards Ignore SocialSpec Configuration

In WebApiDocsGenerator.SocialCards.cs, the style and variant are hardcoded:

"API",        // badge — hardcoded
routeLabel,
options.SocialCardWidth,
options.SocialCardHeight,
"api",        // styleKey — hardcoded
"standard");  // variantKey — hardcoded

If a site sets GeneratedCardStyle: "docs" or GeneratedCardStylesByCollection, that config is applied to site pages but not to API doc cards. This may be intentional (API cards always use the api style), but it's worth an explicit comment, and arguably the variant at least ("standard" vs. "compact") should respect the global config.

---

LooksLikeApiReferenceLabel May Be Too Restrictive

private static bool LooksLikeApiReferenceLabel(string value)
    => ContainsToken(value, "api") && ContainsToken(value, "reference");

This requires both "api" AND "reference" to be present. A label like "API DOCS" or simply "API" won't match, so NormalizeBadgeLabel won't abbreviate it as "API". Given how the other LooksLike* helpers work (single-token), this asymmetry seems unintentional.

---

NormalizeWrapInput Silently Swallows RegexMatchTimeoutException

catch
{
    // Best effort only; fallback to raw text.
}

A bare catch swallows all exceptions, including RegexMatchTimeoutException that would indicate the 1-second timeout was hit. For a card generator producing output files, silent degradation is probably fine here, but at minimum catching RegexMatchTimeoutException explicitly and logging (or catching Exception with a comment distinguishing the expected timeout case) would make intent clearer. Consider at least catch (Exception) with a comment mentioning timeouts.

---

Hardcoded "powershell" Path Segment in BuildApiRouteDisplayLabel

if (segments.Length >= 3 &&
    string.Equals(segments[1], "powershell", StringComparison.OrdinalIgnoreCase))
    return $"/api/powershell/{segments[^1]}";

This is project-specific logic baked into a general-purpose helper. If another project uses this engine with a different second-level API namespace, this hardcoded check produces misleading labels. Consider making this configurable or at least documenting the assumption.

---

SplitToken Chunk Size Change

// Before:
var chunkSize = Math.Max(2, maxChars - 1);
yield return remaining[..chunkSize] + "-";

// After:
var chunkSize = Math.Max(3, maxChars);
yield return remaining[..chunkSize];

Removing the hyphen means very long tokens (e.g., a long method name or URL) are hard-wrapped without any visual indicator. For text displayed on social cards this may or may not matter visually. The minimum floor changing from 2 to 3 is a minor improvement. Just flagging this as an intentional behavioral change worth being aware of.

---

Missing Test Coverage

The PR introduces substantial new logic:

• BuildApiRouteDisplayLabel (route display formatting with branching)
• ResolveSocialBadge / ResolveSocialCardStyle / ResolveSocialCardVariant (resolution precedence)
• TryResolveCollectionCardPreset (case-insensitive fallback lookup)
• AbbreviateRouteLabel (route truncation)
• NormalizeBadgeLabel / NormalizeFooterLabel (normalization pipelines)
• BuildSnippet (paragraph extraction + HTML decode)

None of this has automated test coverage. Given the number of string-manipulation edge cases and the resolution precedence logic, unit tests would significantly increase confidence and prevent regressions. The manual validation described in the PR summary is appreciated, but these functions would benefit from parameterized unit tests.

---

Minor: Null-forgiving Operator After IsNullOrWhiteSpace Guard

Several places use ! after already guarding:

if (!string.IsNullOrWhiteSpace(routeOverride))
    return routeOverride!.Trim();  // ! not needed here

The ! is harmless but slightly noisy — the compiler can't infer non-null here without a nullable annotation, so the ! suppresses a warning. If nullable reference types are enabled on this project, these are fine as-is. If not, they're just redundant suppressions.

---

Summary

Category	Finding	Severity
Code quality	InferSocialCardStyle/ClassifyStyle duplication	Medium
Correctness	ContainsToken substring vs. word boundary	Low
Correctness	LooksLikeApiReferenceLabel overly strict	Low
Maintainability	Hardcoded palette indices without bounds comment	Low
Completeness	API doc cards ignore SocialSpec config	Medium
Maintainability	Hardcoded "powershell" in route label builder	Low
Reliability	Silent exception swallowing in NormalizeWrapInput	Low
Testing	No unit tests for new logic	Medium

The PR is in good shape for its stated scope. The docs coverage improvements and the cleaner separation of badge vs. route label are the most impactful changes. Addressing the duplication between site builder and card generator classification logic would be the highest-value cleanup.