Harden API docs quality, media, and PowerShell example workflows

[PrzemyslawKlys]

Summary

• remove the redundant API overview right rail and harden the shared API CSS contract so site themes must style the enhanced namespace filter, TOC toggle, inherited-members toggle, and example media metadata
• improve C# API docs diagnostics with sampled source-link preflight warnings, parent-folder sourceRoot hints, and git-aware freshness metadata/badges for generated pages
• expand PowerShell API docs quality end to end: smarter fallback examples, command-specific example ranking, provenance tracking, coverage metrics, CI thresholds, validation, execution, transcripts, playback asset staging, and a reusable example-media manifest
• render richer API examples with media attachments, provenance badges, capture recency, transcript/playback links, and pipeline/CLI visibility for validation and media artifacts
• companion website polish PR: Polish IntelligenceX API docs UI and theme contract IntelligenceX#1226

Testing

• dotnet test .\PowerForge.Tests\PowerForge.Tests.csproj -c Release --filter "FullyQualifiedName~WebApiDocsGeneratorPowerShellTests|FullyQualifiedName~WebPipelineRunnerApiDocsPreflightTests|FullyQualifiedName~WebCliApiDocsPreflightTests|FullyQualifiedName~WebApiDocsGeneratorSourceAndCssTests|FullyQualifiedName~WebApiDocsGeneratorContractTests"

[claude]

PR Review: Harden API docs layout and fallback generation

Overall this is a well-structured PR with clear intent, good test coverage for the new behaviors, and the removal of the right-rail simplifies the layout contract nicely. A few observations below.

---

Code Quality / Maintainability

Magic numbers in the scoring algorithm
GetGeneratedPowerShellExampleScore uses raw integers (14, 36, 10, 8, 28, 40, 12, 16, 8, 6) with no named constants or inline comments explaining what each threshold represents. This makes future tuning difficult.

// Hard to reason about without context:
score += Math.Max(0, 36 - Math.Abs(required.Count - 1) * 10);
score += 14;  // what does 14 represent?

Consider named constants (ScoreNoRequiredParams, ScorePerRequiredParam, etc.) or at least a brief comment block explaining the scoring intent.

IsPowerShellFriendlyExampleType — long OR-chain instead of a set
The method checks ~30 type names via chained .Equals() calls. A static HashSet<string> lookup would be both faster (O(1)) and easier to maintain when adding new types:

private static readonly HashSet<string> _friendlyTypes = new(StringComparer.OrdinalIgnoreCase)
{
    "string", "String", "int", "Int32", "long", "Int64", ...
};

Note also the redundancy: "String" and "string" are both listed, but OrdinalIgnoreCase already covers that — only one form is needed per type.

---

Potential Bugs

Integer overflow risk in GetGeneratedPowerShellExampleParameterScore
The method returns int.MinValue for a null parameter. In GetGeneratedPowerShellExampleScore, required.Sum(GetGeneratedPowerShellExampleParameterScore) would overflow if the parameters list contains nulls. While the outer Where(method => method is not null) guards methods, there's no equivalent guard on method.Parameters. If Parameters can contain null entries, this is a silent overflow bug.

ScriptBlock scores both +12 and -28 simultaneously
IsPowerShellFriendlyExampleType returns true for "ScriptBlock" (type score +12), but GetGeneratedPowerShellExampleParameterScore also docks -28 for a parameter named ScriptBlock. A parameter of type ScriptBlock named ScriptBlock would net -16, which is probably the right outcome, but it's non-obvious. A comment here would help future readers.

IsPowerShellFriendlyExampleType returns true for null/empty types

if (string.IsNullOrWhiteSpace(typeName))
    return true;  // untyped → friendly (+12 score)

Untyped parameters get the friendly-type bonus. This may be intentional for PowerShell's dynamic nature, but it means any untyped junk parameter competes equally with well-typed string parameters. Worth a comment confirming the intent.

---

Behavioral Change Worth Noting

The old BuildGeneratedPowerShellExample preferred parameter sets with more required parameters (OrderByDescending(Count(!IsOptional))). The new scoring reverses this — it now prefers sets with fewer required parameters (approximately 1 required param scores highest). This is the right UX choice for friendlier examples, but it's a meaningful behavioral change not called out in the PR summary. Tests do verify the new behavior, which is good.

---

Test Coverage

The new tests are solid:

• Generate_PowerShellHelp_GeneratesFallbackExamplesFromBestParameterSets — correctly validates that ByName/ById are preferred over ByInputObject, and that InputObject is excluded.
• GenerateDocsHtml_OmitsOverviewWorkspaceRail_WhenUsingDocsTemplate — cleanly pins the layout regression.
• The two preflight tests (CLI + pipeline) for the sourceRoot-above-repo heuristic.

A couple of edge cases not covered:

• PowerShellFallbackExampleLimitPerCommand = 0 — BuildGeneratedPowerShellExamples returns early with limit <= 0, but no test exercises this boundary.
• Deduplication via seen: if two parameter sets produce identical code snippets (e.g., same required params with same names), only one example emits. No test covers this scenario — worth adding to document the intended behavior.

---

TryExtractGitHubRepoName — Minor

The token substitution approach to build a valid URI is clever but slightly fragile if a future template token is added and not included in the replacements. A brief comment noting that this is only testing for github.com hostname + owner/repo structure (not round-tripping the full URL) would help maintainers.

---

Summary

Area	Status
Layout removal (right rail)	✅ Clean, well-tested
sourceRoot preflight heuristic	✅ Solid, two test paths
Fallback example generation refactor	✅ Correct behavior, good tests
Scoring algorithm readability	⚠️ Magic numbers, needs comments/constants
IsPowerShellFriendlyExampleType	⚠️ Long OR-chain, redundant case-insensitive pairs
Null-in-parameters overflow	⚠️ Unlikely but worth guarding

No blocking issues — the core logic is correct and the tests prove it. The scoring magic numbers are the main thing I'd address before merging, since this function will likely need tuning as edge cases emerge.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8652823f2b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Skip parent-repo warning when sourceUrl uses {pathNoRoot}

This preflight branch warns solely on missing {root}, but a config that points sourceRoot to the parent folder and uses {pathNoRoot} is valid (the URL builder strips the first segment before substitution). In that scenario, links are correct yet this warning still fires, and with failOnWarnings:true the pipeline/CLI exits as failure for a non-issue. Please gate this heuristic on the actual path token behavior (at minimum, avoid warning when {pathNoRoot} is used).

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Emit command-only fallback when syntax entries are missing

Fallback example generation now iterates only over type.Methods, so commands with no parsed syntax items produce zero generated examples. This regresses the previous behavior that still emitted a basic command invocation, and it can drop PowerShell example coverage unexpectedly for help files that omit <syntax>/<syntaxItem> (causing emptier docs and potential coverage-gate failures). Add a no-method fallback path that still returns at least Command-Name.

Useful? React with 👍 / 👎.

[claude]

PR Review: Harden API docs layout and fallback generation

Overall this is a well-structured PR with clear goals and good test coverage across all three feature areas. Here are my observations:

---

Layout cleanup (WebApiDocsGenerator.Html.cs)

The removal of the right-rail stats/jump-to section is clean. Unused locals (aliasCount, primaryKindCount) are properly deleted, and the new api-overview-main--full modifier follows the existing naming convention. The companion test GenerateDocsHtml_OmitsOverviewWorkspaceRail_WhenUsingDocsTemplate is appropriately negative — checking that the removed strings no longer appear in the output.

Observation: The api-overview-grid CSS class is removed from the required-selector contract, which is consistent. However, since the class is no longer emitted in HTML it's worth making sure no existing deployed CSS theme files reference it — consumers relying on that class for custom overrides would silently lose the selector without a warning.

---

IsPowerShellFriendlyExampleType — redundant comparisons

The function lists both the C# keyword and the CLR class name for every primitive, e.g.:

type.Equals("String", StringComparison.OrdinalIgnoreCase) ||
type.Equals("string", StringComparison.OrdinalIgnoreCase) ||

Because OrdinalIgnoreCase is already used, these two lines are equivalent. The same redundancy appears for int/Int32, long/Int64, bool/Boolean, etc. This makes the function nearly twice as long as necessary without adding correctness. A HashSet<string>(StringComparer.OrdinalIgnoreCase) or a single pass with the alias already normalised would be cleaner.

---

Scoring gaps in GetGeneratedPowerShellExampleScore

if (required.Count <= 3)
    score += 8;
if (required.Count > 4)
    score -= (required.Count - 4) * 8;

When required.Count == 4, neither branch fires — the count sits in a gap that applies neither bonus nor penalty. This is probably intentional (a "neutral" sweet spot), but it reads like an accidental off-by-one. A short comment explaining that 4 is the neutral threshold would prevent future readers from treating it as a bug.

---

urlSamples.Add missing null/empty guard (WebApiDocsGenerator.ApiDocs.SourceValidation.cs)

if (urlSamples.Count < 4)
    urlSamples.Add(source.Url.Trim());

Unlike the RepoMismatchUrlSamples path which checks !string.IsNullOrWhiteSpace(urlSample) before appending, this call adds unconditionally. If source.Url is empty (valid after trim) an empty entry gets included in the warning message. A symmetric null/empty guard here would match the defensive pattern used elsewhere.

---

TryExtractGitHubRepoName — heuristic false-positive risk

The check correctly detects the common misconfiguration pattern (sourceRoot one level above a single-repo GitHub URL). One edge case: if a project legitimately structures its workspace so that the sourceRoot parent directory shares a name with a sibling folder that happens to match the GitHub repo name (but is unrelated), the warning fires incorrectly. Because this is surfaced only as a warning (not a hard failure) and the message clearly explains the fix, the UX impact is low — but it's worth noting in comments.

The test coverage for this feature is solid: both the CLI (WebCliApiDocsPreflightTests) and pipeline runner (WebPipelineRunnerApiDocsPreflightTests) paths are exercised.

---

SourceCoverageStats array reconstruction in a loop

stats.RepoMismatchUrlSamples = stats.RepoMismatchUrlSamples
    .Append(urlSample)
    .Distinct(StringComparer.OrdinalIgnoreCase)
    .Take(sampleLimit)
    .ToArray();

This allocates a new array on every iteration with a mismatch. sampleLimit is small (5), so the O(n²) cost is negligible in practice. Still, a List<string> local with a capacity check (as used for mismatchSamples immediately above) would be more consistent with the surrounding style.

---

Test quality

• Generate_PowerShellHelp_GeneratesFallbackExamplesFromBestParameterSets effectively validates the scoring preference for ByName/ById over ByInputObject, including the Credential exclusion. The assertions on both code and text kinds are thorough.
• The assertion that Assert.DoesNotContain(..."-InputObject"...) would be even more robust if it also asserted that "-Credential" is absent (since Credential is scored down but the ByInputObject set is the only one that carries it).
• GenerateDocsHtml_WarnsWhenSourceUrlPatternLikelyDuplicatesPathPrefix is updated to assert on example URLs in the warning text — good, as the new URL preview is the main value-add of that change.

---

Summary

Area	Verdict
Layout removal	✅ Clean, well-tested
Source preflight heuristic	✅ Solid, minor false-positive risk is acceptable
PowerShell fallback multi-example	✅ Good improvement; scoring logic is functional
IsPowerShellFriendlyExampleType	⚠️ Redundant comparisons inflate code size
urlSamples.Add guard	⚠️ Inconsistent with surrounding defensive patterns
Scoring gap at required.Count == 4	ℹ️ Intentional but undocumented

The PR is ready to merge once the minor items above are considered. No blocking issues found.

[claude]

Code Review

Overall this is a well-structured PR with good test coverage for each new feature. A few items worth addressing before merge.

Potential Bug: Missing warning code prefix

In WebApiDocsGenerator.Coverage.cs, AppendPowerShellExampleQualityWarnings emits a warning that reads "API docs PowerShell coverage: N command(s) rely only on generated fallback examples." But the new test Generate_PowerShellHelp_WarnsWhenCommandsRelyOnlyOnGeneratedFallbackExamples asserts the warning contains [PFWEB.APIDOCS.POWERSHELL]. The implementation appears to be missing that prefix, so the test should fail as written. Please confirm the test actually passes -- there may be additional code not shown in the diff that prepends the code.

HasOnlyGeneratedPowerShellFallbackExamples - fragile position-based detection

The method detects generated examples by checking whether the item at index i-1 is a text label starting with "Generated fallback example". If examples are ever sorted, filtered, or reordered upstream, this silently breaks. A more robust design would add an IsGeneratedFallback boolean to ApiExampleModel at the point of generation, rather than inferring it from list position at read time.

IsPowerShellFriendlyExampleType - use a HashSet instead of chained Equals calls

The method chains 30+ sequential .Equals() calls. A static readonly HashSet<string> with StringComparer.OrdinalIgnoreCase would be faster and far easier to maintain when the list needs updates.

RepoMismatchUrlSamples - array re-allocation in a loop

The code does stats.RepoMismatchUrlSamples = stats.RepoMismatchUrlSamples.Append(...).Distinct(...).Take(n).ToArray() on every iteration, allocating a new array each time. The surrounding code for InvalidUrlSamples and UnresolvedTemplateSamples collects into a local structure and converts once at the end. Making this consistent would be cleaner.

GetGeneratedPowerShellExampleScore - undocumented magic numbers

Constants like 14, 36, 10, 8, 28, 40, -16 have no comments explaining their rationale. Brief inline comments would help future maintainers calibrate scoring without reverse-engineering the intent.

TryExtractGitHubRepoName - assumes all template tokens are known

The function substitutes known tokens before calling Uri.TryCreate. Any future or unknown token (e.g. {branch}) left in the string could cause parse failure or produce an incorrect result. A comment noting this assumption, or stripping unrecognized {...} tokens with a regex fallback, would make this more resilient.

Positives

• The sourceRoot preflight heuristic is well-scoped: it only fires when sourceRoot exists on disk, sourceUrl targets github.com, {root} is absent from the URL, AND the repo subdirectory actually exists locally. Good false-positive avoidance.
• Multi-example-per-command generation with dedup via HashSet and a configurable limit (PowerShellFallbackExampleLimitPerCommand) is clean and correctly bounded.
• Test coverage is thorough: CLI preflight, pipeline preflight, PowerShell generator, HTML layout, and coverage report are all exercised with independent targeted tests.
• The HTML right-rail removal is guarded by Assert.DoesNotContain for all three removed content strings.
• SourceCoverageGroup is a clean minimal helper class.

Review generated with Claude Code

[claude]

Code Review

Overall this is a solid, well-structured PR. The feature scope is clear, tests are comprehensive, and the separation into partial classes is maintained consistently. A few items worth discussing:

---

Potential Issues

1. HasOnlyGeneratedPowerShellFallbackExamples — fragile string coupling

// WebApiDocsGenerator.Coverage.cs
var generated = i > 0 &&
    type.Examples[i - 1].Kind.Equals("text", StringComparison.OrdinalIgnoreCase) &&
    IsGeneratedPowerShellFallbackLabel(type.Examples[i - 1].Text);

private static bool IsGeneratedPowerShellFallbackLabel(string? text)
    => !string.IsNullOrWhiteSpace(text) &&
       text.TrimStart().StartsWith("Generated fallback example", StringComparison.OrdinalIgnoreCase);

The detection side string-matches its own output from the generation side. If the label prefix ever changes in BuildGeneratedPowerShellExampleLabel, HasOnlyGeneratedPowerShellFallbackExamples silently starts treating generated examples as authored (false negatives). A bool IsGenerated flag on ApiExampleModel — or any non-text sentinel — would make this contract explicit and safe to refactor.

---

2. RepoMismatchUrlSamples — O(n²) array re-materialization

// WebApiDocsGenerator.Coverage.cs
if (stats.RepoMismatchUrlSamples.Length < sampleLimit)
{
    stats.RepoMismatchUrlSamples = stats.RepoMismatchUrlSamples
        .Append(urlSample)
        .Distinct(StringComparer.OrdinalIgnoreCase)
        .Take(sampleLimit)
        .ToArray();   // called once per mismatch hit
}

.ToArray() re-materializes and re-deduplicates on every mismatch. The < sampleLimit guard keeps this bounded, but a HashSet<string> field (converted to array at the end) would be cleaner and consistent with how mismatchSamples is already handled in the same method.

---

3. IsPowerShellFriendlyExampleType — redundant type alias checks

type.Equals("String", StringComparison.OrdinalIgnoreCase) ||
type.Equals("string", StringComparison.OrdinalIgnoreCase) ||   // redundant — OrdinalIgnoreCase already covers this
type.Equals("Int32", StringComparison.OrdinalIgnoreCase) ||
type.Equals("int", StringComparison.OrdinalIgnoreCase) ||      // redundant
// ...

StringComparison.OrdinalIgnoreCase folds String/string, Int32/int, Boolean/bool, etc. The duplicate entries are harmless but add noise. Each pair can be reduced to one entry.

---

4. TryExtractGitHubRepoName — heuristic limitation

The function fills in template tokens with placeholder values before parsing the URL:

var candidate = pattern
    .Replace("{path}", "sample/path.cs", ...)
    // ...
if (!Uri.TryCreate(candidate, UriKind.Absolute, out var uri)) return false;
var segments = uri.AbsolutePath.Split('/');
repoName = segments[1];   // assumes layout: github.com/org/repo/...

This works for the common https://github.com/org/repo/blob/main/{path} pattern. However, if {path} is the first segment after the org (e.g., https://github.com/org/{path}#L{line} where the path includes the repo name), segments[1] would be "sample" rather than the real repo name, producing a false positive preflight warning. A comment documenting the assumed URL structure would help, and returning false when segments[1] matches a placeholder value (e.g., "sample") would be a simple safeguard.

---

Minor Observations

5. Magic scoring numbers — GetGeneratedPowerShellExampleScore and GetImportedPowerShellExampleScore use values like 140, 100, 36, 28, 40, 18, etc. with no comments on the rationale. These are hard to reason about or adjust later. Named constants (e.g., ExactCommandFileMatchBonus) or at least inline comments would help future maintainers.

6. GetImportedPowerShellExampleScore — undocumented line-number penalty

score -= Math.Max(0, lineNumber);

The intent (prefer examples near the top of a file) is reasonable but not obvious. A brief comment would clarify this isn't a bug.

7. NormalizePowerShellExampleToken — unnecessary char array allocation

var chars = value
    .Where(static ch => char.IsLetterOrDigit(ch))
    .Select(static ch => char.ToLowerInvariant(ch))
    .ToArray();
return new string(chars);

This allocates an intermediate char[]. For a method called in a scoring hot path, string.Create or a StringBuilder with AsSpan would avoid the allocation, though the current implementation is correct and unlikely to be a bottleneck in practice.

---

What's Working Well

• Test coverage is thorough and realistic — the XML help fixtures closely mirror real MAML output.
• The parameter-set scoring heuristic (preferring simple string/int params, deprioritizing InputObject/Credential/ScriptBlock) reflects real PowerShell UX conventions well.
• The [PFWEB.APIDOCS.POWERSHELL] warning separation from [PFWEB.APIDOCS.SOURCE] gives CI exactly the granularity needed to distinguish coverage categories.
• Removal of the right rail stats sidebar is a clean simplification; dead aliasCount/primaryKindCount locals are properly removed.
• SourceCoverageGroup is a nice lightweight record-like wrapper that avoids repeating group-label logic across multiple warning paths.

The two items I'd recommend addressing before merge: the string-coupling in HasOnlyGeneratedPowerShellFallbackExamples (#1) and the TryExtractGitHubRepoName heuristic limitation (#4), since both could cause silent incorrect behavior under non-obvious conditions. The rest are nice-to-haves.

[claude]

PR Review: Harden API docs layout and fallback generation

Overall this is a well-structured PR with solid test coverage for new features. The three areas of change (CSS/layout cleanup, source-link preflight, PowerShell fallback example quality) are each well-motivated and documented. Below are a few concerns worth addressing.

---

Potential Bugs

1. HasOnlyGeneratedPowerShellFallbackExamples — brittle sentinel heuristic

WebApiDocsGenerator.Coverage.cs (line ~984):

var generated = i > 0 &&
                type.Examples[i - 1] is not null &&
                type.Examples[i - 1].Kind.Equals("text", ...) &&
                IsGeneratedPowerShellFallbackLabel(type.Examples[i - 1].Text);
if (!generated)
    return false;

This relies on the generated text label physically preceding the code example in the Examples list. Two risks:

• When i == 0 and the first example is a code block (which shouldn't happen for generated examples, but is possible for authored ones), the logic short-circuits correctly — but this is an implicit assumption with no guard.
• If BuildGeneratedPowerShellExampleLabel ever changes its prefix (or IsGeneratedPowerShellFallbackLabel's prefix check diverges), the classification silently breaks in production without any test failure.

Suggestion: Add a dedicated bool IsGeneratedFallback property to ApiExampleModel and set it at generation time. This removes the string-matching dependency entirely.

---

2. GetImportedPowerShellExampleScore — misleading parameter name

score -= Math.Max(0, lineNumber);

The parameter is named lineNumber but is actually the snippet's index within the collected snippets list (not a true line number in the file). The intent (prefer earlier-appearing snippets) is sound, but the name suggests something different. Renaming to snippetIndex would clarify.

---

Code Quality

3. IsPowerShellFriendlyExampleType — redundant comparisons

The method lists both "String" and "string" as separate checks with OrdinalIgnoreCase, making the second check unreachable dead code:

type.Equals("String", StringComparison.OrdinalIgnoreCase) ||
type.Equals("string", StringComparison.OrdinalIgnoreCase) ||  // redundant

This pattern repeats for Int32/int, Double/double, bool/Boolean, etc. A HashSet<string>(StringComparer.OrdinalIgnoreCase) initialized once would eliminate ~30 duplicate comparisons and be easier to extend.

---

4. NormalizePowerShellExampleToken — LINQ char allocation

var chars = value
    .Where(static ch => char.IsLetterOrDigit(ch))
    .Select(static ch => char.ToLowerInvariant(ch))
    .ToArray();
return new string(chars);

This allocates an intermediate char[]. For a scoring helper called per file × per snippet × per command, a StringBuilder or string.Create approach would avoid the extra allocation. Minor, but worth noting since scoring runs in a tight loop.

---

5. RepoMismatchUrlSamples accumulation — O(n²) pattern

In WebApiDocsGenerator.Coverage.cs:

stats.RepoMismatchUrlSamples = stats.RepoMismatchUrlSamples
    .Append(urlSample)
    .Distinct(StringComparer.OrdinalIgnoreCase)
    .Take(sampleLimit)
    .ToArray();

This re-enumerates and allocates a new array on every hit. Given sampleLimit is small this won't matter in practice, but it's inconsistent with the HashSet-based approach used for mismatchSamples right above it. A HashSet for URL samples too would be cleaner.

---

6. Undocumented magic scoring numbers

GetGeneratedPowerShellExampleScore uses values like 14, 36, 10, 8, and GetImportedPowerShellExampleScore uses 140, 100, 35, 70, 18 without any comment explaining the rationale. When someone needs to adjust the heuristics later, it won't be obvious why those numbers were chosen or how they relate to each other. Even a short comment per scoring block ("prefer parameter sets with exactly 1 required param") would help.

---

Missing Test Coverage

• maxPowerShellGeneratedFallbackOnlyExamplePercent (percent-based threshold) has no test — only the count variant (maxPowerShellGeneratedFallbackOnlyExampleCount) is exercised in the new pipeline test.
• Mixed authored + generated examples: no test verifies that a command with at least one authored example is not flagged as generated-only (i.e., that HasOnlyGeneratedPowerShellFallbackExamples returns false when authored examples are present alongside generated ones).

---

Minor Nits

• WebPipelineRunner.Tasks.Content.cs — the new TryExtractGitHubRepoName is private and well-covered by the preflight tests, but a dedicated unit test would make the URL-parsing edge cases (non-GitHub hosts, <2 path segments, template tokens in unexpected positions) explicit.
• The CSS selector list change (removing .api-overview-grid, adding .pf-combobox-*, .type-toc-*, etc.) is correctly reflected in both the production code and the contract test — good catch keeping those in sync.

---

Summary: The feature logic is sound and tests are thorough for the happy paths. The main actionable items are the sentinel-string fragility in HasOnlyGeneratedPowerShellFallbackExamples (#1) and the redundant IsPowerShellFriendlyExampleType comparisons (#3). The rest are lower-priority polish items.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 92ec9807bf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Reject max percent thresholds above 100

maxPowerShellGeneratedFallbackOnlyExamplePercent is documented and treated as a percentage, but AddCoverageMaxThreshold only enforces >= 0, so values like 150 are silently accepted and make the gate effectively meaningless instead of surfacing a config error. This creates a false sense of coverage enforcement in CI when users mistype or misunderstand the threshold range.

Useful? React with 👍 / 👎.

[PrzemyslawKlys]

Added another API-docs hardening pass to this branch:

• track PowerShell example provenance in generated JSON via example.origin
• split coverage into authoredHelpCodeExamples / importedScriptCodeExamples / generatedFallbackCodeExamples
• keep generatedFallbackOnlyExamples as the CI guardrail
• add regression coverage for authored vs imported vs generated example buckets

Verification:

• dotnet test .\\PowerForge.Tests\\PowerForge.Tests.csproj -c Release --filter "FullyQualifiedName~WebApiDocsGeneratorPowerShellTests|FullyQualifiedName~WebApiDocsGeneratorSourceAndCssTests|FullyQualifiedName~WebPipelineRunnerApiDocsPreflightTests|FullyQualifiedName~WebCliApiDocsPreflightTests"
• Passed: 39, Failed: 0

[claude]

PR Review: Harden API docs layout and fallback generation

Overall this is a well-structured PR with good test coverage. The three feature areas (layout cleanup, source-link preflight, PowerShell example improvements) are logically grouped. A few observations below.

---

Potential Bug: Missing warning code prefix

In WebApiDocsGenerator.Coverage.cs, AppendPowerShellExampleQualityWarnings emits a warning string that does not include [PFWEB.APIDOCS.POWERSHELL]. But the test Generate_PowerShellHelp_WarnsWhenCommandsRelyOnlyOnGeneratedFallbackExamples asserts the warning does contain that code. If the test passes, there must be a second code path outside the visible diff that emits the prefix. If not, this is a failing test worth verifying before merge.

---

Minor: Redundant comparisons in IsPowerShellFriendlyExampleType

All comparisons already use StringComparison.OrdinalIgnoreCase, so paired entries like "String" / "string" are redundant. The same duplication repeats for int/Int32, bool/Boolean, double/Double, etc. Each type only needs one entry.

---

Performance: Array re-creation in AnalyzeSourceCoverage

RepoMismatchUrlSamples is rebuilt into a new array on every matching hit:

stats.RepoMismatchUrlSamples = stats.RepoMismatchUrlSamples
    .Append(urlSample).Distinct(...).Take(sampleLimit).ToArray();

For large symbol sets this is O(n^2). A List<string> with a count guard converted to array once at the end would be more efficient. InvalidUrlSamples and UnresolvedTemplateSamples share the same pattern.

---

Robustness: TryExtractGitHubRepoName placeholder substitution

The method replaces {path}, {root}, etc. with dummy strings to form a parseable URI. If any template token appears in the query string or fragment, the result could parse fine but yield the wrong segments[1]. The heuristic is sound for common cases; a short comment explaining the intentional approximation would help future maintainers.

---

UX: Overview rail removal

The removed right rail provided at-a-glance module stats and a namespace jump-list useful for large modules. The removal is intentional per the PR summary. A follow-up to surface the namespace jump-list in the existing sidebar filter rail may be worth tracking for large-module UX.

---

Positives

• Test coverage is thorough - every new code path has a dedicated integration test with proper try/finally cleanup.
• origin field on examples is a clean, forward-compatible design that avoids polluting the main model.
• ApiExampleOrigins constants class prevents stringly-typed origin values scattering across the codebase.
• Scoring system for fallback examples is pragmatic: favouring Name/Path/Id parameters and penalising InputObject/Credential/ScriptBlock aligns well with PowerShell community conventions.
• The BuildGeneratedPowerShellExamples (multi) / BuildGeneratedPowerShellExample (single) split is a clean refactor that keeps responsibilities clear.
• maxPowerShellGeneratedFallbackOnlyExamplePercent / Count pipeline thresholds give CI a useful quality gate that was previously missing.

[claude]

PR Review: Harden API docs layout and fallback generation

Overall this is a well-structured PR with good feature cohesion and solid test coverage. The three pillars - layout cleanup, preflight heuristics, and example provenance tracking - are all independently useful.

Potential bug: bool missing from GetPowerShellSampleValue

The PR adds lowercase aliases for numeric types (int, long, uint, short, byte, sbyte, etc.) but does not add 'bool' to the Boolean/Bool branch in GetPowerShellSampleValue. IsPowerShellFriendlyExampleType treats 'bool' as friendly (correct), but GetPowerShellSampleValue will not emit dollar-true for it. A generated example like 'Command -IsEnabled IsEnabled' would look odd. Suggest adding type.Equals('bool', StringComparison.OrdinalIgnoreCase) alongside the existing 'Boolean' and 'Bool' checks.

Dead code in IsPowerShellFriendlyExampleType

Since all comparisons already use StringComparison.OrdinalIgnoreCase, the explicit lowercase duplicates are redundant - Equals('String', OrdinalIgnoreCase) and Equals('string', OrdinalIgnoreCase) produce identical results. This applies to string, int, long, uint, ulong, short, ushort, byte, sbyte, double, float, decimal, bool, guid, uri, datetime. Not a runtime bug, but it nearly doubles the method length and makes auditing harder.

Magic numbers in scoring functions

GetGeneratedPowerShellExampleScore and GetImportedPowerShellExampleScore use unexplained constants (14, 36, 10, 8, 140, 100, 35, 18, etc.). The relative weights encode meaningful domain logic - exact filename match is stronger than noun match, which is stronger than path match - but a future reader has no way to understand why 140 vs 100 vs 35. Brief inline comments explaining the rationale would help maintainability.

api-overview-main--full not in CSS contract

The HTML builder now emits an api-overview-main--full modifier class alongside api-overview-main. That modifier is absent from the required-CSS-selectors list in WebApiDocsGenerator.cs and from the contract test, so a theme that omits it will not be warned. Intentional or oversight?

CSS breaking change for existing themes

Removing .api-overview-grid from the required selectors and deleting the right-rail HTML is correct for the new layout, but is a silent breaking change for anyone with a custom CSS file that sizes or hides that grid. Worth a note in the changelog.

Minor: Assert.Single vs Assert.Equal(1)

Generate_PowerShellHelp_PrefersCommandSpecificExampleScriptsOverGenericOnes uses Assert.Single(codeExamples) while surrounding tests use Assert.Equal(N, codeExamples.Length). Both work, just inconsistent style within the same file.

Strengths

• Provenance tracking (origin field) is emitted conditionally (only when non-null), keeping the JSON schema backward-compatible for existing consumers.
• SerializeExamples eliminates five copy-pasted inline projections - good deduplication.
• Preflight heuristic for sourceRoot one level above repo is a genuinely useful guard with a clear, actionable warning message.
• Test coverage is thorough: each new code path has a dedicated fact test, temp dirs are cleaned in finally blocks, and assertions check both content and origin together.

The bool sample-value gap and the redundant OrdinalIgnoreCase entries are the only items I would want addressed before merge; the rest are suggestions.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12a08a23a3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Enforce maxPerCommand during candidate collection

The refactor now delays populating results until after all files are scanned, but the scan loop still uses results.TryGetValue(...existing.Count >= maxPerCommand) as its early-exit guard. In practice this means the configured maxPerCommand no longer limits work while parsing, so large Examples/ trees can accumulate every matching snippet in memory even when the limit is 1, causing avoidable CPU/memory growth in CI and local builds. Cap commandCandidates (or otherwise enforce the limit) during collection rather than only in the final projection step.

Useful? React with 👍 / 👎.

[PrzemyslawKlys]

Added the next API-docs foundation pass to this branch:

• first-class example media contract in the engine (examples[].media)
• XML-doc authoring support for <image>, <video>, <terminal>, and <media kind="..."> inside <example> blocks
• docs-template rendering + fallback styling for example media
• regression test covering JSON + HTML output for XML example media

Verification:

• dotnet test .\\PowerForge.Tests\\PowerForge.Tests.csproj -c Release --filter "FullyQualifiedName~WebApiDocsGeneratorPowerShellTests|FullyQualifiedName~WebApiDocsGeneratorSourceAndCssTests|FullyQualifiedName~WebPipelineRunnerApiDocsPreflightTests|FullyQualifiedName~WebCliApiDocsPreflightTests"
• Passed: 41, Failed: 0

[claude]

PR Review: Harden API docs layout and fallback generation

Overall this is a well-structured PR with clear goals and solid test coverage.

---

Positive highlights

• Example provenance (origin field) is a great addition. Distinguishing AuthoredHelp / ImportedScript / GeneratedFallback enables downstream CI quality gates that are much more meaningful than a simple "has examples" boolean.
• AppendPowerShellExampleQualityWarnings and the matching [PFWEB.APIDOCS.POWERSHELL] warning give CI clear signal when a module ships with only machine-generated examples.
• The sourceRoot one-level-above preflight check (TryExtractGitHubRepoName) is pragmatic and covered in both CLI and pipeline runner tests.
• SerializeExamples cleanly centralises the repetitive inline projection that was duplicated six times -- good refactor.
• Test coverage is thorough: every new feature has at least one dedicated [Fact] exercising the happy path and the coverage/warning surface.

---

Issues and suggestions

1. Magic numbers in scoring functions are hard to maintain

GetGeneratedPowerShellExampleScore and GetImportedPowerShellExampleScore use raw literals (140, 100, 35, 70, 36, 28, 40, etc.) without any rationale. Suggestion: add a brief comment block explaining the relative weighting intent (e.g. "exact filename match outweighs path match by 4x") so future maintainers can reason about the trade-offs without reverse-engineering the numbers.

2. IsPowerShellFriendlyExampleType -- long OR-chain is fragile and slow

The method has ~30 individual type.Equals(...) branches. A HashSet<string>(StringComparer.OrdinalIgnoreCase) initialised once as private static readonly would be O(1) lookup, shorter, and easier to extend. The array-suffix recursion for handling string[] is fine as-is.

3. URL scheme not validated when rendering media HTML

In AppendExampleMedia, media URLs come from XML doc attributes and are HTML-encoded but not scheme-validated. HtmlEncode does not strip javascript: or data: URIs. If the doc XML is ever from a partially-trusted source, a <image src="javascript:..." /> would pass through. Consider a simple allowlist guard (permit /, https://, http://) before emitting src and href attributes.

4. RepoMismatchUrlSamples accumulation creates repeated allocations

The outer Length < sampleLimit guard is redundant with the inner .Take(sampleLimit), and the pattern creates a new array on every mismatch hit. For large assemblies with many mismatches this creates unnecessary GC pressure. A List<string> with an early-return guard (consistent with how mismatchSamples just above it is handled) would be cleaner.

5. CSS breaking change -- needs a migration note

Removing .api-overview-grid from the required CSS contract and adding several new selectors is a breaking change for anyone with a custom CssHref stylesheet. The docs already list the new selectors, but a prominent migration callout would help users know they need to update stylesheets when upgrading.

6. Test: intent of ByInputObject exclusion could be clearer

The DoesNotContain(-InputObject) assertion is the key behaviour being tested (complex parameter type = lower score). A one-line comment explaining why InputObject is excluded would make the test self-documenting for future readers.

7. Minor: score -= Math.Max(0, lineNumber) -- verify index convention

If the caller passes a 0-based index, line 0 incurs no penalty (correct). If it passes a 1-based index, even the first line gets a -1 penalty. Worth a quick check that the convention is consistent at all call sites.

---

Summary

Area	Assessment
Code quality	Good overall; scoring magic numbers need comments
Potential bugs	URL scheme bypass in media HTML; minor GC churn in RepoMismatchUrlSamples
Performance	IsPowerShellFriendlyExampleType could use a HashSet; otherwise fine for doc-gen
Security	Low risk in typical use; javascript: URL passthrough worth hardening
Test coverage	Excellent -- every new feature has dedicated tests including coverage/warning signal
Breaking changes	CSS contract change needs a clear migration note

Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bc3af3c468

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Propagate video MIME type into rendered source tags

TryParseExampleMedia captures mimeType/content-type into ApiExampleMediaModel.MimeType, but the video renderer drops that metadata and emits <source> without a type attribute. For video examples served from extensionless URLs (signed/CDN endpoints) or multi-format assets, browsers can fail codec detection and not play the example even though authors provided an explicit MIME type. Add the type attribute when media.MimeType is present so authored metadata actually influences playback.

Useful? React with 👍 / 👎.

[PrzemyslawKlys]

Added the next PowerShell docs-quality pass to this branch.

What landed:

• first-class PowerShell example validation in PowerForge.Web, backed by the out-of-process PowerShell parser
• optional �pidocs CLI + pipeline validation/reporting/fail gates
• a dedicated powershell-example-validation.json report (default under API output)
• [PFWEB.APIDOCS.POWERSHELL] warnings for invalid imported scripts and scripts that do not reference documented commands
• tighter example discovery so detached/temp layouts do not accidentally scan unrelated parent Examples/ folders

Focused verification passed:

• dotnet test .\PowerForge.Tests\PowerForge.Tests.csproj -c Release --filter "FullyQualifiedNameWebApiDocsGeneratorPowerShellTests|FullyQualifiedNameWebPipelineRunnerApiDocsPreflightTests|FullyQualifiedNameWebCliApiDocsPreflightTests|FullyQualifiedNameWebApiDocsGeneratorSourceAndCssTests"
• Passed: 44, Failed: 0

[claude]

test comment please ignore

[claude]

Code Review: PR 226 - Harden API docs layout and fallback generation

Overall this is a well-structured PR with clear intent and good test coverage. Three main areas: PowerShell fallback scoring, source-link preflight checks, and HTML/CSS layout hardening. Below are observations grouped by severity.

---

MEDIUM PRIORITY

1. Unbounded lineNumber score penalty in GetImportedPowerShellExampleScore

score -= Math.Max(0, lineNumber) gives a snippet at line 500 a penalty of -500, swamping positive signals. Clamping to Math.Min(lineNumber, 50) would be more robust.

2. Logic bug in CLI result-reconstruction condition (WebCliCommandHandlers.DocsCommands.cs)

result.PowerShellExampleValidationPath is assigned just before the reconstruction check, so the string equality always returns false and the result is always reconstructed when validation runs. Fix with: bool needsReconstruct = filteredWarnings.Length != generatorWarnings.Length || powerShellExampleValidation is not null;

3. O(n^2) array reallocation in RepoMismatchUrlSamples (WebApiDocsGenerator.Coverage.cs)

The .Append().Distinct().Take().ToArray() chain inside the source-link loop re-allocates on every iteration. Use a HashSet as intermediate storage and convert at the end - same pattern as mismatchSamples nearby.

---

LOW PRIORITY

4. Magic scoring numbers without named constants

Raw integers (14, 36, 10, 140, 100, 35, 70...) in the scoring methods are load-bearing heuristics with no explanation. Extract to private const int fields with comments.

5. Redundant case variants and missing types in IsPowerShellFriendlyExampleType

Both String and string are listed despite OrdinalIgnoreCase making lowercase aliases dead code. Also char is absent despite other primitives being covered, and nullable types and multi-dimensional arrays are unhandled.

6. Inconsistent urlSamples collection vs. display limits

Up to 4 entries collected (Count < 4) but only 2 shown (Take(2)). Align these limits.

7. PreferPwsh not exposed in pipeline/CLI configuration

PreferPwsh defaults to true but cannot be overridden from pipeline JSON or CLI flags. Users on Windows who need Windows PowerShell 5.1 have no way to opt out.

8. BuildMinimalPowerShellHelpForValidation duplicated across two test classes

Identical private helpers in WebApiDocsGeneratorPowerShellTests and WebPipelineRunnerApiDocsPreflightTests. Extract to a shared TestHelpers class.

9. No test for maxPowerShellGeneratedFallbackOnlyExamplePercent threshold path

The formatAsPercent fix to AddCoverageMaxThreshold is untested. A test passing a percent value would close this gap.

10. Rail removal is a breaking change for custom CSS

Removing .api-overview-grid, .api-overview-rail, and stat-classes will break user CSS targeting those selectors. Worth a changelog note.

---

INFO / SUGGESTIONS

11. docs in ShouldProbePowerShellHelpParentDirectory is overly broad; cultures are already handled by the regex.

12. baseUrl/slugMap not applied to media src/href in AppendExampleMedia; relative paths may produce broken links.

13. The rail-removal test only checks human-readable strings. Also assert DoesNotContain api-overview-rail to catch structural regressions.

14. The embedded PowerShell validation script cannot be linted by PSScriptAnalyzer. Consider a unit test that parses it with Parser::ParseInput.

---

WHAT IS DONE WELL

• The origin provenance field on all example entries is excellent for debugging which path generated a snippet.
• The out-of-process PowerShell validation approach (separate process, temp dir, configurable timeout) is the right architecture.
• The ConvertTo-Json single-element workaround via @results is a thoughtful defensive measure.
• Light-theme CSS overrides for new media classes are complete and mirror dark-mode defaults.
• The Assert.All origin assertion in the PowerShell tests is a good cross-cutting safety check.

---

Automated review generated by Claude - please verify suggestions against project context before acting on them.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ae587dd06

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Mark files invalid when PowerShell validation process fails

When the external parser run fails (ValidationSucceeded == false, e.g., pwsh missing, timeout, or non-zero exit), parseErrorsByFile is empty and each file is still recorded as ValidSyntax = true because validity is inferred only from parseErrors.Length == 0. This makes the generated report claim all scripts are syntactically valid even though validation did not complete, which can produce false-positive quality signals whenever failOnPowerShellExampleValidation is not enabled.

Useful? React with 👍 / 👎.

[PrzemyslawKlys]

Added opt-in git freshness metadata for API docs in commit 0fc2cd4.

What’s included:

• engine-native freshness model + JSON output (freshness.status, lastModifiedUtc, commitSha, ageDays, sourcePath)
• docs-template freshness badges for sidebar, quick-start cards, namespace chips, and type detail headers
• CLI + pipeline wiring (--git-freshness, gitFreshnessNewDays, gitFreshnessUpdatedDays)
• pipeline schema + docs updates
• git-backed regression coverage for generator + pipeline PowerShell flows

Verification passed:
dotnet test .\\PowerForge.Tests\\PowerForge.Tests.csproj -c Release --filter "FullyQualifiedName~WebApiDocsGeneratorPowerShellTests|FullyQualifiedName~WebPipelineRunnerApiDocsPreflightTests|FullyQualifiedName~WebCliApiDocsPreflightTests|FullyQualifiedName~WebApiDocsGeneratorSourceAndCssTests"

Result: 46 passed, 0 failed.

Note: unrelated local edits in PowerForge.Web/Services/ReleaseHubRenderer.cs and PowerForge.Tests/WebReleaseHubRenderingTests.cs remain untouched and unstaged.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 56faf9dfdf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Make execution failure flag imply example execution

This branch turns on validation when failOnPowerShellExampleExecution is set, but it leaves executePowerShellExamples unchanged. In that configuration the validator runs without execution, ExecutionCompleted remains false, and the later gate throws on !ExecutionCompleted, so the step fails unconditionally even when all scripts are syntactically valid. This makes failOnPowerShellExampleExecution: true unusable unless callers also discover they must set executePowerShellExamples: true separately.

Useful? React with 👍 / 👎.

[PrzemyslawKlys]

This pass turns playback asset-health warnings into measurable coverage + CI gates.

What changed:

• internal example media now carries health flags for:
  • unsupported sidecars
  • oversized assets
  • stale assets
• coverage now reports command-level playback health metrics:
  • importedScriptPlaybackMediaUnsupportedSidecars
  • importedScriptPlaybackMediaOversizedAssets
  • importedScriptPlaybackMediaStaleAssets
• pipeline thresholds can now gate those issues with:
  • maxPowerShellImportedScriptPlaybackMediaUnsupportedSidecarCount
  • maxPowerShellImportedScriptPlaybackMediaOversizedAssetCount
  • maxPowerShellImportedScriptPlaybackMediaStaleAssetCount
• docs/schema were updated so the new controls are supported and discoverable

Coverage/tests added:

• generator regression now asserts asset-health coverage payloads for unsupported/oversized/stale playback assets
• pipeline regression now fails on stale playback media when threshold is set to 0

Verification:

• dotnet test .\\PowerForge.Tests\\PowerForge.Tests.csproj -c Release --filter "FullyQualifiedName~WebApiDocsGeneratorPowerShellTests|FullyQualifiedName~WebPipelineRunnerApiDocsPreflightTests|FullyQualifiedName~WebCliApiDocsPreflightTests|FullyQualifiedName~WebApiDocsGeneratorSourceAndCssTests|FullyQualifiedName~WebApiDocsGeneratorContractTests"
• Passed: 71, Failed: 0

[claude]

PR Review: Harden API docs layout and fallback generation

Overall this is a solid, well-structured PR. The three stated goals are achieved cleanly, test coverage is substantial, and the documentation is thorough. Below are observations grouped by severity.

Bugs / Correctness

1. int.MinValue propagates through Sum() - potential overflow

GetGeneratedPowerShellExampleParameterScore returns int.MinValue for a null parameter. In GetGeneratedPowerShellExampleScore, this feeds into required.Sum(GetGeneratedPowerShellExampleParameterScore). Enumerable.Sum uses checked arithmetic in .NET 6+; summing two int.MinValue entries throws OverflowException. The required list is filtered from method.Parameters.Where(...), but if the Parameters collection itself contains nulls (e.g. from a malformed MAML document), this throws instead of returning a low score.

Fix: either guard against nulls in the Parameters collection upstream, or use int.MinValue / 2 (or a named constant like VeryUnfavorableScore = -10000) as the sentinel.

2. ScriptBlock type gets +12 but ScriptBlock parameter name gets -28

A parameter typed and named ScriptBlock nets -16. That is probably intentional, but the interaction between IsPowerShellFriendlyExampleType and the name-based penalty is not obvious. A comment would make the intent explicit.

Code Quality

3. Redundant aliases in IsPowerShellFriendlyExampleType

Every comparison already uses StringComparison.OrdinalIgnoreCase, so paired entries like String/string, bool/Bool, Guid/guid, etc. are dead code. A static readonly HashSet with StringComparer.OrdinalIgnoreCase would be O(1), remove ~15 duplicate entries, and be easier to extend.

4. Magic numbers in the scoring algorithm

Raw integers (14, 36, 10, 8, 28, 40, 12, 16, 6, 30, 22) with no explanatory names or comments make the algorithm hard to maintain. Named constants or a block comment explaining the scale would help.

5. IsPowerShellFriendlyExampleType returns true for null/empty types

An untyped parameter competes equally with a well-typed string, meaning unresolved/partial MAML entries silently receive a friendly-type bonus. Returning false for untyped parameters would make scoring more conservative and correct.

Documentation Inconsistency

6. Doc says powerShellFallbackExampleLimit, code uses PowerShellFallbackExampleLimitPerCommand

Docs/PowerForge.Web.ApiDocs.md references powerShellFallbackExampleLimit as the user-facing pipeline key, but the C# property is PowerShellFallbackExampleLimitPerCommand. The doc should match the exact key name readers will type in their YAML.

Minor Observations

7. TryExtractGitHubRepoName token-substitution is fragile

Works for the common case (https://github.com/owner/repo/blob/main/path), but breaks if the root token appears in the owner segment. In that case segments[1] returns the placeholder SampleRepo instead of the real repo name, silently missing the misconfiguration. Low risk since this is only a preflight warning, but worth a comment.

8. Test gap: PowerShellFallbackExampleLimitPerCommand = 0 boundary

The tests cover limits of 1 and 2. A limit of 0 is guarded by limit <= 0 returning empty, but no test asserts that limit=0 produces zero examples without crashing. Simple to add.

Summary

The int.MinValue/Sum() overflow is the only item I would fix before merging. The rest are polish. Nice work overall - the scoring heuristic is well-thought-out and the test suite is comprehensive.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 61e907baa2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Make fail-on-validation enable PowerShell validation

failOnPowerShellExampleValidation is parsed, but validatePowerShellExamples is only auto-enabled for execution flags. In a pipeline that sets only failOnPowerShellExampleValidation: true, validation never runs (if (apiType == PowerShell && validatePowerShellExamples) is skipped), so syntax issues in imported scripts are silently ignored and CI can pass with a false sense of enforcement. The fail flag should imply validation, analogous to the execution path.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Keep media health flags when a script is reused

When the same example .ps1 is encountered again, this early return forces HasUnsupportedSidecars/HasOversizedAssets/HasStaleAssets to false for later commands that reuse that script. Multi-command example files are common, so only the first command gets health flags while the rest are underreported. That skews coverage metrics (importedScriptPlaybackMedia*) and can let max-threshold quality gates pass even though multiple commands share unhealthy media assets.

Useful? React with 👍 / 👎.

[PrzemyslawKlys]

This pass makes generated example media more trustworthy to readers by surfacing capture recency.

What changed:

• generated/imported PowerShell terminal media now carries:
  • capturedAtUtc
  • sourceUpdatedAtUtc
• JSON only emits those fields when PowerForge actually knows them (no always-null noise)
• docs-template HTML now renders a subtle media metadata line for generated terminal examples
• stale captures now show a reader-facing note: Script changed after this capture
• .example-media-meta is now part of the API docs CSS contract + fallback styling

Coverage/tests added:

• transcript and playback tests now assert recency fields exist for generated PowerShell media
• stale playback HTML test now verifies the user-facing stale note
• XML-authored media regression confirms we do not emit capture metadata when it was not generated by PowerForge

Verification:

• dotnet test .\\PowerForge.Tests\\PowerForge.Tests.csproj -c Release --filter "FullyQualifiedName~WebApiDocsGeneratorPowerShellTests|FullyQualifiedName~WebPipelineRunnerApiDocsPreflightTests|FullyQualifiedName~WebCliApiDocsPreflightTests|FullyQualifiedName~WebApiDocsGeneratorSourceAndCssTests|FullyQualifiedName~WebApiDocsGeneratorContractTests"
• Passed: 71, Failed: 0

[claude]

PR Review: #226 – Harden API docs layout and fallback generation

Overall this is a well-structured, thoughtfully documented PR that adds significant capability (git freshness metadata, PowerShell example provenance/validation/execution, richer media nodes). Test coverage is solid and the documentation is thorough. A few items are worth addressing before merge:

---

🔴 Potential Issues

1. Blocking async calls in synchronous context (Freshness.cs)

// WebApiDocsGenerator.Freshness.cs – TryGetGitFreshness
var topLevel = gitClient.ShowTopLevelAsync(directory).GetAwaiter().GetResult();
// ...
var log = gitClient.RunRawAsync(...).GetAwaiter().GetResult();

Using .GetAwaiter().GetResult() on async methods inside a sync context risks thread-pool starvation (classic deadlock scenario when called from e.g. ASP.NET SynchronizationContext). Since Generate() is the top-level call and is already synchronous, either:

• Make TryGetGitFreshness fully async and await it through the call chain, or
• Add a synchronous RunRaw / ShowTopLevel overload to GitClient to avoid wrapping async code.

2. sourcePath leaked in freshness JSON output

// BuildFreshnessJson
["sourcePath"] = freshness.SourcePath  // absolute local filesystem path

The sourcePath field emits the absolute path on the build machine (e.g. C:\Users\ci\src\MyLib\Foo.cs) into published JSON files that end-users can fetch. This is an information-disclosure risk. Consider stripping it from published output (keep it internal) or making it opt-in. The commitSha is sufficient for traceability without exposing the build tree layout.

---

🟡 Design Concerns

3. Internal asset-health flags on ApiExampleMediaModel

public bool HasUnsupportedSidecars { get; set; }
public bool HasOversizedAssets { get; set; }
public bool HasStaleAssets { get; set; }

These are build-time quality flags that should live in the coverage/validation layer, not on the serialized model. They will be serialized into the published examples JSON payload, surfacing internal build metadata to consumers. Recommend moving them to a separate health-tracking record and not including them in the output JSON.

4. Breaking CSS contract change without a migration note

The PR removes .api-overview-grid from the required CSS selector contract and adds .namespace-group-header, .namespace-group-actions, .overview-group-toggle, .pf-combobox*, .type-toc*, .member-toggle input. Teams with custom fallback.css overrides will get new [PFWEB.APIDOCS.CSS] warnings after upgrading. A brief "CSS contract migration" note in the changelog or warning message would help.

5. gitFreshnessNewDays > gitFreshnessUpdatedDays silently swaps thresholds

// Freshness.cs
var updatedDays = Math.Max(options.GitFreshnessNewDays, options.GitFreshnessUpdatedDays);
var newDays = Math.Clamp(options.GitFreshnessNewDays, 0, updatedDays);

If a user configures gitFreshnessNewDays: 60, gitFreshnessUpdatedDays: 30, this silently re-interprets them rather than emitting a warning. A [PFWEB.APIDOCS.FRESHNESS] warning when newDays > updatedDays would surface the misconfiguration.

---

🟢 Observations / Minor

6. Duplicate TryDeleteDirectory implementations

There are now two TryDeleteDirectory definitions in the test assembly – one in WebApiDocsGeneratorPowerShellTests.cs (refactored in this PR) and one inline in WebPipelineRunnerApiDocsPreflightTests.cs. Looks like consolidation happened for some test classes but not all; a shared test helper would clean this up.

7. PowerShell execution scope is broad

The executePowerShellExamples feature runs imported .ps1 files with access to the host process environment. The scripts are opted in by matching documented commands, but there's no sandbox. This is clearly opt-in and documented, but it's worth a note in the [PFWEB.APIDOCS.POWERSHELL] warning when execution is enabled: something like "execution runs scripts in the current user context without sandboxing."

8. filesElement.ValueKind == JsonValueKind.Object fallback path

var fileEntries = filesElement.ValueKind == JsonValueKind.Array
    ? filesElement.EnumerateArray().ToArray()
    : new[] { filesElement };  // treats a single object as an array of one

The PowerShell script always emits @($results) which guarantees an array even for a single result. The object fallback is a safety net, but the behavior of treating an unexpected object as a single-element array may silently swallow a malformed response. At minimum a comment explaining why this is safe would help future readers.

---

✅ What's done well

• Provenance tracking (AuthoredHelp / ImportedScript / GeneratedFallback) is a clean addition that makes coverage reporting meaningful and visible to readers.
• Scoring/deduplication for imported example candidates is a solid improvement over the previous first-match strategy.
• ShouldProbePowerShellHelpParentDirectory correctly restricts the parent-directory probe to culture folders and known doc roots, fixing a latent over-collection issue.
• Preflight sourceRoot heuristic (detecting parent-of-repo misconfiguration) is a high-signal diagnostic that addresses a real common mistake.
• Test coverage is thorough – freshness, validation, preflight, CSS contract, and execution all have targeted tests.
• The PowerShell validation script uses the AST parser ([System.Management.Automation.Language.Parser]::ParseFile) rather than executing code for validation – correct approach.
• GetImportedPowerShellExampleScore naming approach for command-specific file preference is a nice improvement over the old implicit ordering.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ff428c8893

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Resolve freshness candidates relative to sourceRoot

This path resolution uses Path.GetFullPath(path) without anchoring to sourceRoot, but API source paths are emitted as source-root-relative values (see BuildSourceLink in WebApiDocsGenerator.ApiDocs.Models.cs). When docs are generated from a different working directory (for example, a website repo that points sourceRoot at a sibling code repo), these candidates resolve to the wrong filesystem location, so real source files are skipped and freshness falls back to XML/help files, producing incorrect per-type freshness badges and metadata.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Make fail-on execution imply PowerShell example execution

This branch only forces validation when --fail-on-ps-example-execution is set, but it does not enable executePowerShellExamples. In that configuration validation runs without execution, ExecutionCompleted remains false, and the later fail gate (failOnPowerShellExampleExecution) fails unconditionally even if scripts are syntactically valid, making the fail-on-execution flag unusable unless users also discover they must pass --execute-ps-examples.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Make fail-on validation imply PowerShell validation

The CLI parses --fail-on-ps-example-validation, but the failure check is guarded by validatePowerShellExamples; if users set only the fail flag, validation never runs and invalid imported scripts are silently ignored. That creates a false-positive pass for CI/automation that expects the fail flag alone to enforce script validation.

Useful? React with 👍 / 👎.

[PrzemyslawKlys]

This pass adds a reusable manifest for imported PowerShell example media.

What changed:

• API docs generation now writes powershell-example-media-manifest.json when imported terminal media exists
• the manifest summarizes media linkage per command, including:
  • command/type identity
  • media URL + poster URL
  • source script path
  • staged asset/poster paths
  • capture/source timestamps
  • playback health flags
• WebApiDocsResult now exposes PowerShellExampleMediaManifestPath
• docs were updated to document the manifest output

Coverage/tests added:

• playback-sidecar test now asserts the manifest exists and contains the expected source/asset linkage
• stale playback test now asserts the manifest carries health flags for unsupported/oversized/stale assets

Verification:

• dotnet test .\\PowerForge.Tests\\PowerForge.Tests.csproj -c Release --filter "FullyQualifiedName~WebApiDocsGeneratorPowerShellTests|FullyQualifiedName~WebPipelineRunnerApiDocsPreflightTests|FullyQualifiedName~WebCliApiDocsPreflightTests|FullyQualifiedName~WebApiDocsGeneratorSourceAndCssTests|FullyQualifiedName~WebApiDocsGeneratorContractTests"
• Passed: 71, Failed: 0

[claude]

PR Review: Harden API docs layout and fallback generation

This is a substantial, well-structured PR that adds git freshness metadata, PowerShell example validation/execution, example-origin tracking, media sidecar support, and richer coverage thresholds. Overall architecture is coherent and test coverage is solid. Below are observations by category.

POTENTIAL BUGS

1. Silent freshness threshold inversion in AppendGitFreshnessMetadata (Freshness.cs)

If a user sets GitFreshnessNewDays > GitFreshnessUpdatedDays (e.g. newDays=100, updatedDays=50), updatedDays is silently promoted to match newDays, meaning nothing is ever classified as "updated". A preflight warning similar to other misconfig warnings would help catch this.

2. Unused captured result in refactored test (WebApiDocsGeneratorPowerShellTests.cs, line 203)

var result = WebApiDocsGenerator.Generate(options) -- result is assigned but never asserted on. Either add an assertion or drop the variable assignment.

3. ExecutionStdOut/ExecutionStdErr unbounded in memory

Per-file stdout/stderr is held in WebApiDocsPowerShellExampleFileValidationResult and serialized to the JSON report. Long-running or chatty scripts can produce large output. A truncation guard (e.g. 64 KB) before serialization would prevent unexpectedly large reports.

PERFORMANCE

4. Blocking async calls in TryGetGitFreshness (Freshness.cs)

gitClient.ShowTopLevelAsync(...).GetAwaiter().GetResult() and gitClient.RunRawAsync(...).GetAwaiter().GetResult() are called in a sequential foreach over every type. For large assemblies this serializes all git I/O and holds threadpool threads. The existing freshnessCache and repositoryRootCache limit redundant calls -- good -- but types spread across many source files still produce per-file git invocations. Consider batching git log calls or making AppendGitFreshnessMetadata async end-to-end.

SECURITY

5. Path traversal guard applies only after relative-path computation (TryStageApiExampleAsset)

The escape check (StartsWith("..")) is applied to the relative path computed against the examples root. When PowerShellExamplesPath is null/empty this check is skipped. A positive-allowlist check (source must be under one of the configured roots) before File.Copy would close this more firmly.

6. PowerShell execution of user-supplied scripts

ExecuteMatchedPowerShellExamples runs arbitrary .ps1 files from psExamplesPath inside the doc-generation process with no sandbox. This is opt-in and intentional, but adding a note to the docs that this feature should only be enabled in trusted CI environments would help operators understand the trust boundary.

CODE QUALITY

7. System.Diagnostics import added to test files may be unused

Both WebApiDocsGeneratorPowerShellTests.cs and WebPipelineRunnerApiDocsPreflightTests.cs gain a using System.Diagnostics; that does not appear referenced in the visible diff. Worth verifying it is actually used to avoid dead using directives.

8. Stale-asset detection uses filesystem timestamps (WarnOnStalePowerShellPlaybackAsset)

File.GetLastWriteTimeUtc is used to compare reference vs. asset ages. In CI environments where git checkout resets all mtime values to the current time, stale warnings will never fire. A git-log-based timestamp (already computed for the freshness feature) or a documented caveat would set accurate operator expectations.

9. 4-byte SHA256 hash for staging filenames (ComputeStableShortHash)

32 bits gives ~1-in-4B collision probability per name. Acceptable for typical doc repos since the full path is hashed, but a brief comment explaining why 4 bytes is sufficient would aid future readers.

10. api-overview-grid removed from required CSS contract but still present in fallback.css

The selector was removed from RequiredApiDocsCssSelectors while the class still exists in fallback.css. If intentional (kept for backwards compat but no longer contract-enforced), a brief comment would clarify this is not an oversight.

DOCUMENTATION

The docs updates in PowerForge.Web.ApiDocs.md and PowerForge.Web.Pipeline.md are thorough and well-written. One small gap: the git freshness section does not mention the CI filesystem-timestamp caveat from point 8 -- operators enabling generateGitFreshness or stale-asset warnings in CI may wonder why those warnings never trigger.

SUMMARY

The feature set is well-designed and test coverage is solid, especially the integration tests that spin up a real git repo in temp with controlled commit dates. The two most actionable items are the silent threshold-inversion bug in freshness configuration and the sync-over-async pattern in TryGetGitFreshness. The rest are minor hardening or polish suggestions.

Generated with Claude Code (https://claude.com/claude-code)

[PrzemyslawKlys]

This pass wires the PowerShell example media manifest into CLI/pipeline visibility.

What changed:

• pipeline apidocs step summaries now append:
  • ps-example-media: powershell-example-media-manifest.json
    when imported terminal media exists
• direct CLI web apidocs output now logs the media manifest path in non-JSON mode
• JSON CLI output now preserves PowerShellExampleMediaManifestPath when warnings/validation paths cause the result envelope to be rebuilt
• docs were updated so this step-summary behavior is discoverable

Coverage/tests added:

• pipeline regression verifies the step message includes ps-example-media: powershell-example-media-manifest.json

Verification:

• dotnet test .\\PowerForge.Tests\\PowerForge.Tests.csproj -c Release --filter "FullyQualifiedName~WebApiDocsGeneratorPowerShellTests|FullyQualifiedName~WebPipelineRunnerApiDocsPreflightTests|FullyQualifiedName~WebCliApiDocsPreflightTests|FullyQualifiedName~WebApiDocsGeneratorSourceAndCssTests|FullyQualifiedName~WebApiDocsGeneratorContractTests"
• Passed: 72, Failed: 0

[PrzemyslawKlys]

Companion website polish PR: https://github.com/EvotecIT/IntelligenceX/pull/1226\n\nRecommended review split:\n- start here for shared engine/template, validation, coverage, and pipeline changes\n- then review the IntelligenceX PR for site-level CSS and theme manifest adoption

[claude]

PR Review: Harden API docs layout and fallback generation

This is a substantial PR (~5400 lines net) adding git freshness metadata, PowerShell example validation/execution, media manifest generation, example provenance tracking, and several CSS/contract improvements. The overall approach is sound and well-structured. Here are my findings:

---

Positives

• Graceful degradation throughout — git freshness, PowerShell validation, and media manifest all fail-safe: warnings are emitted but generation continues. This is the right model for optional features.
• Bounded inputs — Math.Clamp(options.TimeoutSeconds, 5, 600) on all configurable timeouts prevents runaway processes.
• Proper string comparison hygiene — consistent use of StringComparer.OrdinalIgnoreCase and StringComparison.OrdinalIgnoreCase throughout.
• Test cleanup improvement — replacing if (Directory.Exists(root)) Directory.Delete(root, true) with TryDeleteDirectory(root) is a good defensive fix.
• Freshness test correctness — manipulating GIT_AUTHOR_DATE/GIT_COMMITTER_DATE via environment variable injection to control commit timestamps is the right approach for deterministic freshness assertions.

---

Issues and Suggestions

1. Unused result variable — WebApiDocsGeneratorPowerShellTests.cs

var result = WebApiDocsGenerator.Generate(options);

result is captured in Generate_PowerShellHelp_DoesNotInjectPrismAssets_WhenPrismModeOff but never asserted on. Either add a Assert.NotNull(result) / coverage assertion or drop the assignment to avoid confusion.

---

2. Blocking async calls in hot path — WebApiDocsGenerator.Freshness.cs

var topLevel = gitClient.ShowTopLevelAsync(directory).GetAwaiter().GetResult();
// ...
var log = gitClient.RunRawAsync(...).GetAwaiter().GetResult();

Calling .GetAwaiter().GetResult() on async methods in a synchronous hot path can deadlock in certain synchronization contexts (e.g. ASP.NET classic, test runners with a custom scheduler). If GitClient methods have synchronous equivalents, prefer those. If not, consider wrapping in Task.Run(...).GetAwaiter().GetResult() to ensure execution on a thread-pool thread, or better yet propagate async up the call chain if feasible.

---

3. Path traversal check robustness — WebApiDocsGenerator.Freshness.cs

var relativePath = Path.GetRelativePath(repositoryRoot, fullPath);
if (string.IsNullOrWhiteSpace(relativePath) ||
    relativePath.StartsWith("..", StringComparison.Ordinal))
    return null;

Path.GetRelativePath on .NET normalizes separators and resolves .. components, so this is generally safe on current .NET runtimes. However, symbolic links are not followed by Path.GetFullPath — a symlink inside repositoryRoot pointing outside it would pass this check. If the source tree can contain user-controlled symlinks, consider using Path.GetFullPath(Path.Combine(repositoryRoot, relativePath)) and verifying it still starts with repositoryRoot as a secondary guard.

---

4. Silent failure in bootstrap module resolution — WebApiDocsGenerator.PowerShellValidation.cs

catch { return null; }

The bare catch { return null; } in ResolvePowerShellExampleExecutionBootstrapModulePath() (or equivalent) swallows all exceptions including OutOfMemoryException and ThreadAbortException. Consider at minimum catch (Exception) and adding a warning/log entry so failures are surfaced during CI debugging. Currently, if bootstrap resolution fails silently, execution falls through with no diagnostic.

---

5. Output truncation is silent — WebApiDocsGenerator.PowerShellValidation.cs

The hard cap on PowerShell process stdout capture (16 KB) means that large validation outputs or verbose error messages will be truncated without any indication to the caller. Consider either:

• Appending a truncation marker to the captured string when the limit is hit
• Including the total byte count in the result so callers can detect truncation

---

6. Duplicate test helpers across test files

RunGit(), IsGitAvailable(), and TryDeleteDirectory() appear to be defined (or will be defined) in multiple test classes. Consider extracting these into a shared GitTestHelpers static class or a test base class to avoid drift. In particular, if RunGit's environment variable handling ever needs a fix, it should only need to change in one place.

---

7. Schema file — powerforge.web.pipelinespec.schema.json

The schema additions for new pipeline options (validation timeouts, execution flags, freshness days, etc.) look comprehensive. One suggestion: the new maxPowerShell* count thresholds use minimum: 0 but no maximum — consider adding a maximum to match the documented >= 0 constraint and provide IDE tooling with a useful bound.

---

8. CSS: minified single-line rules in fallback.css

The new freshness/media/badge CSS is added as minified single-line rules, which is consistent with the existing style in this file. No issue — just noting it is intentional and consistent.

---

Minor Nits

• generatedAtUtc in the media manifest uses DateTime.UtcNow.ToString("O") while other places use DateTimeOffset.UtcNow — prefer DateTimeOffset for round-trip fidelity and timezone-explicit output.
• The urlSamples set in WebApiDocsGenerator.ApiDocs.SourceValidation.cs collects up to 4 samples but the warning only renders 2 (urlSamples.Take(2)). The cap could be 2 from the start to match usage.
• In the new coverage threshold list in PowerForge.Web.Pipeline.md, the line is quite long. Consider splitting the threshold names across multiple bullet points for readability (matches the existing style elsewhere in that doc).

---

Summary

The architecture is well-reasoned and the opt-in design for freshness/validation/execution keeps the blast radius of new features low. The main items worth addressing before merge are the blocking async calls in the freshness path (#2), the silent bootstrap failure swallow (#4), and the duplicated test helpers (#6). The rest are polish/robustness suggestions.

Reviewed by Claude (claude-sonnet-4-6)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 35e29d42f7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Stage execution transcripts into API output before linking

When powerShellExampleValidationReport is configured outside the API output directory (for example ./_reports/...), ExecutionArtifactPath points outside options.OutputPath, TryConvertApiOutputPathToRoute returns null, and imported examples silently lose transcript media links even after successful execution. This breaks the documented “reuse execution transcripts as terminal media” flow for custom report locations; the generator should copy or stage those artifacts under API output before computing URLs.

Useful? React with 👍 / 👎.

[PrzemyslawKlys]

Addressed the current review batch in 91f6e693.

What changed:

• skip the parent-repo preflight warning when sourceUrl uses {pathNoRoot}
• restore a command-only generated fallback example when PowerShell help has no syntax entries
• make failOnPowerShellExampleValidation imply validation, and failOnPowerShellExampleExecution imply execution, in both CLI and pipeline flows
• reject percent-based API coverage thresholds above 100
• anchor git freshness candidate paths to sourceRoot for source-relative API paths
• stage execution transcripts into API output before linking them, so external validation-report locations still produce terminal media links
• propagate video MIME types into rendered <source> tags
• tighten source URL sample guarding and clean up the PowerShell example scoring code/readability

Regression coverage added for each of the above.

Verification:

• dotnet test .\PowerForge.Tests\PowerForge.Tests.csproj -c Release --filter "FullyQualifiedName~WebApiDocsGeneratorPowerShellTests|FullyQualifiedName~WebPipelineRunnerApiDocsPreflightTests|FullyQualifiedName~WebCliApiDocsPreflightTests|FullyQualifiedName~WebApiDocsGeneratorSourceAndCssTests|FullyQualifiedName~WebApiDocsGeneratorContractTests"
• Passed: 81, Failed: 0

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 91f6e69335

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Mark no-file execution as completed before returning

When no example scripts are discovered, this early return sets ValidationSucceeded but leaves execution fields at defaults, so ExecutionCompleted stays false. In pipeline/CLI runs where failOnPowerShellExampleExecution is enabled (which now implies execution), modules with zero discovered scripts will fail unconditionally on !ExecutionCompleted even though nothing actually ran or failed. Set execution status explicitly for the empty-file case (or bypass execution gating when no files are discovered) to avoid false CI failures.

Useful? React with 👍 / 👎.

[claude]

PR #226 Review: Harden API docs quality, media, and PowerShell example workflows

This is a substantial PR adding PowerShell example validation/execution, git freshness metadata, richer coverage metrics, example provenance tracking, and playback media sidecar support. Overall the design is thoughtful and the test coverage is extensive. Here are the findings organized by severity.

---

🔴 High — Security / Data Leakage

1. Absolute build-agent paths emitted in public JSON artifacts

WebApiDocsGenerator.Freshness.cs unconditionally serializes freshness.sourcePath (the full absolute filesystem path) into every type JSON file when generateGitFreshness is enabled. Similarly, powershell-example-validation.json serializes HelpPath, ManifestPath, FilePath, ExecutionArtifactPath, and Executable as absolute paths.

When these artifacts are published to public API doc sites or stored as CI artifacts, they leak the build agent's directory structure, username, and drive letter. The media manifest already does this right with NormalizePowerShellExampleManifestPath — the same relativization should be applied here.

Recommendation: Relativize all paths in both BuildFreshnessJson and WritePowerShellExampleValidationReport against the output root, just like the media manifest does.

2. PowerShell execution does not validate that .ps1 files are within the allowed examples root

ValidatePowerShellExamples executes matched .ps1 files after syntax validation + command-name matching, but never asserts that file.FilePath is canonically within options.PowerShellExamplesPath. If a malformed manifest or tool surfaces a .ps1 path outside the configured examples root, it will be executed with the PowerShell host's privileges.

Recommendation: Add a check that Path.GetFullPath(file.FilePath) starts with Path.GetFullPath(options.PowerShellExamplesPath) before executing.

3. Captured stdout/stderr stored verbatim in CI artifacts

ExecutionStdOut and ExecutionStdErr (capped at 16,000 chars) are serialized into powershell-example-validation.json. If example scripts output sensitive data (module secrets, credentials, tokens used in test setup), that output is captured and stored. The documentation does not warn about this.

Recommendation: Add a note in the docs that execution output is captured and stored in the validation report.

---

🟠 Medium — Correctness / Reliability

4. Sync-over-async in TryGetGitFreshness

var topLevel = gitClient.ShowTopLevelAsync(directory).GetAwaiter().GetResult();
var log = gitClient.RunRawAsync(...).GetAwaiter().GetResult();

These sync-over-async calls risk deadlocking if the call chain ever gains a SynchronizationContext. Since freshness is best-effort and opt-in, consider using synchronous Process calls directly or wrapping with Task.Run(...).GetAwaiter().GetResult().

5. Silent correction of GitFreshnessNewDays > GitFreshnessUpdatedDays misconfiguration

var updatedDays = Math.Max(options.GitFreshnessNewDays, options.GitFreshnessUpdatedDays);
var newDays = Math.Clamp(options.GitFreshnessNewDays, 0, updatedDays);

If a user mistakenly sets gitFreshnessNewDays: 100 and gitFreshnessUpdatedDays: 14, both are silently remapped to 100, making the updated window unreachable. A [PFWEB.APIDOCS.SOURCE] warning should be emitted when newDays > updatedDays.

6. Unbounded line-number scoring penalty in example ranking

score -= Math.Max(0, lineNumber);

A match at line 194 always produces a negative score regardless of file-name quality (max bonuses total ~193). Matches in any reasonably-sized file will be permanently disadvantaged. A normalized penalty like Math.Min(lineNumber / 10, 50) would produce more proportionate results.

7. Intermediate candidate pruning is too aggressive

if (commandCandidates.Count > limitPerCommand * 3)
{
    commandCandidates = commandCandidates...Take(limitPerCommand).ToList(); // prunes to limitPerCommand, not *3

The guard fires at Count > limit * 3 but prunes all the way to limit, permanently discarding candidates from later files that might have scored higher. Pruning to limit * 2 as an intermediate step with a final Take(limit) at the end would be fairer.

8. HasImportedPlaybackMediaWithPoster vs. WithoutPoster semantics mismatch

WithPoster uses All(has poster) while WithoutPoster uses Any(missing poster). For a command with mixed coverage (some examples have posters, some don't), it counts in WithoutPoster but not in WithPoster. The two counts are not complementary and their sum does not equal the total — which may confuse CI threshold authors. This design choice should be documented explicitly.

---

🟡 Low — Code Quality / Maintainability

9. PowerShellRunner not disposed

var runnerToUse = runner ?? new PowerShellRunner();

If IPowerShellRunner is IDisposable (which process-wrapping implementations typically are), this leaks resources. Use a using pattern or ensure the caller owns the lifetime.

10. Test helper duplication across two test files

BuildMinimalPowerShellHelpForValidation(), IsGitAvailable(), RunGit(), and TryDeleteDirectory() are copy-pasted identically into both WebApiDocsGeneratorPowerShellTests.cs and WebPipelineRunnerApiDocsPreflightTests.cs. Any fix must be applied in two places. Extract to a shared ApiDocsTestHelpers static class.

11. RunGit and IsGitAvailable don't check WaitForExit return value

If git hangs past the timeout, process.ExitCode is read on a still-running process, which throws on some runtimes. The correct pattern:

if (!process.WaitForExit(timeoutMs)) { process.Kill(); throw ...; }

12. CloneMember silently drops new ApiExampleMediaModel fields

The clone operation omits SourcePath, AssetPath, PosterAssetPath, CapturedAtUtc, SourceUpdatedAtUtc, HasUnsupportedSidecars, HasOversizedAssets, and HasStaleAssets. These fields are silently lost when members are cloned for extension method attribution.

13. New coverage threshold tests only cover the failure path

Tests for maxPowerShellGeneratedFallbackOnlyExampleCount, minPowerShellAuthoredHelpCodeExamplesPercent, etc. only assert that exceeding a threshold fails the step. There are no complementary tests showing that a module meeting the threshold passes, leaving off-by-one errors in comparison direction undetected.

14. Magic number 100 cap on command lists is undocumented

commandsUsingAuthoredHelpCodeExamples, commandsUsingImportedScriptPlaybackMediaStaleAssets, etc. are all .Take(100). This cap is not configurable and not mentioned in docs — large modules with >100 commands will silently get truncated lists in coverage reports.

15. Possible duplicate test method name

RunPipeline_ApiDocs_FailsWhenLegacyAliasModeIsInvalid appears to be defined twice in WebPipelineRunnerApiDocsPreflightTests.cs, which will cause a compile error or test runner warning.

---

✅ What's Done Well

• The PowerShell single-quote escaping in ToPowerShellSingleQuotedLiteral (doubling embedded ') is correct and prevents injection through path values.
• The temp directory uses Guid.NewGuid().ToString("N") — not predictable.
• The .. guard in TryStageApiExampleAsset prevents output-rooted path traversal.
• Git config user.email/user.name are set locally in freshness tests — good isolation practice.
• The provenance tracking (AuthoredHelp / ImportedScript / GeneratedFallback) is a well-structured addition that will genuinely help CI distinguish example quality tiers.
• The media manifest relativization via NormalizePowerShellExampleManifestPath is the right pattern — just needs to be applied consistently to the validation report and freshness JSON too.

---

Reviewed by Claude Sonnet 4.6

[PrzemyslawKlys]

Addressed the latest API-docs review pass in bed21341:

• fixed the missing WebApiDocsOptions.SiteBaseUrl / LanguageCode branch drift that was breaking DotNetPublishTests
• normalized PowerShell validation/freshness paths so generated JSON stops leaking build-agent absolute paths
• marked no-script execution runs as completed to avoid false failOnPowerShellExampleExecution failures
• guarded matched-example execution so explicitly configured PowerShellExamplesPath acts as an execution boundary
• documented the normalized validation report fields and captured stdout/stderr behavior

Local verification:

• dotnet build .\PowerForge.Web.Cli\PowerForge.Web.Cli.csproj -c Debug
• dotnet test .\PowerForge.Tests\PowerForge.Tests.csproj -c Debug --filter "FullyQualifiedName~WebApiDocsGeneratorPowerShellTests|FullyQualifiedName~WebPipelineRunnerApiDocsPreflightTests|FullyQualifiedName~WebCliApiDocsPreflightTests|FullyQualifiedName~WebApiDocsGeneratorSourceAndCssTests|FullyQualifiedName~WebApiDocsGeneratorContractTests"
• result: 83 passed, 0 failed

I also re-ran the targeted logic against the updated tests for relative artifact paths and normalized freshness source paths. Release-mode local verification is currently noisy in this workspace because long-lived PowerForge.Web.Cli processes are holding the Release output under bin\Release, so I used Debug for the focused validation while the fresh GitHub CI run confirms the full Release matrix.

[claude]

Code Review: PR #226 — Harden API docs quality, media, and PowerShell example workflows

This is a substantial PR (~6,000 lines) that adds git freshness metadata, PowerShell example provenance tracking, media manifest generation, syntax/execution validation, and richer coverage reporting. The overall architecture is solid and consistent with the existing codebase. Here are my observations:

---

Potential Bugs

1. Freshness window misconfiguration is silently mishandled
In WebApiDocsGenerator.Freshness.cs:

var updatedDays = Math.Max(options.GitFreshnessNewDays, options.GitFreshnessUpdatedDays);
var newDays = Math.Clamp(options.GitFreshnessNewDays, 0, updatedDays);

If a user sets GitFreshnessNewDays > GitFreshnessUpdatedDays (e.g., newDays=90, updatedDays=14), updatedDays gets silently promoted to 90, and newDays is clamped to 90 too — meaning nothing is ever labeled "updated" and everything within 90 days is "new". This will likely surprise users. Consider emitting a warning instead of silently correcting the values, or at minimum document this clamping behavior in the XML doc.

2. Sync-over-async pattern (GetAwaiter().GetResult())
In TryGetGitFreshness, git calls are made synchronously:

var topLevel = gitClient.ShowTopLevelAsync(directory).GetAwaiter().GetResult();
var log = gitClient.RunRawAsync(...).GetAwaiter().GetResult();

These can deadlock in contexts with a non-default SynchronizationContext (e.g., ASP.NET classic, WinForms). While this is likely called from a console/build context only, the risk is worth noting. Consider adding ConfigureAwait(false) on the async call or making AppendGitFreshnessMetadata itself async if the caller chain permits.

3. AppendPowerShellExampleQualityWarnings early-exits before emitting health warnings

if (playbackWithoutPoster.Length == 0)
    return;  // <-- exits here

The warnings for HasUnsupportedSidecars, HasOversizedAssets, and HasStaleAssets are handled in AppendPowerShellExampleValidationWarnings (called from ValidatePowerShellExamples), so they're not lost — but if AppendPowerShellExampleQualityWarnings is called independently during Generate(), those health warnings won't surface. Verify that the Generate() flow covers all the health warning paths.

---

Code Quality

4. Silent exception swallowing without diagnostics
AddFreshnessCandidate and NormalizePowerShellExampleManifestPath both use bare catch { } blocks labeled "best effort." While acceptable for resilience, these can hide real issues (e.g., path containing illegal characters, unexpected filesystem states). Consider logging to warnings at debug/trace level:

catch (Exception ex)
{
    // best effort: skip files that can't be resolved
    // warnings.Add($"[debug] skipped path '{path}': {ex.Message}");
}

5. Unused using import
WebApiDocsGenerator.Parse.PowerShellExamples.cs adds using System.Security.Cryptography; — confirm this is actually used somewhere in the file (e.g., for hashing media file content to detect staleness). If not needed, remove it to keep the file clean.

6. FriendlyPowerShellExampleTypes contains C# aliases alongside .NET type names

"String", "Int32", "int", "Int64", "long", ...

Both aliases and framework names are listed, which is intentional (matching both MAML and reflection sources). A brief comment here would help future maintainers understand why duplicates are intentional.

---

Performance Considerations

7. Multiple LINQ chains over commandTypes in BuildCoveragePayload
The BuildCoveragePayload method now enumerates commandTypes ~12 separate times (once per coverage metric). For very large modules this is a minor inefficiency. Not a real problem in practice since coverage generation is a one-shot build step, but worth noting if the method is ever called in a hot path.

8. Git call per directory (not per repository root)
The repositoryRootCache is keyed by directory path, so a module with N source directories will make N git rev-parse --show-toplevel calls even if they all share the same repo root. Once the root is found for one directory, its parent directories are not pre-populated in the cache. For modules with many source directories, this can cause redundant git calls. Consider keying by the resolved repositoryRoot after the first lookup.

---

Security Considerations

9. ExecuteMatchedExamples runs user-provided PowerShell scripts
This is appropriately guarded behind an explicit opt-in flag (ExecuteMatchedExamples = true) and uses an injected IPowerShellRunner interface (good for testing). However:

• The ExecutionStdOut and ExecutionStdErr are stored in the result and written to the validation report JSON. If scripts emit sensitive values (tokens, credentials), these will appear in the report artifact. Consider documenting this caveat prominently in the XML doc and potentially truncating captured output.
• The execution timeout is clamped to 600 seconds (10 min) per file. For large example suites, total execution time could be very long. An aggregate timeout might be worth adding.

---

Minor Issues

10. Trailing inconsistent whitespace in WebApiDocsOptions
In the diff for WebApiDocsGenerator.cs, the new SiteBaseUrl and LanguageCode properties have 6-space indentation while surrounding properties use 4-space. Worth normalizing.

11. Schema version not shown in diff
Schemas/powerforge.web.pipelinespec.schema.json is listed as changed. Ensure the schema $schema version or a version field is bumped to reflect the many new pipeline configuration options (coverage thresholds, generateGitFreshness, validation options). Breaking changes to required fields should be clearly reflected in schema versioning.

---

What's Done Well

• Consistent use of static lambdas (static c => c.Name) throughout — good allocation hygiene.
• All public methods have XML doc comments; new models are fully documented.
• Two-level caching (repositoryRootCache + freshnessCache) prevents redundant git invocations.
• IPowerShellRunner interface injection makes the validation path fully testable without needing a real PowerShell installation.
• Provenance tracking (AuthoredHelp / ImportedScript / GeneratedFallback) is a clean design that makes coverage analysis meaningful.
• Comprehensive test coverage added for all major new components.
• All new features are opt-in with explicit flags — no breaking changes to defaults.
• The CSS contract documentation is thorough and will help site theme implementors.

---

Overall this is high quality work. The main items worth addressing before merge are #1 (silent freshness window misconfiguration) and #9 (documenting the stdout/stderr capture behavior for security-sensitive environments). The rest are minor polish items.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bed21341f5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Handle file psExamplesPath before manifest path relativization

When psExamplesPath points to a single .ps1 file (which is supported), this code treats that file path as a directory root for Path.GetRelativePath. For the matched file itself, the relative value becomes ".", so sourcePath/assetPath entries in powershell-example-media-manifest.json lose the actual filename and become ambiguous. Use the file’s parent directory as the root when PowerShellExamplesPath is a file so manifest paths remain meaningful.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Restage media when source asset timestamp is older

The staging copy only runs when the source mtime is newer than the destination. In incremental builds, switching to an older branch/commit (or restoring older captured media) can make the source older than an existing staged file, so stale media from a previous run is kept and linked in generated docs. Staging should refresh when timestamps differ (or compare content) so output always matches current source assets.

Useful? React with 👍 / 👎.

[PrzemyslawKlys]

Addressed the latest inline PowerShell media follow-up in 286b9834:

• fixed powershell-example-media-manifest.json path normalization when PowerShellExamplesPath points to a single .ps1 file
• fixed playback/media restaging so older-but-current source assets still refresh staged output during incremental builds or branch switches
• added focused regressions for both cases

Verification:

• dotnet test .\PowerForge.Tests\PowerForge.Tests.csproj -c Debug --filter "FullyQualifiedName~WebApiDocsGeneratorPowerShellTests"
• result: 30 passed, 0 failed

[claude]

PR #226 Review — Harden API docs quality, media, and PowerShell example workflows

This is a substantial and well-structured PR (~6,400 lines). The overall design is sound — the feature set is clearly useful, the docs are thorough, and the test coverage is broad. Below are specific findings worth addressing before merge.

---

🐛 Bugs / Logic Issues

1. CloneMember drops 5 media metadata fields (WebApiDocsGenerator.Reflection.Attributes.cs)

The clone of ApiExampleMediaModel omits CapturedAtUtc, SourceUpdatedAtUtc, HasUnsupportedSidecars, HasOversizedAssets, and HasStaleAssets. When member models are cloned for extension-method surfacing, freshness/stale metadata is silently dropped. This means those badges and warnings won't render for cloned member examples.

There's no test covering this path — adding one would catch the regression.

2. GetImportedPowerShellExampleScore — line-number penalty isn't calibrated (WebApiDocsGenerator.Parse.PowerShellExamples.cs)

score -= Math.Max(0, lineNumber);

This subtracts the raw 0-based line number, so a snippet at line 150 loses 150 points — which completely overwhelms the exact-name-match bonus (140 points). For files longer than ~140 lines, line position dominates the ranking regardless of naming. Consider normalizing the penalty (e.g., score -= lineNumber / 10) or capping it.

3. Redundant self-assignment in WebPipelineRunner.Tasks.Content.cs

if (executePowerShellExamples || failOnPowerShellExampleExecution)
    executePowerShellExamples = true;

The first branch is a no-op. The intent is clearly that failOnPowerShellExampleExecution implies execution — this should be:

if (failOnPowerShellExampleExecution)
    executePowerShellExamples = true;

---

🔒 Security

4. No trust boundary on user-supplied manifestPath before execution (WebApiDocsGenerator.PowerShellValidation.cs)

BuildPowerShellExampleExecutionCommand constructs a PowerShell script containing Import-Module -Name '<manifestPath>'. The path is resolved from HelpPath or PowerShellModuleManifestPath, both user/pipeline-controlled. If this tooling is ever used in a shared CI context where pipeline config is not fully trusted, a crafted .psd1 path could execute arbitrary code during doc generation. Consider validating the resolved manifest path falls under a known project root before using it in a subprocess.

5. ComputeStableShortHash truncates to 4 bytes (8 hex chars)

return Convert.ToHexString(bytes[..4]).ToLowerInvariant();

At 4 bytes, birthday collisions become likely around ~65k staged files. While that's an extreme case for docs, a collision would silently overwrite one staged asset with another. Using 8 bytes (16 hex chars) would make the probability negligible with no meaningful downside.

---

⚡ Performance

6. Blocking async git calls per-type in WebApiDocsGenerator.Freshness.cs

gitClient.ShowTopLevelAsync(directory).GetAwaiter().GetResult()
gitClient.RunRawAsync(...).GetAwaiter().GetResult()

These are called inside a foreach over each type's source file — potentially hundreds of synchronous subprocess invocations. There's no batching (e.g., git log -- file1 file2 file3) and no CancellationToken plumbed through. For a module with 100 commands this could add significant wall-clock time to generation. Consider: (a) batching files within the same directory, or (b) at minimum documenting the performance characteristic in the generateGitFreshness docs so teams can make an informed opt-in decision.

---

🧪 Test Coverage Gaps

7. IsGitAvailable() guard silently passes rather than skips (WebApiDocsGeneratorPowerShellTests.cs and pipeline tests)

if (!IsGitAvailable())
    return;

In a CI environment without git, the freshness tests return early and are reported as passed, not skipped. This makes the test results misleading. Prefer xunit skip attributes or Assert.Skip(...):

if (!IsGitAvailable())
    Skip.If(true, "git not available");

8. No test for CloneMember media metadata propagation

As noted in finding #1, the 5 dropped fields have no test coverage. A test asserting that a cloned member's example media retains all metadata fields would catch this class of regression.

9. BuildMinimalPowerShellHelpForValidation duplicated verbatim across 3 test classes

This helper appears copy-pasted into WebApiDocsGeneratorPowerShellTests, WebCliApiDocsPreflightTests, and WebPipelineRunnerApiDocsPreflightTests. Moving it to a shared test helper class would reduce maintenance burden.

---

✅ What's Working Well

• The IsWithinConfiguredPowerShellExamplesPath guard before script execution is correctly implemented using Path.GetRelativePath and .. detection.
• Path normalization in validation reports (stripping build-agent absolute paths) is a thoughtful touch for CI reproducibility.
• The coverage report's distinction between AuthoredHelp, ImportedScript, and GeneratedFallback is clean and the new JSON origin field is well-documented.
• The generateGitFreshness: false default (opt-in) is the right call given the per-file git cost.
• The CSS contract test additions in WebApiDocsGeneratorContractTests.cs keep theme compatibility well-guarded.

---

Priority Summary

#	Severity	Item
1	Medium	CloneMember drops 5 media metadata fields silently
2	Medium	Line-number penalty scale overwhelms name-match bonus
6	Medium	Blocking git subprocess per-type with no batching/cancellation
7	Medium	Silent test pass vs. skip when git unavailable
4	Low-Medium	No manifest path trust check before Import-Module execution
5	Low	4-byte hash collision risk in ComputeStableShortHash
3, 8, 9	Low	No-op assignment, missing clone test, duplicated test helper

Overall this is a well-thought-out feature set. Addressing items 1, 2, 6, and 7 would meaningfully improve correctness and test reliability before merge.

Reviewed by Claude Sonnet 4.6

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 286b98344f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Deduplicate imported playback media per script

When one .ps1 file contributes multiple matched snippets for the same command, this early return emits a terminal media example for each snippet even though they all reference the same sidecar asset. TryStageApiExampleAsset only deduplicates file copying, not emitted examples, so generated docs can show repeated identical media blocks and powershell-example-media-manifest.json can contain duplicate entries for the same command/media URL pair. Gate playback media emission with the same per-command artifact dedupe used by transcript media.

Useful? React with 👍 / 👎.