elastic / elastic/docs-builder

[Feature Request]: Improve error messages for applies-to problems

Open
#3,009 2 comments 0 reactions 0 assignees View on GitHub
ai-triaged ai:eng-question ai:writer-question enhancement needs triage stale
Dominant language
C#
Stars
24
Forks
44
Avg merge
1d 7h
Merged PRs (30d)
146

Description

### Prerequisites

- [x] I have searched existing issues to ensure this feature hasn't already been requested
- [x] I have tested using the latest version of docs-builder

### What problem are you trying to solve?

The error messages that occur when there's an applies-to formatting error are very unhelpful.
For example in https://github.com/elastic/elasticsearch/pull/144885 the message that appeared in the CI failure was:

```
Error: Unhandled IsolatedBuildService exception: While scanning a plain scalar value, found invalid mapping.
(Line: 1, Col: 26, Idx: 25) - (Line: 1, Col: 26, Idx: 25): While scanning a plain scalar value, found invalid mapping.
```

The actual fix was https://github.com/elastic/elasticsearch/pull/144885/changes/43de107169c00bbba4939506c20f0d3c42c8e8cf

### Proposed Solution

Add more detail to help pinpoint the problem. Here's an example of a quick AI-generated plan:

# Plan

Improved `{applies-item}` YAML error messages

## Problem

- Invalid one-line forms such as `stack: ga 9.4, serverless: ga` are not valid YAML as a single scalar; the fix in the linked PR uses a JSON object: `{ "stack": "ga 9.4", "serverless": "ga" }`.
- Today, `AppliesItemBlock.FinalizeAndValidate` in `src/Elastic.Markdown/Myst/Directives/AppliesSwitch/AppliesSwitchBlock.cs` calls `**GenerateSyncKey` before `ParseApplicableTo**` (lines 62–68). A bad argument can therefore fail during sync-key deserialization first, which complicates diagnostics and can surface as a raw YamlDotNet message without actionable guidance.
- `YamlSerialization.Deserialize` in `src/Elastic.Markdown/Myst/YamlSerialization.cs` already wraps `YamlException` in `InvalidOperationException` with a `yamlSourceContext` prefix, but does not add applicability-specific hints.

## Approach

### 1. Reorder `FinalizeAndValidate` (directive behavior)

In `AppliesSwitchBlock.cs` (`src/Elastic.Markdown/Myst/Directives/AppliesSwitch/AppliesSwitchBlock.cs`):

- Run `**ParseApplicableTo` first** (when `AppliesToDefinition` is non-empty).
- Set `**SyncKey`** to `Prop("sync") ??` a new helper that hashes from `**ApplicableTo**` when `AppliesTo` is non-null (e.g. `GenerateSyncKeyFromApplicable(ApplicableTo)`), mirroring the existing `ShortId.Create(applicableTo.ToString())` logic in `[GenerateSyncKey](src/Elastic.Markdown/Myst/Directives/AppliesSwitch/AppliesSwitchBlock.cs)` (lines 93–99).
- Keep the **public** `GenerateSyncKey(string, ProductsConfiguration)` used by tests: implement as deserialize + `GenerateSyncKeyFromApplicable` so behavior stays centralized.

**Edge case:** If parsing fails and returns null, `SyncKey` may be null unless `sync` prop is set—acceptable for invalid tabs; document in code comment if tests need adjustment.

### 2. Contextual hint in `YamlSerialization` (optional, targeted)

In `[YamlSerialization.cs](src/Elastic.Markdown/Myst/YamlSerialization.cs)`, inside the `YamlException` catch when `yamlSourceContext` is non-null:

- Only when the **target type is `ApplicableTo`** (use `typeof(T) == typeof(ApplicableTo)` or a dedicated overload—prefer **minimal API surface**: branch inside existing generic method) **and** `yamlSourceContext` contains `applies-item` (case-insensitive).
- Append a **short hint** when both:
- The exception message suggests structural YAML issues (e.g. contains `invalid mapping` or `plain scalar`), **and**
- The input string **looks like comma-separated top-level keys** (heuristic: regex such as `,\s*\w[\w-]*\s*:`), which distinguishes:
- **Bad:** `stack: ga 9.4, serverless: ga`
- **Still valid** patterns like `stack: preview 9.0, ga 9.1` (comma without `, word:`) used in `[ApplicabilitySwitchTests](tests/Elastic.Markdown.Tests/Directives/ApplicabilitySwitchTests.cs)` (`NormalizesSyncKeyOrder`).

**Hint copy (concise, style-neutral):** Explain that multiple keys on one line should use a **JSON object** or a **braced YAML mapping**; avoid implying that *all* commas are wrong.

### 3. Directive-level message polish (optional)

In `ParseApplicableTo`, optionally prefix `EmitError` with a clearer title, e.g. `Unable to parse {applies-item} applicability` plus the raw argument snippet, so diagnostics in the panel read better than a bare YamlDotNet line. The inner exception (with hint from step 2) remains attached.

### 4. Documentation (optional, small)

In `docs/syntax/applies-switch.md`, add a short **“Troubleshooting”** or **“Multiple keys on one line”** note: prefer JSON or `{ key: value, ... }` braced form when specifying several keys—aligned with existing “Multiple applies_to definitions” section (lines 41–77) so it does not duplicate the whole page.

### 5. Tests

- Add or extend a test in `ApplicabilitySwitchTests`(`tests/Elastic.Markdown.Tests/Directives/ApplicabilitySwitchTests.cs`) (or a small dedicated test file) that feeds an **invalid** comma-separated two-key string and asserts the emitted diagnostic **includes** the hint substring (or that `InvalidOperationException.Message` contains it if testing at `YamlSerialization` level).
- Run existing `**GenerateSyncKey`** / `**NormalizesSyncKeyOrder**` tests to ensure no regression.

## Files to touch

| File | Change |
| ------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------ |
| `[src/Elastic.Markdown/Myst/YamlSerialization.cs](src/Elastic.Markdown/Myst/YamlSerialization.cs)` | ApplicableTo + applies-item hint logic after `YamlException` |
| `[src/Elastic.Markdown/Myst/Directives/AppliesSwitch/AppliesSwitchBlock.cs](src/Elastic.Markdown/Myst/Directives/AppliesSwitch/AppliesSwitchBlock.cs)` | Reorder finalize; `GenerateSyncKeyFromApplicable`; refactor public `GenerateSyncKey` |
| `[docs/syntax/applies-switch.md](docs/syntax/applies-switch.md)` | Optional short troubleshooting blurb |
| `[tests/Elastic.Markdown.Tests/Directives/ApplicabilitySwitchTests.cs](tests/Elastic.Markdown.Tests/Directives/ApplicabilitySwitchTests.cs)` | New assertion for hint / invalid input |

## Out of scope

- Changing the `ApplicableTo` schema or Markdig directive syntax.
- Broad hints for every YAML failure site (front matter, settings YAML, etc.)—keep this scoped to `**ApplicableTo**` + `**applies-item**` context unless you explicitly expand later.

### Examples and Research

_No response_

### Alternative Solutions

_No response_

### Additional Context

_No response_

### How important is this feature to you?

Nice to have

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.