Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
Contributor
📝 WalkthroughWalkthroughAdds many .forge/skills documentation files and AGENTS.md; introduces TransientWeatherAlertException; updates NWS provider to build area/zone query parameters and to throw on non-success HTTP statuses (429/5xx treated as transient); WeatherAlertService records transient failures and rethrows. ChangesForge Skills and Weather Provider
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![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: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.forge/skills/messaging/SKILL.md (1)
298-298:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove incomplete line number.
Line 298 contains only the number "298" with no content. This appears to be a formatting artifact that should be removed.
🧹 Proposed fix
| Want single lib for mediator + messaging | Wolverine (replaces both Mediator and MassTransit) | -298🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.forge/skills/messaging/SKILL.md at line 298, Remove the stray standalone numeric token "298" from the SKILL.md markdown (it’s an empty/artifact line) so the document has no orphan line numbers; simply delete that line and verify surrounding markdown headings and numbering remain correct.
🧹 Nitpick comments (5)
.forge/skills/api-versioning/SKILL.md (1)
27-31: ⚡ Quick winClarify
AssumeDefaultVersionWhenUnspecifiedto avoid conflicting guidance.The sample sets
AssumeDefaultVersionWhenUnspecified = true, which can conflict with the “version from day one” principle by silently accepting unversioned requests. Consider adding a note that this is migration-friendly, and recommendfalsefor strict/public APIs.Suggested doc tweak
builder.Services.AddApiVersioning(options => { options.DefaultApiVersion = new ApiVersion(1, 0); - options.AssumeDefaultVersionWhenUnspecified = true; + // Use `true` only during migration from unversioned clients. + // For strict/public APIs, prefer `false` to require explicit versions. + options.AssumeDefaultVersionWhenUnspecified = false; options.ReportApiVersions = true; options.ApiVersionReader = new UrlSegmentApiVersionReader(); })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.forge/skills/api-versioning/SKILL.md around lines 27 - 31, The example currently sets AssumeDefaultVersionWhenUnspecified = true which can silently accept unversioned requests and conflict with a “version from day one” policy; update the SKILL.md snippet or adjacent text to clarify that AssumeDefaultVersionWhenUnspecified=true is migration-friendly for internal/back-compat scenarios but that public/strict APIs should set AssumeDefaultVersionWhenUnspecified = false (and explicitly document DefaultApiVersion and ReportApiVersions usage), or alternatively show both options with a short note recommending false for new/public APIs and true only for migration paths; reference the AddApiVersioning configuration and the AssumeDefaultVersionWhenUnspecified/DefaultApiVersion/ReportApiVersions keys so readers can find the exact snippet to change..forge/skills/modern-csharp/SKILL.md (1)
39-106: ⚖️ Poor tradeoffThe
fieldkeyword syntax is correct for C# 14; consider documenting thread-safety implications for lazy initialization.The
fieldcontextual keyword syntax shown here is correct and matches the official C# 14 implementation. The documentation examples are valid.However, the lazy initialization pattern on line 69 merits a brief caveat:
get => field ??= LoadProducts();If
Productsis accessed concurrently, multiple threads could callLoadProducts()simultaneously. Consider adding a note that this pattern is suitable for single-threaded scenarios or when the property is accessed under synchronization, and direct readers toLazy<T>for thread-safe lazy initialization in shared/concurrent contexts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.forge/skills/modern-csharp/SKILL.md around lines 39 - 106, The lazy-init example in ProductCatalog (the Products property using get => field ??= LoadProducts();) needs a short caveat about thread-safety: update the SKILL.md section near the ProductCatalog example to state that this pattern is not thread-safe and may call LoadProducts() multiple times if accessed concurrently, and recommend using Lazy<T> or explicit synchronization (lock) for shared/concurrent scenarios or indicate it’s suitable only for single-threaded contexts..forge/skills/health-check/SKILL.md (1)
34-294: 💤 Low valueConsider adding language specifiers to fenced code blocks.
The markdown linter flags 13 fenced code blocks without language specifiers. While these blocks render correctly, adding identifiers (e.g.,
text,yaml, or leaving the descriptor blocks unmarked with simple indentation) improves consistency and ensures proper rendering across all Markdown processors.This affects the Tool/Metric specification blocks, the quick health check template, and the anti-pattern examples.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.forge/skills/health-check/SKILL.md around lines 34 - 294, The markdown contains multiple fenced code blocks without language specifiers (e.g., the Tool/Metric specification blocks starting with "Tool: dotnet build --no-restore", the QUICK HEALTH (4 dimensions) block, and the anti-pattern examples), causing linter warnings; fix by adding appropriate language identifiers to each fenced block (for example use text, yaml, or bash as appropriate) so that the fenced blocks like the Tool/Metric blocks, the quick health check template, and the anti-pattern example blocks explicitly specify their language and satisfy the markdown linter..forge/skills/instinct-system/SKILL.md (1)
33-235: 💤 Low valueConsider adding language specifiers to workflow blocks.
Similar to the health-check skill, several fenced code blocks describing workflows and protocols lack language specifiers. Consider marking these as
textor using simple indentation for improved markdown consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.forge/skills/instinct-system/SKILL.md around lines 33 - 235, Several fenced code blocks in headings like OBSERVE, Instinct Storage Format, Confidence Adjustment Rules, ACTIVE SEEKING PROTOCOL and PROMOTION STEPS lack language specifiers; update each opening fence (e.g., the blocks starting with ``` or the workflow/example blocks) to include an appropriate language tag (suggest `text` for plain prose blocks, `markdown` for markdown examples, or `yaml` for the rule lists) so they render consistently—change each ``` to ```text (or the chosen tag) while leaving the block contents and closing ``` unchanged..forge/skills/context-discipline/SKILL.md (1)
34-188: 💤 Low valueConsider adding language specifiers to example blocks.
Ten fenced code blocks describing workflows and decision matrices lack language specifiers. Marking these as
textwould improve markdown rendering consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.forge/skills/context-discipline/SKILL.md around lines 34 - 188, Several fenced code blocks in the SKILL.md file (the blocks starting with "TASK: Understand how OrderService works", the "Subagent Offloading Decision Matrix", and the various examples/anti-pattern blocks) are missing language specifiers; update each triple-backtick fence to use a language tag (use "text" for plain prose blocks) so markdown renderers consistently apply formatting. Locate the fenced blocks by searching for the backtick-delimited blocks that begin with "TASK:", the decision matrix headers, or the anti-pattern examples and add the language marker immediately after the opening ``` for each block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.forge/skills/architecture-advisor/SKILL.md:
- Line 100: Several fenced code blocks in SKILL.md are missing language
identifiers and trigger the MD040 lint warning; update each example fence (the
plain ``` blocks at the example snippets) to include an explicit language token
such as ```text, ```bash, ```csharp, etc., matching the content of the block.
Locate the unlabeled fences around the example snippets and replace the opening
triple backticks with a language-specific fence (e.g., ```text for plain output,
```bash for shell commands, ```csharp for C# code) so all examples comply with
MD040 and improve readability.
In @.forge/skills/aspire/SKILL.md:
- Line 116: The fenced code block containing the solution tree (the block that
starts at the existing fence around "MyApp.slnx ... MyApp.AppHost/ Program.cs")
needs a language identifier to satisfy markdownlint MD040; edit the opening
triple-backtick for that block (as seen in SKILL.md near the tree content) to be
```text (or ```plaintext) so the block becomes a plain-text fenced block,
leaving the content and closing backticks unchanged.
In @.forge/skills/autonomous-loops/SKILL.md:
- Around line 33-69: The markdown contains multiple fenced code blocks (e.g.,
the BUILD-FIX LOOP example and other blocks referenced) without language
specifiers causing markdownlint MD040; update each triple-backtick fence to
include an appropriate language token (use "text" for pseudo-code or a specific
language like "bash", "csharp", etc. where suitable) so the blocks render and
lint correctly—search for fences around the BUILD-FIX LOOP and the other ranges
noted (73-85, 91-129, 133-145, 151-189, 195-228, 234-251, 257-280, 286-297,
303-312, 316-328, 332-342, 346-362, 366-376) and add the language specifier to
each opening ``` line.
In @.forge/skills/clean-architecture/SKILL.md:
- Line 26: The fenced code block currently written as ``` lacks a language
identifier; update that fenced code block (the triple-backtick block) to include
a language tag such as `text` (e.g., change ``` to ```text) so markdownlint
MD040 is satisfied and the block is explicitly typed.
In @.forge/skills/code-review-workflow/SKILL.md:
- Around line 35-44: The fenced code blocks in SKILL.md (e.g., the blocks
showing "→ detect_antipatterns", "→ get_diagnostics", and "→ get_public_api")
are unlabeled and trigger markdownlint MD040; update each triple-backtick fence
to include an explicit language identifier (for example use ```text or
```markdown) for every unlabeled block (including the other listed ranges such
as 48-51, 54-58, etc.) so syntax highlighting and linting pass; ensure you
change each fence that surrounds the snippets containing identifiers like
detect_antipatterns, get_diagnostics, and get_public_api to use a consistent
language label.
- Around line 56-57: Update the dependency-direction example that currently
reads "Verify: dependency direction is correct (Domain → nothing, Infra →
Domain, Api → Application)" so it matches this repo’s layering rule; replace
that example with the repo model "Config → Model → Services →
Repositories/Providers → Web/Workers, where each layer depends only on layers to
its left", or if you prefer to keep the original generic example, add an
explicit label saying "Generic example — not this repo's rule" to avoid
enforcing it; edit the sentence and any adjacent guidance in the same section so
the verification check and the "Flag: circular references, wrong-direction
dependencies" guidance use the repo’s layer names or clearly mark the example as
generic.
In @.forge/skills/de-sloppify/SKILL.md:
- Around line 52-54: Multiple fenced code blocks in SKILL.md (for example the
block containing `MCP: get_diagnostics(scope: "solution", severityFilter:
"warning")`) lack language identifiers; update each unlabeled triple-backtick
block to include an appropriate language token (e.g., text, bash, csharp) so
markdown lint MD040 is resolved. Locate the blocks shown in the review (the
example and the ranges covering 66-68, 71-83, 102-105, 143-145, 197-206,
210-222, 226-235, 239-249, 253-262) and change their opening ``` to ```text or
another suitable language identifier that matches the content. Ensure the
closing ``` remains unchanged and run the markdown linter to verify MD040 is
fixed.
In @.forge/skills/docker/SKILL.md:
- Line 63: Update the `.dockerignore` fenced code block by adding the language
tag "text" to the opening fence (i.e., change the fenced block for .dockerignore
to use ```text) so markdownlint MD040 warnings are suppressed; locate the fenced
block labeled for the .dockerignore example in the SKILL.md content and add the
language tag to the opening triple-backtick line.
- Around line 55-57: The HEALTHCHECK currently tries to start a second app
process via CMD
["dotnet","MyApp.Api.dll","--urls","http://localhost:8080/health/live"] which
will conflict with the ENTRYPOINT process; replace that CMD with a fast probe
(e.g., use curl -f http://localhost:8080/health/live || exit 1 or an equivalent
lightweight HTTP check) so the healthcheck only performs an HTTP GET against the
running service, and ensure the runtime image includes curl or use an
alternative probe tool; update the HEALTHCHECK directive (the line starting with
HEALTHCHECK and the CMD array) accordingly.
In @.forge/skills/messaging/SKILL.md:
- Around line 165-201: The saga should own its state directly instead of using
OrderSagaState: delete the OrderSagaState record, make OrderSaga inherit from
Saga (not Saga<OrderSagaState>), move the properties CustomerId and
PaymentReceived onto the OrderSaga class along with Id, change the Start method
signature to return (OrderSaga, ProcessPayment) and instantiate/populate an
OrderSaga (set Id and CustomerId) before returning it with the ProcessPayment
command, and ensure the Handle(PaymentCompleted) and Handle(PaymentFailed)
methods update the instance properties (e.g., set PaymentReceived, call
MarkCompleted()) and return the corresponding commands
(CompleteOrder/CancelOrder) using the saga's Id.
In @.forge/skills/model-selection/SKILL.md:
- Line 33: The markdown fenced code blocks in SKILL.md lack language tags
causing MD040 lint warnings; update each fenced block referenced (the blocks
containing "ROUTINE TASKS → Sonnet 4.6", "PHASE 1: PLAN (Opus 4.6)", the "Claude
Code CLI" CLI snippet, the "SUBAGENT: \"Find all authentication middleware in
the project\"" block, and the examples starting with "// BAD — Opus...", "// BAD
— Sonnet...", and "// BAD — all subagents use Opus") by adding appropriate
language identifiers (use ```text for diagrams/lists and ```bash for CLI
examples) so each fenced block has a language label.
In @.forge/skills/modern-csharp/SKILL.md:
- Around line 108-122: Replace the invalid extension members example (the
`public extension OrderExtensions for Order` block) with the proper C# 14 form:
wrap the extension block inside a static class (e.g., `OrderExtensions`) and use
the block-based `extension(Order order)` receiver syntax, updating member bodies
to reference the explicit receiver (e.g., change `Total` to `order.Total` in
`TotalWithTax`, `IsHighValue`, and `ToSummary`) so the example compiles with C#
14.
In @.forge/skills/project-setup/SKILL.md:
- Line 86: Several fenced code blocks in SKILL.md are missing language
identifiers (MD040). For each untyped triple-backtick block (examples include
the snippet containing "get_project_graph" and the other occurrences at the
noted locations), add an appropriate language tag (e.g., ```text, ```bash,
```markdown, or ```yaml) immediately after the opening ``` so markdownlint
passes; update each opening fence for the blocks around the lines referenced
(including the snippet with "get_project_graph") to use the correct tag while
leaving the block contents unchanged.
In @.forge/skills/project-structure/SKILL.md:
- Line 26: Two fenced code blocks use unlabeled triple backticks (```) which
triggers markdownlint MD040; update each unlabeled block in SKILL.md (the
triple-backtick blocks at the reported locations) to specify a language, e.g.,
change ``` to ```text for the tree/structure snippets so both fenced blocks
include a language label.
In @.forge/skills/self-correction-loop/SKILL.md:
- Around line 32-56: Label every fenced code block in
.forge/skills/self-correction-loop/SKILL.md with a language identifier (e.g.,
change ``` to ```text or ```markdown) so markdownlint rule MD040 is satisfied;
specifically update the fenced blocks under the section headings DETECT,
ACKNOWLEDGE, GENERALIZE, CHECK, STORE, CONFIRM and the additional blocks noted
(the other ranges mentioned in the review) to include a language token.
In @.forge/skills/session-management/SKILL.md:
- Around line 33-65: The markdown contains multiple unlabeled fenced code blocks
(e.g., the protocol-like blocks under "STEP 1: Load Handoff", "STEP 2: Load
Memory", "STEP 3: Load Instincts", "STEP 4: Detect .NET Solution", "STEP 5:
Present Summary" and other sections listed) which trigger MD040; update each
triple-backtick fence to include an explicit language tag (use text for
protocol/plain sections or a more specific tag if appropriate) across the
flagged ranges (71-101, 107-123, 129-140, 180-191, 197-208, 214-231, 237-250,
254-265, 269-282, 286-297, 301-311, 315-331) so every code fence is labeled and
the linter no longer reports MD040.
In `@Providers/Resgrid.Providers.Weather/NwsWeatherAlertProvider.cs`:
- Around line 49-54: The URL builder appends "?" unconditionally which breaks
URLs when WeatherAlertSource.CustomEndpoint (the initial url variable) already
contains query parameters; update the logic that builds the final url (the block
that inspects stateCodes, zoneCodes and queryParams) to detect if url already
contains a '?' and use '&' to append additional parameters instead of '?',
ensuring you join queryParams with '&' and append with either "?" or "&"
accordingly (or use Uri/UriBuilder to merge query strings) so final URLs are
valid whether or not the CustomEndpoint already has query params.
- Around line 101-109: The current logic in NwsWeatherAlertProvider treats
rate-limit/server errors and client errors identically by throwing the same
HttpRequestException; change it so retryable failures (when
(int)response.StatusCode == 429 || (int)response.StatusCode >= 500) throw a
distinct transient exception that the poller will retry (e.g., throw a
TransientHttpException or a custom RetryableHttpRequestException) while keeping
client/permanent failures (4xx) as HttpRequestException so callers can
distinguish and mark the source failed; update exception types where
response.StatusCode and errorMsg are used and ensure the
poller/ProcessAllActiveSourcesAsync handles the new transient exception
appropriately.
---
Outside diff comments:
In @.forge/skills/messaging/SKILL.md:
- Line 298: Remove the stray standalone numeric token "298" from the SKILL.md
markdown (it’s an empty/artifact line) so the document has no orphan line
numbers; simply delete that line and verify surrounding markdown headings and
numbering remain correct.
---
Nitpick comments:
In @.forge/skills/api-versioning/SKILL.md:
- Around line 27-31: The example currently sets
AssumeDefaultVersionWhenUnspecified = true which can silently accept unversioned
requests and conflict with a “version from day one” policy; update the SKILL.md
snippet or adjacent text to clarify that
AssumeDefaultVersionWhenUnspecified=true is migration-friendly for
internal/back-compat scenarios but that public/strict APIs should set
AssumeDefaultVersionWhenUnspecified = false (and explicitly document
DefaultApiVersion and ReportApiVersions usage), or alternatively show both
options with a short note recommending false for new/public APIs and true only
for migration paths; reference the AddApiVersioning configuration and the
AssumeDefaultVersionWhenUnspecified/DefaultApiVersion/ReportApiVersions keys so
readers can find the exact snippet to change.
In @.forge/skills/context-discipline/SKILL.md:
- Around line 34-188: Several fenced code blocks in the SKILL.md file (the
blocks starting with "TASK: Understand how OrderService works", the "Subagent
Offloading Decision Matrix", and the various examples/anti-pattern blocks) are
missing language specifiers; update each triple-backtick fence to use a language
tag (use "text" for plain prose blocks) so markdown renderers consistently apply
formatting. Locate the fenced blocks by searching for the backtick-delimited
blocks that begin with "TASK:", the decision matrix headers, or the anti-pattern
examples and add the language marker immediately after the opening ``` for each
block.
In @.forge/skills/health-check/SKILL.md:
- Around line 34-294: The markdown contains multiple fenced code blocks without
language specifiers (e.g., the Tool/Metric specification blocks starting with
"Tool: dotnet build --no-restore", the QUICK HEALTH (4 dimensions) block, and
the anti-pattern examples), causing linter warnings; fix by adding appropriate
language identifiers to each fenced block (for example use text, yaml, or bash
as appropriate) so that the fenced blocks like the Tool/Metric blocks, the quick
health check template, and the anti-pattern example blocks explicitly specify
their language and satisfy the markdown linter.
In @.forge/skills/instinct-system/SKILL.md:
- Around line 33-235: Several fenced code blocks in headings like OBSERVE,
Instinct Storage Format, Confidence Adjustment Rules, ACTIVE SEEKING PROTOCOL
and PROMOTION STEPS lack language specifiers; update each opening fence (e.g.,
the blocks starting with ``` or the workflow/example blocks) to include an
appropriate language tag (suggest `text` for plain prose blocks, `markdown` for
markdown examples, or `yaml` for the rule lists) so they render
consistently—change each ``` to ```text (or the chosen tag) while leaving the
block contents and closing ``` unchanged.
In @.forge/skills/modern-csharp/SKILL.md:
- Around line 39-106: The lazy-init example in ProductCatalog (the Products
property using get => field ??= LoadProducts();) needs a short caveat about
thread-safety: update the SKILL.md section near the ProductCatalog example to
state that this pattern is not thread-safe and may call LoadProducts() multiple
times if accessed concurrently, and recommend using Lazy<T> or explicit
synchronization (lock) for shared/concurrent scenarios or indicate it’s suitable
only for single-threaded contexts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2f9c7682-213c-480e-af97-cbb5f4d6fa2b
📒 Files selected for processing (49)
.forge/skills/80-20-review/SKILL.md.forge/skills/api-versioning/SKILL.md.forge/skills/architecture-advisor/SKILL.md.forge/skills/aspire/SKILL.md.forge/skills/authentication/SKILL.md.forge/skills/autonomous-loops/SKILL.md.forge/skills/caching/SKILL.md.forge/skills/ci-cd/SKILL.md.forge/skills/clean-architecture/SKILL.md.forge/skills/code-review-workflow/SKILL.md.forge/skills/configuration/SKILL.md.forge/skills/container-publish/SKILL.md.forge/skills/context-discipline/SKILL.md.forge/skills/convention-learner/SKILL.md.forge/skills/ddd/SKILL.md.forge/skills/de-sloppify/SKILL.md.forge/skills/dependency-injection/SKILL.md.forge/skills/docker/SKILL.md.forge/skills/ef-core/SKILL.md.forge/skills/error-handling/SKILL.md.forge/skills/health-check/SKILL.md.forge/skills/httpclient-factory/SKILL.md.forge/skills/instinct-system/SKILL.md.forge/skills/learning-log/SKILL.md.forge/skills/logging/SKILL.md.forge/skills/messaging/SKILL.md.forge/skills/migration-workflow/SKILL.md.forge/skills/minimal-api/SKILL.md.forge/skills/model-selection/SKILL.md.forge/skills/modern-csharp/SKILL.md.forge/skills/openapi/SKILL.md.forge/skills/opentelemetry/SKILL.md.forge/skills/project-setup/SKILL.md.forge/skills/project-structure/SKILL.md.forge/skills/resilience/SKILL.md.forge/skills/scaffolding/SKILL.md.forge/skills/scalar/SKILL.md.forge/skills/security-scan/SKILL.md.forge/skills/self-correction-loop/SKILL.md.forge/skills/serilog/SKILL.md.forge/skills/session-management/SKILL.md.forge/skills/split-memory/SKILL.md.forge/skills/testing/SKILL.md.forge/skills/verification-loop/SKILL.md.forge/skills/vertical-slice/SKILL.md.forge/skills/workflow-mastery/SKILL.md.forge/skills/wrap-up-ritual/SKILL.mdAGENTS.mdProviders/Resgrid.Providers.Weather/NwsWeatherAlertProvider.cs
|
|
||
| Organize by feature, not by layer. Each operation is a self-contained slice. | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Specify fence languages for all example blocks (MD040).
Several code fences are missing language identifiers. Adding explicit types (text, csharp, bash, etc.) will clear lint warnings and improve readability.
Also applies to: 117-117, 132-132, 147-147, 175-175, 190-190, 208-208, 226-226
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 100-100: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.forge/skills/architecture-advisor/SKILL.md at line 100, Several fenced code
blocks in SKILL.md are missing language identifiers and trigger the MD040 lint
warning; update each example fence (the plain ``` blocks at the example
snippets) to include an explicit language token such as ```text, ```bash,
```csharp, etc., matching the content of the block. Locate the unlabeled fences
around the example snippets and replace the opening triple backticks with a
language-specific fence (e.g., ```text for plain output, ```bash for shell
commands, ```csharp for C# code) so all examples comply with MD040 and improve
readability.
|
|
||
| ### Solution Structure with Aspire | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language identifier to the fenced block at Line 116.
This trips markdownlint MD040. Use text (or plaintext) for the solution tree block.
Suggested patch
-```
+```text
MyApp.slnx
├── MyApp.AppHost/ # Aspire orchestrator
│ └── Program.cs
@@
-```
+```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 116-116: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.forge/skills/aspire/SKILL.md at line 116, The fenced code block containing
the solution tree (the block that starts at the existing fence around
"MyApp.slnx ... MyApp.AppHost/ Program.cs") needs a language identifier to
satisfy markdownlint MD040; edit the opening triple-backtick for that block (as
seen in SKILL.md near the tree content) to be ```text (or ```plaintext) so the
block becomes a plain-text fenced block, leaving the content and closing
backticks unchanged.
Comment on lines
+33
to
+69
| ``` | ||
| BUILD-FIX LOOP: | ||
| max_iterations = 5 | ||
| previous_errors = [] | ||
|
|
||
| for iteration in 1..max_iterations: | ||
| result = dotnet build [project/solution] | ||
|
|
||
| if result.exit_code == 0: | ||
| report "BUILD PASS after {iteration} iteration(s)" | ||
| return PASS | ||
|
|
||
| errors = parse_errors(result.output) | ||
|
|
||
| if errors == previous_errors: | ||
| report "STUCK — same {len(errors)} error(s) after fix attempt" | ||
| report "Errors: {errors}" | ||
| return STUCK | ||
|
|
||
| if len(errors) > len(previous_errors) and iteration > 1: | ||
| report "REGRESSING — {len(errors)} errors, up from {len(previous_errors)}" | ||
| report "Last fix introduced new errors. Reverting iteration {iteration}." | ||
| revert_last_changes() | ||
| return REGRESSION | ||
|
|
||
| report "Iteration {iteration}: {len(errors)} error(s) found" | ||
| for error in errors: | ||
| category = categorize(error) | ||
| fix = determine_fix(error, category) | ||
| apply(fix) | ||
| report " Fixed {error.code}: {error.message} → {fix.description}" | ||
|
|
||
| previous_errors = errors | ||
|
|
||
| report "MAX ITERATIONS reached with {len(errors)} error(s) remaining" | ||
| return FAIL | ||
| ``` |
Contributor
There was a problem hiding this comment.
Specify languages for the unlabeled fenced code sections.
Several fenced blocks are missing language specifiers, which is triggering markdownlint MD040. Adding text (or specific languages where appropriate) will fix this cleanly.
Also applies to: 73-85, 91-129, 133-145, 151-189, 195-228, 234-251, 257-280, 286-297, 303-312, 316-328, 332-342, 346-362, 366-376
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.forge/skills/autonomous-loops/SKILL.md around lines 33 - 69, The markdown
contains multiple fenced code blocks (e.g., the BUILD-FIX LOOP example and other
blocks referenced) without language specifiers causing markdownlint MD040;
update each triple-backtick fence to include an appropriate language token (use
"text" for pseudo-code or a specific language like "bash", "csharp", etc. where
suitable) so the blocks render and lint correctly—search for fences around the
BUILD-FIX LOOP and the other ranges noted (73-85, 91-129, 133-145, 151-189,
195-228, 234-251, 257-280, 286-297, 303-312, 316-328, 332-342, 346-362, 366-376)
and add the language specifier to each opening ``` line.
|
|
||
| ### Project Layout | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language identifier to the fenced code block
The code fence at Line 26 is missing a language tag; adding one (e.g., text) removes markdownlint MD040 noise.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 26-26: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.forge/skills/clean-architecture/SKILL.md at line 26, The fenced code block
currently written as ``` lacks a language identifier; update that fenced code
block (the triple-backtick block) to include a language tag such as `text`
(e.g., change ``` to ```text) so markdownlint MD040 is satisfied and the block
is explicitly typed.
Comment on lines
+35
to
+44
| ``` | ||
| → detect_antipatterns (file: each changed .cs file) | ||
| Catch: async void, sync-over-async, DateTime.Now, new HttpClient(), broad catch, etc. | ||
|
|
||
| → get_diagnostics (scope: file, path: each changed file) | ||
| Catch: new compiler warnings, nullability issues, unused variables | ||
|
|
||
| → get_public_api (typeName: each modified type) | ||
| Check: API surface changes — new public members, removed members, signature changes | ||
| ``` |
Contributor
There was a problem hiding this comment.
Fix unlabeled fenced code blocks to satisfy markdownlint MD040.
Multiple fenced blocks in this file are missing language identifiers. Adding explicit languages (text, markdown, etc.) will clear lint warnings and improve syntax highlighting.
Also applies to: 48-51, 54-58, 61-65, 142-146, 149-155, 161-165, 167-172, 176-181, 183-188, 192-196, 198-205, 209-212, 214-220
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.forge/skills/code-review-workflow/SKILL.md around lines 35 - 44, The fenced
code blocks in SKILL.md (e.g., the blocks showing "→ detect_antipatterns", "→
get_diagnostics", and "→ get_public_api") are unlabeled and trigger markdownlint
MD040; update each triple-backtick fence to include an explicit language
identifier (for example use ```text or ```markdown) for every unlabeled block
(including the other listed ranges such as 48-51, 54-58, etc.) so syntax
highlighting and linting pass; ensure you change each fence that surrounds the
snippets containing identifiers like detect_antipatterns, get_diagnostics, and
get_public_api to use a consistent language label.
|
|
||
| ### Solution Layout | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Specify fence languages for unlabeled code blocks
Both unlabeled fenced blocks should include a language (text is fine for tree/structure snippets) to satisfy markdownlint MD040.
Also applies to: 181-181
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 26-26: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.forge/skills/project-structure/SKILL.md at line 26, Two fenced code blocks
use unlabeled triple backticks (```) which triggers markdownlint MD040; update
each unlabeled block in SKILL.md (the triple-backtick blocks at the reported
locations) to specify a language, e.g., change ``` to ```text for the
tree/structure snippets so both fenced blocks include a language label.
Comment on lines
+32
to
+56
| ``` | ||
| 1. DETECT — User says something like: | ||
| - "No, use X instead of Y" | ||
| - "We don't do it that way here" | ||
| - "That's wrong, it should be..." | ||
| - "Always/Never do X in this project" | ||
| - "Remember this for next time" | ||
|
|
||
| 2. ACKNOWLEDGE — Confirm understanding of the correction | ||
| "Got it — using HybridCache instead of IMemoryCache." | ||
|
|
||
| 3. GENERALIZE — Extract the class-level rule | ||
| Specific: "Don't use IMemoryCache in the Orders endpoint" | ||
| General: "Always use HybridCache instead of IMemoryCache — it provides | ||
| stampede protection and L1+L2 caching out of the box." | ||
|
|
||
| 4. CHECK — Scan MEMORY.md for existing related rules | ||
| - If a related rule exists, UPDATE it (broader scope, better wording) | ||
| - If no related rule exists, ADD a new one under the right category | ||
|
|
||
| 5. STORE — Write to MEMORY.md under the appropriate category | ||
|
|
||
| 6. CONFIRM — Tell the user what was captured | ||
| "Added to Memory > Data Access: Always use HybridCache over IMemoryCache." | ||
| ``` |
Contributor
There was a problem hiding this comment.
Label fenced blocks with a language to clear MD040 warnings.
These code fences should include language identifiers (text/markdown) for lint compliance and better rendering.
Also applies to: 86-99, 105-112, 122-133, 137-143, 147-155, 159-165
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.forge/skills/self-correction-loop/SKILL.md around lines 32 - 56, Label
every fenced code block in .forge/skills/self-correction-loop/SKILL.md with a
language identifier (e.g., change ``` to ```text or ```markdown) so markdownlint
rule MD040 is satisfied; specifically update the fenced blocks under the section
headings DETECT, ACKNOWLEDGE, GENERALIZE, CHECK, STORE, CONFIRM and the
additional blocks noted (the other ranges mentioned in the review) to include a
language token.
Comment on lines
+33
to
+65
| ``` | ||
| STEP 1: Load Handoff | ||
| → Check for .claude/handoff.md | ||
| → If found: read and summarize pending work | ||
| → If not found: note "No handoff file — starting fresh" | ||
|
|
||
| STEP 2: Load Memory | ||
| → Check for MEMORY.md (project root or .claude/) | ||
| → If found: scan for rules relevant to the likely task | ||
| → If not found: note "No memory file — will create on first learning" | ||
|
|
||
| STEP 3: Load Instincts | ||
| → Check for .claude/instincts.md | ||
| → If found: load instincts at 0.7+ into active context | ||
| → If not found: note "No instincts file — will create on first observation" | ||
|
|
||
| STEP 4: Detect .NET Solution | ||
| → Search for .slnx files in current directory | ||
| → If not found, search for .sln files | ||
| → If not found, search parent directories (up to 3 levels) | ||
| → If not found, search child directories (1 level) | ||
| → If found: confirm MCP tools are connected | ||
| → If not found: warn "No solution detected — MCP tools unavailable" | ||
|
|
||
| STEP 5: Present Summary | ||
| "Session context loaded: | ||
| - Last session: [summary from handoff or 'no previous session'] | ||
| - Pending tasks: [list from handoff or 'none'] | ||
| - Active rules: [count from MEMORY.md] | ||
| - Active instincts: [count at 0.7+] | ||
| - Solution: [solution name and path, or 'not detected'] | ||
| Ready to continue. What would you like to work on?" | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add missing code-fence language tags throughout this document.
Unlabeled fenced blocks are triggering MD040. Please add explicit fence languages (likely text for protocol blocks) across the flagged sections.
Also applies to: 71-101, 107-123, 129-140, 180-191, 197-208, 214-231, 237-250, 254-265, 269-282, 286-297, 301-311, 315-331
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.forge/skills/session-management/SKILL.md around lines 33 - 65, The markdown
contains multiple unlabeled fenced code blocks (e.g., the protocol-like blocks
under "STEP 1: Load Handoff", "STEP 2: Load Memory", "STEP 3: Load Instincts",
"STEP 4: Detect .NET Solution", "STEP 5: Present Summary" and other sections
listed) which trigger MD040; update each triple-backtick fence to include an
explicit language tag (use text for protocol/plain sections or a more specific
tag if appropriate) across the flagged ranges (71-101, 107-123, 129-140,
180-191, 197-208, 214-231, 237-250, 254-265, 269-282, 286-297, 301-311, 315-331)
so every code fence is labeled and the linter no longer reports MD040.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Core/Resgrid.Services/WeatherAlertService.cs (1)
244-252: ⚡ Quick winConsider setting
IsFailure = falsefor consistency.The transient exception handler leaves
IsFailureunchanged, which can create confusion if a source previously had a permanent failure. In that scenario,IsFailureremainstruewhileErrorMessageindicates a transient issue, which seems contradictory.Since transient failures (rate limits, server errors) are fundamentally non-permanent and should be retried, explicitly setting
IsFailure = falsewould better communicate that the source is not considered "broken."💡 Proposed clarification
catch (TransientWeatherAlertException ex) { // Transient failures (rate-limit, server errors) — record the error // but don't flag the source as permanently failed so it will be retried. source.LastPollUtc = DateTime.UtcNow; + source.IsFailure = false; source.ErrorMessage = $"Transient: {ex.Message}"; await _weatherAlertSourceRepository.UpdateAsync(source, ct, true); throw; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Resgrid.Services/WeatherAlertService.cs` around lines 244 - 252, In the TransientWeatherAlertException catch block inside WeatherAlertService (the handler that currently sets source.LastPollUtc and source.ErrorMessage and calls _weatherAlertSourceRepository.UpdateAsync), explicitly set source.IsFailure = false before calling UpdateAsync so transient errors don't leave a previously-failed source marked as permanently failed; keep the rest of the logic (LastPollUtc, ErrorMessage, await _weatherAlertSourceRepository.UpdateAsync(source, ct, true); and rethrow) the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Core/Resgrid.Services/WeatherAlertService.cs`:
- Around line 244-252: In the TransientWeatherAlertException catch block inside
WeatherAlertService (the handler that currently sets source.LastPollUtc and
source.ErrorMessage and calls _weatherAlertSourceRepository.UpdateAsync),
explicitly set source.IsFailure = false before calling UpdateAsync so transient
errors don't leave a previously-failed source marked as permanently failed; keep
the rest of the logic (LastPollUtc, ErrorMessage, await
_weatherAlertSourceRepository.UpdateAsync(source, ct, true); and rethrow) the
same.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 95a2ce5b-489d-4fae-8cd8-5924a8643d80
📒 Files selected for processing (3)
Core/Resgrid.Model/TransientWeatherAlertException.csCore/Resgrid.Services/WeatherAlertService.csProviders/Resgrid.Providers.Weather/NwsWeatherAlertProvider.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Providers/Resgrid.Providers.Weather/NwsWeatherAlertProvider.cs
Member
Author
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Merged
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
1 participant
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 by CodeRabbit
Documentation
Bug Fixes