Conversation
📝 WalkthroughWalkthroughThe changes add a new API configuration model and resolution flow to the documentation builder. Sequence DiagramsequenceDiagram
actor ConfigBuilder as Configuration Builder
participant CF as ConfigurationFile
participant AC as ApiConfiguration
participant FS as File System
participant DI as Diagnostics
participant RAC as ResolvedApiConfiguration
ConfigBuilder->>CF: Process DocumentationSetFile.Api entries
loop For each API entry
CF->>AC: Read ApiConfiguration
CF->>AC: Validate (IsValid)
alt valid
AC-->>CF: valid
CF->>FS: Resolve Template (if configured)
alt template exists
FS-->>CF: TemplateFile (IFileInfo)
else missing
CF->>DI: Emit warning, drop template
end
CF->>AC: GetSpecPaths()
AC-->>CF: spec path(s)
loop each spec path
CF->>FS: Resolve spec file
alt exists
FS-->>CF: SpecFile (IFileInfo)
else missing
CF->>DI: Emit error (missing spec)
end
end
alt at least one spec resolved
CF->>RAC: Create ResolvedApiConfiguration(ProductKey, SpecFiles, TemplateFile)
RAC-->>CF: resolvedConfig
CF->>CF: Store in ApiConfigurations
CF->>CF: Store primary spec in OpenApiSpecifications
else none resolved
CF->>DI: Emit error, skip product
end
else invalid
AC-->>CF: invalid
CF->>DI: Emit error, skip product
end
end
Suggested labels
🚥 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✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Elastic.Documentation.Configuration/Toc/ApiConfiguration.cs`:
- Around line 41-43: The IsValid getter currently treats empty or whitespace
Spec and Specs entries as valid; update IsValid to require non-empty,
non-whitespace values by checking that Spec is not null or whitespace (use
string.IsNullOrWhiteSpace) and that Specs contains at least one entry that is
not null/whitespace (e.g., Specs != null && Specs.Any(s =>
!string.IsNullOrWhiteSpace(s))); also preserve the mutual-exclusion check so a
non-empty Spec and a non-empty Specs collection cannot both be present at the
same time.
In `@src/Elastic.Documentation.Configuration/Toc/ApiConfigurationConverter.cs`:
- Around line 37-76: The parser can stall when values have unexpected shapes;
update ApiConfigurationConverter so each case for "spec", "template", and
"specs" always advances past the value (or delegates to rootDeserializer) even
if the token isn't the expected type, and use parser.SkipThisAndNestedEvents()
to consume unknown or nested values: for "spec"/"template" if parser.Current is
a Scalar set the property and MoveNext(), else call rootDeserializer.Deserialize
or call parser.SkipThisAndNestedEvents() and then MoveNext(); for "specs" if not
SequenceStart skip the entire value via parser.SkipThisAndNestedEvents(); and in
the default branch replace the simple MoveNext() with
parser.SkipThisAndNestedEvents() to avoid stalling.
🪄 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 Plus
Run ID: 6f050587-1c01-4420-9bf5-054e4bbca9c5
📒 Files selected for processing (8)
src/Elastic.Documentation.Configuration/Builder/ConfigurationFile.cssrc/Elastic.Documentation.Configuration/ConfigurationFileProvider.cssrc/Elastic.Documentation.Configuration/Toc/ApiConfiguration.cssrc/Elastic.Documentation.Configuration/Toc/ApiConfigurationConverter.cssrc/Elastic.Documentation.Configuration/Toc/DocumentationSetFile.cstests/Elastic.Documentation.Configuration.Tests/ApiConfigurationTests.cstests/Elastic.Documentation.Configuration.Tests/DocumentationSetFileTests.cstests/Elastic.Documentation.Configuration.Tests/PhysicalDocsetTests.cs
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/Elastic.Documentation.Configuration/Toc/ApiConfiguration.cs (1)
41-42:⚠️ Potential issue | 🟠 MajorEnforce
spec/specsmutual exclusivity inIsValid.Line 41 currently validates only
Spec, so configurations can still be treated as valid when bothspecandspecsare populated, which contradicts the model contract and downstream validation message.Proposed fix
public bool IsValid => - !string.IsNullOrWhiteSpace(Spec); + !string.IsNullOrWhiteSpace(Spec) && + (Specs == null || Specs.Count == 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Elastic.Documentation.Configuration/Toc/ApiConfiguration.cs` around lines 41 - 42, The IsValid getter currently only checks Spec and must be updated to enforce mutual exclusivity between Spec and Specs: change IsValid to return true only when exactly one of Spec or Specs is populated (i.e., Spec is non-empty string XOR Specs is non-null and contains at least one non-empty entry). Use string.IsNullOrWhiteSpace(Spec) for Spec and check (Specs != null && Specs.Any()) — optionally ensuring Specs elements are non-empty — and reject the case where both are populated or both empty, referencing the IsValid property and the Spec and Specs members.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/Elastic.Documentation.Configuration/Toc/ApiConfiguration.cs`:
- Around line 41-42: The IsValid getter currently only checks Spec and must be
updated to enforce mutual exclusivity between Spec and Specs: change IsValid to
return true only when exactly one of Spec or Specs is populated (i.e., Spec is
non-empty string XOR Specs is non-null and contains at least one non-empty
entry). Use string.IsNullOrWhiteSpace(Spec) for Spec and check (Specs != null &&
Specs.Any()) — optionally ensuring Specs elements are non-empty — and reject the
case where both are populated or both empty, referencing the IsValid property
and the Spec and Specs members.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d749819-9afe-4db5-ab9e-f0c0aad75418
📒 Files selected for processing (3)
src/Elastic.Documentation.Configuration/Toc/ApiConfiguration.cssrc/Elastic.Documentation.Configuration/Toc/ApiConfigurationConverter.cstests/Elastic.Documentation.Configuration.Tests/ApiConfigurationTests.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Elastic.Documentation.Configuration/Toc/ApiConfigurationConverter.cs
Summary
This PR provides the solid configuration foundation needed for subsequent PRs that implement support for custom API landing pages in the API Explorer.
The custom landing pages can optionally include sections such as https://www.elastic.co/docs/api/doc/kibana/topic/topic-kibana-spaces so that they don't have to be inserted in the OpenAPI document via overlays.
✅ New YAML Syntax Support:
Implementation Details
Files Created/Modified:
src/Elastic.Documentation.Configuration/Toc/ApiConfiguration.cs✅template,spec,specsfieldsResolvedApiConfigurationfor validated file referencessrc/Elastic.Documentation.Configuration/Toc/ApiConfigurationConverter.cs✅src/Elastic.Documentation.Configuration/Toc/DocumentationSetFile.cs✅apifield fromDictionary<string, string>toDictionary<string, ApiConfiguration>src/Elastic.Documentation.Configuration/ConfigurationFileProvider.cs✅ApiConfigurationConverterin the static deserializersrc/Elastic.Documentation.Configuration/Builder/ConfigurationFile.cs✅ApiConfigurationsand legacyOpenApiSpecificationsAddApiSubstitutions(saved for Phase 3)tests/Elastic.Documentation.Configuration.Tests/ApiConfigurationTests.cs✅✅ Full Backward Compatibility: Existing
api: stringconfigurations continue working seamlessly✅ Configuration Validation: Template paths and spec files are validated at build time
✅ Zero Behavior Changes: No impact on current documentation generation - purely infrastructure
Build Status:
The YAML converter maintains full backward compatibility. When users write:
It automatically converts to an
ApiConfigurationobject with theSpecfield populated, so the old syntax continues to work seamlessly while enabling the new template capabilities.Generative AI disclosure
Tool(s) and model(s) used: composer-2, claude-4-sonnet-thinking