chore(docs): add markdownlint and fix all SDK page violations#306
chore(docs): add markdownlint and fix all SDK page violations#306marythought wants to merge 8 commits intomainfrom
Conversation
- Remove duplicate h1 (frontmatter title already renders as h1) - Add MDX components to MD033 allow list in markdownlint config - Move Encrypt/Decrypt Options headings into tdf.mdx to fix MD051 fragment links - Convert bold emphasis to proper headings (MD036) - Restructure heading hierarchy: add "Manage TDFs" h2, demote methods to h3 - Promote experimental section sub-headings to h3 - Run prettier to fix table column style (MD060) Closes #305 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove duplicate h1 - Convert details/summary collapsibles to proper h3 headings - Convert bold emphasis to h4 headings (Signature, Parameters, etc.) - Add language to fenced code block (MD040) - Fix fragment link for renamed heading - Run prettier for table formatting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove duplicate h1 - Convert details/summary collapsibles to proper h3 headings - Convert bold emphasis to h4 headings - Fix fragment links for renamed headings - Run prettier for table formatting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove duplicate h1 - Convert bold emphasis to h3 headings - Run prettier for table formatting and blank lines Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove duplicate h1 - Convert bold emphasis to h3 headings - Run prettier for table formatting and blank lines Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove duplicate h1 from all pages - Replace HTML card layout with markdown list in quickstart index - Convert bold emphasis to headings in authentication.mdx - Add language to fenced code blocks in troubleshooting.mdx - Fix blank line around fenced code block in list - Run prettier for table formatting and blank lines Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 4 minutes and 37 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a .markdownlint.yaml configuration and performs extensive formatting updates across the SDK documentation to ensure consistency. Key changes include standardizing quote usage in code blocks, improving table alignment, and adjusting heading levels for better structure. One piece of feedback was provided regarding invalid JavaScript syntax in an object literal example within the policy documentation.
| // JavaScript | ||
| { name: 'read' } | ||
| { | ||
| name: "read"; |
|
📄 Preview deployed to https://opentdf-docs-pr-306.surge.sh |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.markdownlint.yaml (1)
1-30: Consider setting an explicitstylefor MD060.MD060 accepts values
"aligned","compact","tight", and"any"(default). WithMD060: true, the default"any"style is applied, which accepts any of the supported styles per table — lenient enough that it effectively won't catch much. Given the PR ran Prettier (which produces aligned tables), considerstyle: alignedto lock in the style chosen by this PR:Optional tightening
-MD060: true # Table column style +MD060: + style: alignedOtherwise the config YAML parses correctly (comments on the mapping-key lines are ignored; the indented block becomes the value).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.markdownlint.yaml around lines 1 - 30, Replace the current boolean MD060: true entry with an explicit mapping that sets the desired table style; specifically update the MD060 rule (the MD060 key in the config) to use style: aligned so markdownlint enforces aligned tables instead of the permissive default "any".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/sdks/discovery.mdx`:
- Around line 807-812: The docs pass the platform root as defaultKASEndpoint,
which is wrong; update the example so defaultKASEndpoint points to the KAS
endpoint (e.g., `${platformUrl}/kas`) when constructing the OpenTDF client and
calling createTDF; locate the OpenTDF instantiation and the createTDF call
(symbols: OpenTDF, client, createTDF, defaultKASEndpoint) and change the value
to the platform KAS URL rather than platformUrl.
In `@docs/sdks/policy.mdx`:
- Around line 2412-2417: Replace the invalid object literal that uses a
semicolon after the property (the block showing { name: "read"; }) with valid
JavaScript syntax; either change the semicolon to nothing or a comma if adding
more properties, or assign the object to a constant like const action = { name:
"read" }; so the property name and value ("name": "read") are valid and the
example compiles.
In `@docs/sdks/quickstart/javascript.mdx`:
- Around line 264-277: After creating the missing marketing value with
platform.v1.attributes.createAttributeValue, the code still uses the original
stale attribute object (variable attribute) to derive attributeValueId which can
be undefined; fix this by reloading the attribute (e.g., call
platform.v1.attributes.getAttribute or re-run listAttributes and find the
attribute by name/namespace) after await
platform.v1.attributes.createAttributeValue resolves, then read attribute.values
to derive attributeValueId before calling the subject mapping code (the same
refresh is needed for the logic around attributeValueId at the later reference).
---
Nitpick comments:
In @.markdownlint.yaml:
- Around line 1-30: Replace the current boolean MD060: true entry with an
explicit mapping that sets the desired table style; specifically update the
MD060 rule (the MD060 key in the config) to use style: aligned so markdownlint
enforces aligned tables instead of the permissive default "any".
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33aaddf6-37bd-4e63-9870-059aae35c042
📒 Files selected for processing (16)
.markdownlint.yamlcode_samples/tdf/decrypt_options.mdxcode_samples/tdf/encrypt_options.mdxdocs/sdks/authentication.mdxdocs/sdks/authorization.mdxdocs/sdks/discovery.mdxdocs/sdks/index.mdxdocs/sdks/obligations.mdxdocs/sdks/platform-client.mdxdocs/sdks/policy.mdxdocs/sdks/quickstart/go.mdxdocs/sdks/quickstart/index.mdxdocs/sdks/quickstart/java.mdxdocs/sdks/quickstart/javascript.mdxdocs/sdks/tdf.mdxdocs/sdks/troubleshooting.mdx
💤 Files with no reviewable changes (2)
- code_samples/tdf/decrypt_options.mdx
- code_samples/tdf/encrypt_options.mdx
| const client = new OpenTDF({ ...auth, platformUrl }); | ||
| const ciphertext = await client.createTDF({ | ||
| source: { type: 'stream', location: dataStream }, | ||
| attributes: required, | ||
| defaultKASEndpoint: platformUrl, | ||
| source: { type: "stream", location: dataStream }, | ||
| attributes: required, | ||
| defaultKASEndpoint: platformUrl, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect defaultKASEndpoint examples and confirm platform-root usage is not copied elsewhere.
rg -n -C2 'defaultKASEndpoint:' --glob '*.mdx'Repository: opentdf/docs
Length of output: 6483
Use the KAS endpoint, not the platform root.
Line 811 passes platformUrl as defaultKASEndpoint, but quickstart examples and other docs use ${platformUrl}/kas. This workflow will contact the wrong endpoint.
Proposed fix
const ciphertext = await client.createTDF({
source: { type: "stream", location: dataStream },
attributes: required,
- defaultKASEndpoint: platformUrl,
+ defaultKASEndpoint: `${platformUrl}/kas`,
});📝 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.
| const client = new OpenTDF({ ...auth, platformUrl }); | |
| const ciphertext = await client.createTDF({ | |
| source: { type: 'stream', location: dataStream }, | |
| attributes: required, | |
| defaultKASEndpoint: platformUrl, | |
| source: { type: "stream", location: dataStream }, | |
| attributes: required, | |
| defaultKASEndpoint: platformUrl, | |
| }); | |
| const client = new OpenTDF({ ...auth, platformUrl }); | |
| const ciphertext = await client.createTDF({ | |
| source: { type: "stream", location: dataStream }, | |
| attributes: required, | |
| defaultKASEndpoint: `${platformUrl}/kas`, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/sdks/discovery.mdx` around lines 807 - 812, The docs pass the platform
root as defaultKASEndpoint, which is wrong; update the example so
defaultKASEndpoint points to the KAS endpoint (e.g., `${platformUrl}/kas`) when
constructing the OpenTDF client and calling createTDF; locate the OpenTDF
instantiation and the createTDF call (symbols: OpenTDF, client, createTDF,
defaultKASEndpoint) and change the value to the platform KAS URL rather than
platformUrl.
| ```typescript | ||
| // JavaScript | ||
| { name: 'read' } | ||
| { | ||
| name: "read"; | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find JavaScript/TypeScript docs snippets that show `name: "read";` as an object member.
rg -n -C2 'name:\s*"read";' --glob '*.mdx'Repository: opentdf/docs
Length of output: 225
Fix the JavaScript object syntax at line 2415.
The semicolon after name: "read" is invalid in a JavaScript object literal; object properties should be comma-separated. Change to valid syntax.
Proposed fix
```typescript
// JavaScript
-{
- name: "read";
-}
+const action = { name: "read" };
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/sdks/policy.mdx` around lines 2412 - 2417, Replace the invalid object
literal that uses a semicolon after the property (the block showing { name:
"read"; }) with valid JavaScript syntax; either change the semicolon to nothing
or a comma if adding more properties, or assign the object to a constant like
const action = { name: "read" }; so the property name and value ("name": "read")
are valid and the example compiles.
| // Ensure the marketing value exists on the attribute | ||
| const marketingFqn = `${deptFqn}/value/marketing`; | ||
| const listResp = await platform.v1.attributes.listAttributes({}); | ||
| const attribute = listResp.attributes.find( | ||
| (attr) => attr.name === 'department' && attr.namespace?.id === namespaceId | ||
| (attr) => attr.name === "department" && attr.namespace?.id === namespaceId, | ||
| ); | ||
|
|
||
| if (!(await attributeValueExists(platformUrl, auth, marketingFqn))) { | ||
| await platform.v1.attributes.createAttributeValue({ | ||
| attributeId: attribute?.id, | ||
| value: 'marketing', | ||
| value: "marketing", | ||
| }); | ||
| console.log('✅ Added marketing value to department attribute'); | ||
| console.log("✅ Added marketing value to department attribute"); | ||
| } |
There was a problem hiding this comment.
Refresh the attribute before deriving attributeValueId.
When Line 271 creates the missing marketing value, Line 321 still reads from the pre-create attribute.values, so attributeValueId can be undefined and the subject mapping call will fail.
Proposed fix
// Get the attribute value ID for "marketing"
-const marketingValue = attribute?.values?.find((v) => v.value === "marketing");
-const attributeValueId = marketingValue?.id;
+const refreshedAttributes = await platform.v1.attributes.listAttributes({});
+const refreshedAttribute = refreshedAttributes.attributes.find(
+ (attr) => attr.name === "department" && attr.namespace?.id === namespaceId,
+);
+const marketingValue = refreshedAttribute?.values?.find(
+ (v) => v.value === "marketing",
+);
+if (!marketingValue?.id) {
+ throw new Error("Marketing value not found in department attribute");
+}
+const attributeValueId = marketingValue.id;Also applies to: 320-322
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/sdks/quickstart/javascript.mdx` around lines 264 - 277, After creating
the missing marketing value with platform.v1.attributes.createAttributeValue,
the code still uses the original stale attribute object (variable attribute) to
derive attributeValueId which can be undefined; fix this by reloading the
attribute (e.g., call platform.v1.attributes.getAttribute or re-run
listAttributes and find the attribute by name/namespace) after await
platform.v1.attributes.createAttributeValue resolves, then read attribute.values
to derive attributeValueId before calling the subject mapping code (the same
refresh is needed for the logic around attributeValueId at the later reference).
Runs on PRs that touch .md/.mdx files. Only lints changed files, not the entire repo. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove trailing blank line in decrypt_options.mdx (MD012) - Use absolute link for cross-file fragment in encrypt_options.mdx (MD051) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
.markdownlint.yamlconfig based on data-security-platform, tuned for Docusaurus MDX.md/.mdxfiles on PRsChanges by category
# Headingfrom all pages (frontmattertitle:renders as h1)**Signature**,**Parameters**,**Returns**,**Example**to proper headings throughout<details>/<summary>method wrappers in policy.mdx and obligations.mdx to proper###headings## Manage TDFsparent, promoted experimental sub-headingstextlanguage to error output blocks in troubleshooting.mdxCloses #305
Post-merge admin step
After merging, add "Markdownlint" as a required status check in Settings → Branches → Branch protection rules → main → Require status checks to pass before merging. The workflow only runs on PRs that touch markdown files, so it won't block unrelated PRs.
Test plan
npx markdownlint-cli2 "docs/sdks/**/*.mdx"passes with 0 errorsnpx docusaurus buildsucceeds🤖 Generated with Claude Code