-
Notifications
You must be signed in to change notification settings - Fork 2
Improve C# code quality: fix build errors, add repository pattern, extract constants #107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
4d465da
Initial plan
Copilot 46ebe3f
Fix critical nullable reference warnings in AstVisitorExplainer
Copilot 4d3cda8
Extract magic strings into Constants class for improved maintainability
Copilot b3695ae
Add repository pattern for improved testability and separation of con…
Copilot 88a9516
Add InMemoryHelpRepository with comprehensive unit tests
Copilot 5ebb7d8
Address code review feedback: fix inconsistent empty list handling
Copilot e18fe25
Update package references to latest stable versions in project files
JosKWRubicon adc73b3
Refactor GitHub Actions workflow update for dotnet version handling
JosKWRubicon 005a27d
Refactor code to improve null handling and make properties nullable w…
JosKWRubicon 0e230a9
Update VSCode extension recommendations for improved development expe…
JosKWRubicon a87b779
Update code tours
JosKWRubicon 8c627ad
Add code tour for AI explanation feature
JosKWRubicon ac2efdf
Add Copilot instructions for Explain PowerShell repository
JosKWRubicon b136b9a
Add Invoke-AiExplanation function for AI analysis integration
JosKWRubicon 94bf734
Refactor AI Explanation integration tests to use Invoke-AiExplanation…
JosKWRubicon 86d250e
Fix links in README for explainshell.com and CodeTour extension
JosKWRubicon e105b03
Replace Newtonsoft.Json with System.Text.Json for request deserializa…
JosKWRubicon 2179e1d
Log unhandled AST node types and their counts in GetAnalysisResult
JosKWRubicon ce8b3af
Implement cancellation and cleanup for AI explanation requests to pre…
JosKWRubicon be8f39b
Add in-memory help repository seeding for unit tests
JosKWRubicon 8ee37a9
Enhance unit tests for return and throw statements, adding behavior v…
JosKWRubicon 2bd2352
Add explanations for return and throw statements with documentation l…
JosKWRubicon 0c3d2c2
Add HTML synopsis scraper and related functions for documentation ext…
JosKWRubicon df46893
Refactor logical operators for consistency in AstVisitorExplainer
JosKWRubicon 4d9f7e6
Add trap statement explanation and related tests
JosKWRubicon 117da9f
Update documentation links to use the new Microsoft Learn URLs
JosKWRubicon 81fe417
Add tests for tree generation and update syntax analyzer methods to r…
JosKWRubicon 231259e
Implement switch statement explanation and related tests
JosKWRubicon ccbb566
fix failing tests
JosKWRubicon ed1d480
Vastly speed up test suite
JosKWRubicon 85806b5
Refactor test execution scripts and update launch configurations
JosKWRubicon d9d67ca
Add unit tests for SyntaxAnalyzerFunction and refactor dependencies
JosKWRubicon de3c8f7
Add UpdateProfile switch to bootstrap script for conditional profile …
JosKWRubicon b9b95f4
Refactor GenerateTree method for improved clarity and performance
JosKWRubicon ed6ba30
Implement SyntaxAnalyzerClient and related classes for improved code …
JosKWRubicon 8a65db6
Update tour descriptions and line references for improved clarity in …
JosKWRubicon 707dac2
Refactor launch.json and tasks.json for consistency; update launchSet…
JosKWRubicon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| # Copilot instructions for Explain PowerShell | ||
|
|
||
| These instructions are for GitHub Copilot Chat/Edits when working in this repository. | ||
|
|
||
| ## Repo overview (what this project is) | ||
| - A PowerShell oneliner explainer. | ||
| - Backend: Azure Functions (.NET) in `explainpowershell.analysisservice/`. | ||
| - Frontend: Blazor in `explainpowershell.frontend/`. | ||
| - Shared models: `explainpowershell.models/`. | ||
| - Tests: Pester tests in `explainpowershell.analysisservice.tests/`. | ||
| - Infra: Bicep in `explainpowershell.azureinfra/`. | ||
|
|
||
| ## Architecture & flow | ||
| - The primary explanation is AST-based: | ||
| - `SyntaxAnalyzer` parses PowerShell into an AST and produces a list of `Explanation` nodes. | ||
| - The AI feature is additive and optional: | ||
| - The AST analysis returns immediately; the AI call is a separate endpoint invoked after the tree is available. | ||
| - Frontend triggers AI in the background so the UI remains responsive. | ||
|
|
||
| ## Editing guidelines (preferred behavior) | ||
| - Keep in mind this project is open source and intended to be cross platform. | ||
| - Follow existing code style and patterns. | ||
| - Favor readability and maintainability. | ||
| - Prefer small, surgical changes; avoid unrelated refactors. | ||
| - Preserve existing public APIs and JSON shapes unless explicitly requested. | ||
| - Keep AI functionality optional and non-blocking. | ||
| - If AI configuration is missing, the app should still work (AI can silently no-op). | ||
| - Use existing patterns in the codebase (logging, DI, options, error handling). | ||
| - Don’t add new external dependencies unless necessary and justified. | ||
|
|
||
| ## C# conventions | ||
| - Prefer async/await end-to-end. | ||
| - Handle nullability deliberately; avoid introducing new nullable warnings. | ||
| - Use `System.Text.Json` where the project already does; don’t mix serializers in the same flow unless required. | ||
|
|
||
| ## Unit tests | ||
| - Aim for high coverage on new features. | ||
| - Focus on behavior verification over implementation details. | ||
| - When adding tests, follow existing patterns in `explainpowershell.analysisservice.tests/`. | ||
|
|
||
| ## Building | ||
| - On fresh clones, run all code generators before building: `Get-ChildItem -Path $PSScriptRoot/explainpowershell.analysisservice/ -Recurse -Filter *_code_generator.ps1 | ForEach-Object { & $_.FullName }` | ||
|
|
||
| ## PowerShell / Pester conventions | ||
| - Keep tests deterministic and fast; avoid relying on external services unless explicitly an integration test. | ||
| - When adding tests, follow the existing Pester structure and naming. | ||
| - Before adding Pester tests, consider if the behavior can be verified in C# unit tests first. | ||
|
|
||
| ## Running locally | ||
| - For running Pester integration tests locally successfully it is necessary to run `.\bootstrap.ps1` from the repo root, it sets up the required data in Azurite, and calls code generators. | ||
| - For general debuging, running `.\bootstrap.ps1` once is also recommended. If Azurite is present and has helpldata, it is not necessary to run it again. | ||
| - You can load helper methods to test the functionapp locally by importing the following scripts in your PowerShell session: | ||
| ```powershell | ||
| . C:\Users\JosKoelewijn\GitNoOneDrive\explainpowershell/explainpowershell.analysisservice.tests/Invoke-SyntaxAnalyzer.ps1 | ||
| . C:\Users\JosKoelewijn\GitNoOneDrive\explainpowershell/explainpowershell.analysisservice.tests/Invoke-AiExplanation.ps1 | ||
| . C:\Users\JosKoelewijn\GitNoOneDrive\explainpowershell/explainpowershell.analysisservice.tests/Get-HelpDatabaseData.ps1 | ||
| . C:\Users\JosKoelewijn\GitNoOneDrive\explainpowershell/explainpowershell.analysisservice.tests/Get-MetaData.ps1 | ||
| ``` | ||
|
|
||
| ## How to validate changes | ||
| - Prefer the repo task: run the VS Code task named `run tests` (Pester). | ||
| - If you need a build check, use the VS Code `build` task. | ||
|
|
||
| ## Documentation | ||
| - When adding developer-facing features, also update or add a CodeTour in `.tours/` when it improves onboarding. | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| { | ||
| "$schema": "https://aka.ms/codetour-schema", | ||
| "title": "Tour of the AI explanation feature", | ||
| "steps": [ | ||
| { | ||
| "file": "explainpowershell.analysisservice/SyntaxAnalyzer.cs", | ||
| "description": "The AI explanation is intentionally *not* generated during the main AST analysis call.\n\nThis endpoint parses the PowerShell input into an AST and walks it with `AstVisitorExplainer` to produce the structured explanation nodes (the data that becomes the tree you see in the UI).\n\nThat AST-based result is returned quickly so the UI can render immediately.", | ||
| "line": 25 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.analysisservice/SyntaxAnalyzer.cs", | ||
| "description": "Key design choice: the AST endpoint always sets `AiExplanation` to an empty string.\n\nThe AI summary is fetched via a separate endpoint *after* the AST explanation tree is available. This keeps the main analysis deterministic and avoids coupling UI responsiveness to an external AI call.", | ||
| "line": 82 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.frontend/Pages/Index.razor.cs", | ||
| "description": "Frontend starts by calling the AST analysis endpoint (`SyntaxAnalyzer`).\n\nThis request returns the expanded code plus a flat list of explanation nodes (each has an `Id` and optional `ParentId`).", | ||
| "line": 120 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.frontend/Pages/Index.razor.cs", | ||
| "description": "Once the AST analysis result comes back, the frontend builds the explanation tree (based on `Id` / `ParentId`) for display in the `MudTreeView`.\n\nAt this point, the user already has a useful explanation from the AST visitor.", | ||
| "line": 152 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.frontend/Pages/Index.razor.cs", | ||
| "description": "Immediately after the tree is available, the AI request is kicked off *in the background*.\n\nNotice the fire-and-forget pattern (`_ = ...`) so the AST UI is not blocked while the AI endpoint runs.", | ||
| "line": 157 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.frontend/Pages/Index.razor.cs", | ||
| "description": "`LoadAiExplanationAsync` constructs a payload containing:\n- the original PowerShell code\n- the full AST analysis result\n\nThen it POSTs to the backend `AiExplanation` endpoint.\n\nIf the call fails, the UI silently continues without the AI summary (AI is treated as optional).", | ||
| "line": 160 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.frontend/Clients/SyntaxAnalyzerClient.cs", | ||
| "description": "This is the actual HTTP call that requests the AI explanation.\n\nIt sends both the code and the AST result so the backend can build a prompt that is grounded in the already-produced explanation nodes and help metadata.", | ||
| "line": 69 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.frontend/Pages/Index.razor", | ||
| "description": "The UI has a dedicated 'AI explanation' card.\n\nIt's only shown when either:\n- `AiExplanationLoading` is true (spinner), or\n- a non-empty `AiExplanation` has arrived.\n\nThis makes the feature feel additive: the AST explanation tree appears first, then the AI summary appears when ready.", | ||
| "line": 34 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.analysisservice/AiExplanationFunction.cs", | ||
| "description": "Backend entrypoint for the AI feature: an Azure Function named `AiExplanation`.\n\nIt accepts an `AiExplanationRequest` that includes both the PowerShell code and the `AnalysisResult` produced earlier by the AST endpoint.", | ||
| "line": 23 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.analysisservice/AiExplanationFunction.cs", | ||
| "description": "After validating/deserializing the request, this function delegates the real work to `IAiExplanationService.GenerateAsync(...)`.\n\nThis separation keeps the HTTP handler thin and makes the behavior easier to test.", | ||
| "line": 63 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.analysisservice/Program.cs", | ||
| "description": "The AI feature is wired up through DI.\n\nA `ChatClient` is registered only when configuration is present (`Endpoint`, `ApiKey`, `DeploymentName`) and `Enabled` is true. If not configured, the factory returns `null` and AI remains disabled.", | ||
| "line": 36 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.analysisservice/Services/AiExplanationOptions.cs", | ||
| "description": "AI behavior is controlled via `AiExplanationOptions`:\n- the system prompt (how the AI should behave)\n- an example prompt/response (few-shot guidance)\n- payload size guardrails (`MaxPayloadCharacters`)\n- request timeout (`RequestTimeoutSeconds`)\n\nThese settings are loaded from the `AiExplanation` config section.", | ||
| "line": 6 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.analysisservice/Services/AiExplanationService.cs", | ||
| "description": "First guardrail: if DI did not create a `ChatClient`, the service returns `(null, null)` and logs that AI is unavailable.\n\nThis is what makes the feature optional without breaking the main AST explanation flow.", | ||
| "line": 30 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.analysisservice/Services/AiExplanationService.cs", | ||
| "description": "Prompt grounding & size safety: the service builds a ‘slim’ version of the analysis result and serializes it to JSON for the prompt.\n\nIf the JSON is too large, it progressively reduces details (e.g., trims help fields, limits explanation count) so the request stays under `MaxPayloadCharacters`.", | ||
| "line": 57 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.analysisservice/Services/AiExplanationService.cs", | ||
| "description": "Finally, the service builds a chat message list and calls `CompleteChatAsync`.\n\nThe response is reduced to a best-effort single sentence (per the prompt), and the model name is returned too so the UI can display what model produced the summary.", | ||
| "line": 92 | ||
| }, | ||
| { | ||
| "file": "explainpowershell.analysisservice.tests/Invoke-AiExplanation.Tests.ps1", | ||
| "description": "The AI endpoint has Pester integration tests.\n\nNotably, there’s coverage for:\n- accepting a valid `AnalysisResult` payload\n- handling very large payloads (50KB+) gracefully (exercise the payload reduction path)\n- an end-to-end workflow: call SyntaxAnalyzer first, then call AiExplanation with that output.", | ||
| "line": 3 | ||
| } | ||
| ] | ||
| } |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,11 @@ | ||
| { | ||
| "recommendations": [ | ||
| "azurite.azurite", | ||
| "ms-azuretools.vscode-azurefunctions", | ||
| "ms-dotnettools.blazorwasm-companion", | ||
| "ms-dotnettools.csharp", | ||
| "ms-vscode.powershell", | ||
| "ms-dotnettools.vscode-dotnet-runtime", | ||
| "ms-dotnettools.blazorwasm-companion", | ||
| "vsls-contrib.codetour", | ||
| "derivitec-ltd.vscode-dotnet-adapter", | ||
| "ms-vscode.test-adapter-converter", | ||
| "hbenl.vscode-test-explorer", | ||
| "azurite.azurite" | ||
| "ms-vscode.powershell", | ||
| "vsls-contrib.codetour" | ||
| ] | ||
| } |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.