fix: use relative urls for local docs testing - #748
Merged
Merged
Conversation
- Change hardcoded github.io playground url in docs/index.md to relative path - Change hardcoded github.io docs url in playground mainlayout to relative path - Update local testing scripts to set correct base href for playground - Local testing now properly uses relative paths without redirecting to production Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughUpdated docs links and local build scripts to target .NET 10; changed playground bootstrap to load Monaco before Blazor; increased Piston memory limits in the client; added robust real/simulated execution handling with deterministic mock outputs; wrapped and hardened the editor UI; and massively expanded built-in code examples. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(240,248,255,0.5)
participant Browser
participant Monaco
participant Blazor
participant PlaygroundAPI
end
Browser->>Monaco: load Monaco loader (loader.js)
Monaco-->>Browser: monacoRequire & vs paths configured
Browser->>Blazor: call Blazor.start() inside monacoRequire callback
Browser->>Blazor: user clicks "Run" (send editor content)
Blazor->>PlaygroundAPI: TryExecuteViaApiAsync (if EnableApiCalls)
alt API returns success
PlaygroundAPI-->>Blazor: execution result
Blazor->>Browser: display "=== Real Code Execution ===" + output
else API returns 429 / error / disabled
PlaygroundAPI-->>Blazor: null or error
Blazor->>Blazor: simulate deterministic realistic output
Blazor->>Browser: display simulated output with guidance
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/test-docs-local.sh`:
- Around line 86-90: The sed in-place edit for _site/playground/index.html is
not portable on macOS; update the script to detect Darwin vs non-Darwin (uname)
and run a BSD-safe sed invocation (e.g., use sed -i '' 's|...|...|g' on macOS or
sed -i on Linux) or alternatively use sed -i.bak and then remove the .bak file
after the replacement; ensure the branch touches the same target file
(_site/playground/index.html) and that any temporary backup (.bak) is cleaned up
so the script remains safe under set -e.
Handle sed -i portability between macOS (BSD) and Linux (GNU) by detecting OSTYPE and using appropriate syntax. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates documentation/playground links and local DocFX test scripts so local navigation stays on localhost rather than redirecting to the production ooples.github.io site.
Changes:
- Switch Playground ↔ Docs links from hardcoded GitHub Pages URLs to relative paths.
- Update local test scripts to rewrite the Playground
index.html<base href>to/playground/. - Adjust docs homepage Playground link to target the locally served playground content.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/AiDotNet.Playground/MainLayout.razor |
Points “Documentation” link to a relative parent path (../) instead of production GitHub Pages URL. |
scripts/test-docs-local.sh |
Rewrites Playground index.html base href during local doc build to match /playground/ hosting. |
scripts/test-docs-local.ps1 |
Same base-href rewrite for local builds on Windows/PowerShell. |
docs/index.md |
Updates docs homepage “Interactive Playground” link to a relative local path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix Piston API memory limits causing OOM errors in execute.ts - Add realistic simulated values instead of [value] placeholders - Fall back to simulation mode when API fails (errors in console only) - Update simulation footer with real sample paths and deploy instructions - Fix Monaco editor loading with proper require.js handling - Add many new playground examples across all categories - Remove debugging console statements - Add favicon.ico for playground Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/AiDotNet.Playground/Services/CodeExecutionService.cs`:
- Around line 157-178: The current logic treats ApiExecuteResponse with
Success==false as a parse/unavailable error and returns null (leading to
simulation); instead, when
response.Content.ReadFromJsonAsync<ApiExecuteResponse>(...) returns a non-null
result with result.Success==false, surface or propagate that API error
(result.Error) to the caller rather than falling back to simulation: update the
post-parse handling in CodeExecutionService so that after reading
ApiExecuteResponse you only fall back to simulation on parse/HTTP/unavailable
exceptions, but if result is non-null and !result.Success either return the
result (so the caller can display the API compile/runtime error) or throw a
specific exception containing result.Error; keep the existing Console.WriteLine
logging for debugging but stop returning null for API-reported failures.
In `@src/AiDotNet.Playground/Services/ExampleService.cs`:
- Around line 876-882: The code uses Random.NextGaussian which doesn't exist;
replace it by sampling Gaussian values via the Box–Muller transform: implement a
helper (e.g., a private static method or Random extension named NextGaussian or
SampleGaussian) that accepts the Random instance (rng) plus mean/stdDev and
returns a normally distributed double, then call that helper when populating
normalData[i, 0] and normalData[i, 1]; ensure the helper caches the second
sample (or otherwise generates two values per transform) for efficiency and
deterministic seeding with new Random(42).
- Around line 1614-1619: The examples call BuildAsync(trainingData) but never
define trainingData; add a local sample trainingData variable before the
.ConfigureTraining(...).BuildAsync(...) calls (in the ExampleService
class/method containing the LoRA examples) — e.g. declare a small collection of
training examples (List or IEnumerable) with the same shape expected by
BuildAsync (pairs of input/output or a TrainingExample type used elsewhere in
ExampleService), populate it with 2–3 sample items, and reuse that variable for
the LoRA, QLoRA, DoRA and AdaLoRA examples so the code compiles and the examples
run.
♻️ Duplicate comments (2)
scripts/test-docs-local.sh (1)
52-52: Same net10.0 verification as the PowerShell script.Please ensure the project TFM and local SDK expectations are aligned (see earlier note).
scripts/test-docs-local.ps1 (1)
103-108: Preserve HTML encoding when writing index.html.
Set-Contentcan default to UTF-16 in Windows PowerShell, which may corrupt the HTML encoding. This was flagged before; consider an explicit UTF-8 encoding.♻️ Suggested fix
- Set-Content -Path $playgroundIndex -Value $content -NoNewline + Set-Content -Path $playgroundIndex -Value $content -NoNewline -Encoding UTF8
🧹 Nitpick comments (1)
src/AiDotNet.Playground/wwwroot/index.html (1)
48-64: Consider removing debug logging in production.The
console.logat line 55 is useful during development but should be removed or guarded for production builds.🔧 Suggested fix
window.monacoRequire(['vs/editor/editor.main'], function(monacoModule) { // Ensure monaco is available globally for BlazorMonaco if (typeof monaco === 'undefined' && monacoModule) { window.monaco = monacoModule; } - console.log('Monaco Editor loaded successfully, monaco global:', typeof monaco); Blazor.start().catch(function(error) {
- Add rel="noopener noreferrer" to target="_blank" links in MainLayout.razor - Add UTF8 encoding to PowerShell Set-Content for cross-platform support - Use consistent link format for playground in docs/index.md - Show real API compile/runtime errors instead of masking with simulation - Replace non-existent Random.NextGaussian() with Box-Muller transform - Add missing trainingData variable in LoRA example Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/AiDotNet.Playground/Services/CodeExecutionService.cs`:
- Around line 568-573: The current cluster-detection branch in
CodeExecutionService.cs uses lowerVar.Contains("k"), which matches unrelated
names (e.g., token, key); change this to only match a standalone "k" or specific
cluster-related names/keys: replace the Contains("k") check with either an exact
match for "k" (e.g., lowerVar == "k") or a word-boundary/explicit-name match
(e.g., regex or explicit checks for "k", "cluster", "clusters", "num_clusters",
"cluster_count") in the same block that computes value with
GetHashBasedVariation(varName), so variables like "key" or "token" are not
treated as cluster counts.
In `@src/AiDotNet.Playground/Services/ExampleService.cs`:
- Around line 1336-1346: The PPOAgent<double> constructor requires a
PPOOptions<double> object, not individual named parameters; replace the inline
parameter list in the new PPOAgent<double>(...) call by creating a
PPOOptions<double> instance (e.g. options) and set the corresponding properties
(StateSize, ActionSize, PolicyHiddenLayers, PolicyLearningRate,
ValueLearningRate, DiscountFactor, GaeLambda — note replace the incorrect gaeλ
name — ClipEpsilon, EntropyCoefficient, plus any required
TrainingEpochs/MiniBatchSize), then pass that options instance into new
PPOAgent<double>(options) inside the ConfigureModel call on
AiModelBuilder<double,double[],double[]> and continue to BuildAsync().
🧹 Nitpick comments (4)
scripts/test-docs-local.ps1 (1)
102-108: Make base href replacement resilient to template variations.The replacement only matches an exact
<base href="/" />. If the template formatting changes (e.g., no space or no self-closing slash), the update won’t apply and local navigation can break. Consider a regex that matches any base href.♻️ Proposed tweak
- $content = $content -replace '<base href="/" />', '<base href="/playground/" />' + $content = $content -replace '<base\s+href="[^"]*"\s*/?>', '<base href="/playground/" />'src/AiDotNet.Playground/Services/CodeExecutionService.cs (2)
461-464: Consider usingRegexHelperfor consistency.The codebase has a
RegexHelper.Create()method that standardizes timeout handling. Consider using it here for consistency.♻️ Suggested refactor
- var writeLinePattern = new Regex( - @"Console\.WriteLine\s*\(\s*(?:\$?""([^""]*)""|(\w+))\s*\)", - RegexOptions.Multiline, - RegexTimeout); + var writeLinePattern = RegexHelper.Create( + @"Console\.WriteLine\s*\(\s*(?:\$?""([^""]*)""|(\w+))\s*\)", + RegexOptions.Multiline, + RegexTimeout);This would require adding
using AiDotNet.Helpers;or adjusting based on the project structure.
632-640: Note:GetHashCode()is not deterministic across processes/runtimes.The comment states this ensures "the same variable always gets the same simulated value," but
string.GetHashCode()can return different values across different .NET versions, processes, or platforms. For simulation purposes this is likely acceptable, but consider updating the comment to clarify the scope of determinism.📝 Suggested comment update
/// <summary> - /// Gets a deterministic variation (0-1) based on variable name hash. - /// This ensures the same variable always gets the same simulated value. + /// Gets a pseudo-deterministic variation (0-1) based on variable name hash. + /// This provides consistent values within a single process execution. /// </summary>src/AiDotNet.Playground/Services/ExampleService.cs (1)
611-1913: Consider externalizing the example catalog for maintainability.The huge inline dictionary of verbatim code strings is hard to scan, diff, and merge. Moving examples to resource/JSON/MD files (or splitting per category) would improve maintainability and reduce merge conflicts.
- Add back memory limits with higher values (1GB compile, 512MB run) - Fix GetHashCode randomization by using deterministic FNV-1a hash - Fix PPOAgent example to use PPOOptions instead of individual params - Remove console.log from production index.html Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
2 of 3 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
github.ioplayground URL indocs/index.mdto relative path../playground/index.htmlgithub.iodocs URL in Playground'sMainLayout.razorto relative path../test-docs-local.ps1andtest-docs-local.sh) to set correct base href/playground/for the playground subdirectoryProblem
When testing documentation locally using
scripts/test-docs-local.ps1, links to the playground and between playground/docs would redirect to the productionooples.github.iosite instead of staying local.Solution
../playground/and../) instead of absolutegithub.ioURLsindex.htmlduring local build to/playground/so relative paths resolve correctly/AiDotNet/playground/and docs are at/AiDotNet/Test plan
scripts/test-docs-local.ps1locallylocalhost:8080/playground/localhost:8080//AiDotNet/playground/in CI)🤖 Generated with Claude Code