Skip to content

Fix module section heading collisions #73

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.

Sign up for GitHub

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

Closed
flybayer wants to merge 1 commit into from
Closed

Fix module section heading collisions #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions compute/ec2_service/rvn-ec2-service-definition.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ definition:
name: EC2 Service
description: Runs supervised workloads on a stable EC2 Auto Scaling Group, with optional shared ALB routing and switchable container or manual in-place deploys.
release:
version: 0.1.1
description: Reorganize EC2 service inputs and clarify web and storage configuration.
version: 0.1.2
description: Clarify the EC2 service configuration section label.
module:
inputs:
- id: network
Expand All @@ -24,7 +24,7 @@ module:
required: true
- $include: ../../partials/inputs/private-subnet-placement.yml
- id: section_service
label: EC2 service
label: EC2 service config
type: section
- id: name
immutable: true
Expand Down
6 changes: 3 additions & 3 deletions compute/ecs_cluster/rvn-ecs-cluster-definition.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ definition:
name: ECS Cluster
description: Production-ready AWS ECS cluster with Fargate, Fargate Spot, optional EC2 capacity, and shared load balancers.
release:
version: 0.3.1
description: Add additional ALB certificate ARN fields for SNI.
version: 0.3.2
description: Clarify the ECS cluster configuration section label.
module:
inputs:
- id: network
Expand All @@ -22,7 +22,7 @@ module:
required: true
type: $ref:rvn-aws-network
- id: section_cluster
label: ECS cluster
label: ECS cluster config
type: section
- default: <<project.given_id>>-<<environment.given_id>>
description: Name prefix for all resources. Terraform requires 1-28 characters so generated ALB names fit AWS limits.
Expand Down
6 changes: 3 additions & 3 deletions compute/lambda/rvn-lambda-definition.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ definition:
name: Lambda Function
description: AWS Lambda function with zip or container image deployments, alias-based releases, IAM, logs, and optional function URLs.
release:
version: 0.3.2
description: Improve build source and builder instance guidance.
version: 0.3.3
description: Clarify the Lambda function configuration section label.
module:
inputs:
- $include: ../../partials/inputs/aws-account.yml
- $include: ../../partials/inputs/aws-region.yml
- id: section_function
label: Lambda function
label: Lambda function config
type: section
- id: lambda_type
immutable: true
Expand Down
6 changes: 3 additions & 3 deletions hosting/static_site/rvn-aws-static-definition.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ definition:
name: Static Hosting
description: Static file hosting for S3-backed sites and assets delivered through CloudFront.
release:
version: 0.3.4
description: Improve build source, Railpack command, and builder instance guidance.
version: 0.3.5
description: Clarify the static hosting configuration section label.
module:
build:
builder: >-
Expand Down Expand Up @@ -54,7 +54,7 @@ module:
default: us-east-1
description: Region for the S3 bucket. CloudFront and KVS always use us-east-1.
- id: section_hosting
label: Static hosting
label: Static hosting config
type: section
- default: <<project.given_id>>-<<environment.given_id>>-<<module.given_id>>
description: Name for the bucket and other resources' prefix. Must be globally unique.
Expand Down
3 changes: 2 additions & 1 deletion tools/ravion-modules/src/compiler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { readdir, readFile } from "node:fs/promises";
import { dirname, join, resolve } from "node:path";
import YAML from "yaml";
import { parseAuthoringDefinitionFile, type AuthoringDefinition } from "./authoring-schema.js";
import { validateUniqueDefinitionTypes } from "./guardrails.js";
import { validateSectionLabelsDoNotCollideWithDefinitionNames, validateUniqueDefinitionTypes } from "./guardrails.js";

export interface CompiledDefinition {
filePath: string;
Expand Down Expand Up @@ -78,6 +78,7 @@ export async function compileAllDefinitions(rootPath = process.cwd()): Promise<C
const compiled = await Promise.all(definitionFiles.map((filePath) => compileDefinitionFile(filePath)));
const sorted = compiled.sort((left, right) => left.filePath.localeCompare(right.filePath));
validateUniqueDefinitionTypes(sorted);
validateSectionLabelsDoNotCollideWithDefinitionNames(sorted);
return sorted;
}

Expand Down
36 changes: 36 additions & 0 deletions tools/ravion-modules/src/guardrails.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,34 @@ export function validateUniqueDefinitionTypes(definitions: CompiledDefinition[])
}
}

export function validateSectionLabelsDoNotCollideWithDefinitionNames(definitions: CompiledDefinition[]): void {
const collisions: string[] = [];

for (const definition of definitions) {
const inputs = definition.module.inputs;
if (!Array.isArray(inputs)) {
continue;
}

const definitionSlug = slugifyHeading(definition.name);
for (const input of inputs) {
if (!isRecord(input) || input.type !== "section" || typeof input.label !== "string") {
continue;
}

if (slugifyHeading(input.label) === definitionSlug) {
collisions.push(`${definition.type}: section "${input.label}" in ${definition.filePath}`);
}
}
}

if (collisions.length > 0) {
throw new GuardrailError(
`Section labels must not generate the same anchor as their module definition name:\n${collisions.map((collision) => `- ${collision}`).join("\n")}`,
);
}
}

async function findLegacyModuleDefinitionFiles(rootPath: string): Promise<string[]> {
const yamlFiles: string[] = [];
await collectYamlFiles(rootPath, rootPath, yamlFiles);
Expand Down Expand Up @@ -144,6 +172,14 @@ function normalizeRelativePath(rootPath: string, filePath: string): string {
return relative(rootPath, filePath).split(sep).join("/");
}

function slugifyHeading(value: string): string {
return value
.trim()
.toLowerCase()
.replace(/[^a-z0-9_\s-]/g, "")
.replace(/\s+/g, "-");
}
Comment on lines +175 to +181

@greptile-apps greptile-apps Bot Jul 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 partner slugifyHeading diverges from Mintlify's actual anchor algorithm

According to Mintlify's documentation, their algorithm preserves & and /, converts . to -, and turns straight apostrophes into right single quotation marks. The current regex .replace(/[^a-z0-9_\s-]/g, "") strips all of these, so a section labeled "Read/write config" would produce readwrite-config here but read/write-config in Mintlify — a real collision against a definition named "Read/Write Config" would pass the guardrail undetected. All current labels are simple English phrases so there is no present defect, but the check could silently miss future collisions that involve &, /, ., or '.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: tools/ravion-modules/src/guardrails.ts
Line: 175-181

Comment:
**`slugifyHeading` diverges from Mintlify's actual anchor algorithm**

According to Mintlify's documentation, their algorithm preserves `&` and `/`, converts `.` to `-`, and turns straight apostrophes into right single quotation marks. The current regex `.replace(/[^a-z0-9_\s-]/g, "")` strips all of these, so a section labeled "Read/write config" would produce `readwrite-config` here but `read/write-config` in Mintlify — a real collision against a definition named "Read/Write Config" would pass the guardrail undetected. All current labels are simple English phrases so there is no present defect, but the check could silently miss future collisions that involve `&`, `/`, `.`, or `'`.

How can I resolve this? If you propose a fix, please make it concise.


function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === "object" && value !== null && !Array.isArray(value);
}
Expand Down
32 changes: 32 additions & 0 deletions tools/ravion-modules/test/guardrails.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,38 @@ describe("migration guardrails", () => {
(error) => error instanceof GuardrailError && error.message.includes("Duplicate definition.type") && error.message.includes("ravion-aws-network"),
);
});

it("fails when a section label generates the same anchor as the definition name", async () => {
const rootPath = await mkdtemp(join(tmpdir(), "ravion-section-label-"));
const modulePath = join(rootPath, "compute", "ec2_service");
await mkdir(modulePath, { recursive: true });
await writeFile(
join(modulePath, "rvn-ec2-service-definition.yml"),
[
"definition:",
" type: rvn-ec2-service",
" name: EC2 Service",
" description: Test module",
"release:",
" version: 1.0.0",
" description: Initial release.",
"module:",
" inputs:",
" - id: section_service",
" label: EC2 service",
" type: section",
"",
].join("\n"),
);

await assert.rejects(
compileAllDefinitions(rootPath),
(error) =>
error instanceof GuardrailError &&
error.message.includes("Section labels must not generate the same anchor") &&
error.message.includes("rvn-ec2-service"),
);
});
});

async function writeDefinition(rootPath: string, category: string, moduleName: string, type: string): Promise<void> {
Expand Down
Loading