feat(connect): normalize metadata partial field-list formatting#218
feat(connect): normalize metadata partial field-list formatting#218JakeSCahill wants to merge 3 commits into
Conversation
✅ Deploy Preview for docs-extensions-and-macros ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ConnectorDescription
participant extractMetadata
participant normalizeMetadataBlock
participant MetadataRenderer
ConnectorDescription->>extractMetadata: extract Metadata block
extractMetadata->>normalizeMetadataBlock: pass extracted block
normalizeMetadataBlock->>MetadataRenderer: return normalized metadata
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/redpanda-connect/normalize-metadata.js (1)
40-46: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
isFieldListFencedoesn't verify any bullet is actually a field name.The check only requires every non-blank line to look like a bullet (
BULLET.test(l)), not that at least one bullet matches the field-name pattern (FIELD_BULLET). A fenced block containing only descriptive/prose bullets (no real snake_case field name) would still be misclassified as a field list and have its fences stripped, unintentionally changing the rendered doc's formatting.🐛 Proposed fix requiring at least one real field-name bullet
function isFieldListFence (infoString, contentLines) { const info = infoString.trim(); if (info !== '' && info !== 'text') return false; const nonBlank = contentLines.filter((l) => l.trim() !== ''); - return nonBlank.length > 0 && nonBlank.every((l) => BULLET.test(l)); + return nonBlank.length > 0 && nonBlank.every((l) => BULLET.test(l)) && + nonBlank.some((l) => { + const b = l.match(BULLET); + return b && FIELD_BULLET.test(b[2]); + }); }🤖 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 `@tools/redpanda-connect/normalize-metadata.js` around lines 40 - 46, Update isFieldListFence to require at least one non-blank line matching FIELD_BULLET, while preserving the existing info-string and all-lines-BULLET checks. Fences containing only descriptive bullets must not be classified as field lists or stripped.
🤖 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 `@tools/redpanda-connect/normalize-metadata.js`:
- Around line 40-46: Update isFieldListFence to require at least one non-blank
line matching FIELD_BULLET, while preserving the existing info-string and
all-lines-BULLET checks. Fences containing only descriptive bullets must not be
classified as field lists or stripped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a71d9103-03b7-41c2-90a4-fe92e5191843
📒 Files selected for processing (3)
__tests__/tools/metadata-partials.test.jstools/redpanda-connect/helpers/renderConnectMetadata.jstools/redpanda-connect/normalize-metadata.js
|
Recommended fix: Minor suggestions:
|
Metadata partials were rendered verbatim from each connector's upstream
description, so formatting was inconsistent: some field lists are fenced
```text blocks of bare names, others are AsciiDoc bullet lists with the
field name in inline code.
Add a normalization pass (applied in renderConnectMetadata): strip fences
around a bare field list and render the fields as a bullet list with the
field name in inline code. Descriptive bullets ("All headers ...") stay
prose, already inline-coded bullets are left alone, `===` subheadings that
group metadata by operation (e.g. nats_kv) are preserved, and non-field
fenced blocks (YAML examples) are kept verbatim.
Adds a normalize-metadata test suite; updates two existing assertions to
expect inline-coded field names.
…escriptions The metadata normalizer missed two real field-bullet shapes: - fields separated from their description by " - " instead of ":" (command, retry, gcp_pubsub) — the field-bullet pattern only accepted ":" or a parenthetical annotation. - fields whose description contains inline code (mongodb_cdc `schema`, oracledb `transaction_id`/`commit_ts_ms`) — the skip guard checked the whole line for a backtick, so a backtick anywhere in the description prevented coding the field name. Broaden the pattern to accept a " - " separator and change the guard to skip only when the field name itself is already inline-coded. Add tests for both shapes.
a59c9cc to
34f9c87
Compare
2 participants
Problem
Metadata partials are rendered verbatim from each connector's upstream
== Metadatasection, so formatting is inconsistent across connectors:```text/```, e.g.http_server,csv,gcp_cloud_storage,nats_kv)oracledb_cdc)Reported on redpanda-data/rp-connect-docs#459.
Fix
Add
normalize-metadata.jsand apply it inrenderConnectMetadata. For an extracted metadata block it:- `http_server_user_agent`), keeping annotations (- `mod_time` (RFC3339)) and descriptions (- `operation`: …);===subheadings that legitimately group metadata by operation (e.g. thenats_kvprocessor);Field names are detected as lowercase snake_case leading tokens, which cleanly distinguishes them from descriptive bullets.
Tests
npx jest __tests__/tools/metadata-partials.test.js→ 23 passing, including a newnormalize-metadatasuite covering the```text/bare-fence/nats_kv/mod_time (RFC3339)/YAML-example cases. Two existing assertions updated to expect inline-coded field names.Related
===under Metadata) is fixed at the source: fix(csv): promote "Output CSV column order" out of the Metadata section benthos#462 (csv), fix(gcp_cloud_storage): promote Credentials out of the Metadata section connect#4620 (gcp_cloud_storage).