fix(property-extractor): resolve member calls on inline constants in defaults - #228
Merged
Conversation
✅ Deploy Preview for docs-extensions-and-macros ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
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:
📝 WalkthroughWalkthroughThe property extractor’s function-call pattern now accepts dotted qualified names. Cache construction extracts supported member accessor values from inline Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PropertyExtractor
participant CppHeader
participant ConstexprCache
PropertyExtractor->>ConstexprCache: build cache from C++ declarations
ConstexprCache->>CppHeader: scan inline constants
CppHeader-->>ConstexprCache: provide topic accessor value
ConstexprCache-->>PropertyExtractor: store qualified and unqualified lookup keys
PropertyExtractor->>ConstexprCache: resolve member-call expression
ConstexprCache-->>PropertyExtractor: return literal string value
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
__tests__/tools/property-extractor/test_constant_resolution.py (1)
42-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the production resolution path.
These assertions bypass
resolve_cpp_function_call()and the renderer by callingConstexprCache.lookup_function()directly. Add an assertion through the production entry point so the tests also prove thatmodel::schema_registry_internal_tp.topic()renders as"_schemas".🤖 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 `@__tests__/tools/property-extractor/test_constant_resolution.py` around lines 42 - 52, Update the constant-resolution tests to exercise the production entry point resolve_cpp_function_call() and its renderer, rather than only calling ConstexprCache.lookup_function(). Add coverage for the qualified model::schema_registry_internal_tp.topic() call and assert that the rendered result is "_schemas", while preserving the existing qualified and unqualified cache assertions if still useful.
🤖 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.
Inline comments:
In `@tools/property-extractor/property_extractor.py`:
- Line 82: Update FUNCTION_CALL_PATTERN or its resolver caller to require the
entire expression to be a function call, preventing match() from accepting
trailing operators or suffix expressions. Preserve valid call parsing and add a
regression test covering a function call followed by an operator or appended
expression.
---
Nitpick comments:
In `@__tests__/tools/property-extractor/test_constant_resolution.py`:
- Around line 42-52: Update the constant-resolution tests to exercise the
production entry point resolve_cpp_function_call() and its renderer, rather than
only calling ConstexprCache.lookup_function(). Add coverage for the qualified
model::schema_registry_internal_tp.topic() call and assert that the rendered
result is "_schemas", while preserving the existing qualified and unqualified
cache assertions if still useful.
🪄 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 Plus
Run ID: 3bae8fe4-59aa-4b4b-8db0-9bd3e3e9f4a5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
__tests__/tools/property-extractor/test_constant_resolution.pypackage.jsontools/property-extractor/property_extractor.py
Contributor
|
No findings on the change itself. One merge mechanic for the stated order: this branch bumps |
Contributor
Author
|
Acknowledged on the version mechanic — this stays parked until #225 merges and publishes 5.3.0, then gets rebased with a bump to 5.3.1 (the package.json/lock conflict resolves in that rebase). |
JakeSCahill
added a commit
that referenced
this pull request
Jul 31, 2026
Both open questions were answered by rendering real connector data (connect 4.103.0, 244 descriptions) with Asciidoctor: Large-block mitigation: collapsible dropped in favor of passthrough plus reporting. At the 1200-char threshold the collapsible fired on five real connectors, and the worst case (redpanda_migrator, 6637 chars) has a hand-curated published page the migration would never auto-swap anyway. Hiding primary content also depends on browser-specific find and fragment auto-expand behavior in closed details elements, and bodies carrying their own ==== delimiters could never be wrapped (the http_server guard), so behavior diverged silently per page. Heading demotion was rejected with direct evidence: demoting snowflake_put's embedded headings produces six 'section title out of sequence' warnings because the description renders before the page's first == section. Passthrough matches the published pages' structure exactly. The generator now reports long heading-less descriptions during generation (10 current candidates) so structure gets added upstream instead of being hidden by the docs build. Summary placement: the partial now carries two tagged regions in one file. tag=attrs holds a :description: attribute line (flattened to one line via the new summaryAttribute helper: three summaries contain hard breaks) for pages to include in their header, so search snippets and meta descriptions refresh too. tag=body holds the summary, version note, and rendered description for the body include. Verified with Asciidoctor that a page consuming both tags gets the fresh attribute and body with zero warnings. Version 5.4.0: a feature bump that also sidesteps the 5.3.x patch train (#225 takes 5.3.0, #228 re-bumps to 5.3.1).
Contributor
|
Re-checked: this still bumps No findings on the change itself, and |
…defaults The v26.1.14 property log_eviction_exempt_topics leaked raw C++ into its rendered default: [model::schema_registry_internal_tp.topic()] instead of ["_schemas"] (redpanda-data/docs#1824). Two gaps caused it: FUNCTION_CALL_PATTERN did not admit dots, so member calls never reached the resolver, and the constexpr cache had no pattern for inline topic_partition constants. Widen the pattern and cache <variable>.topic -> literal for inline const model::topic_partition declarations, qualified and unqualified. Unresolvable expressions still pass through unchanged, so behavior for every other default is unaffected.
JakeSCahill
force-pushed
the
fix/resolve-member-call-defaults
branch
from
July 31, 2026 15:17
dd35dc5 to
47d9d4e
Compare
Contributor
Author
|
@micheleRP re-bump done now that #225 published 5.3.0: rebased onto main, version is 5.3.1, and the package.json/lockfile conflicts are resolved (lockfile regenerated from main's, version fields consistent). Full JS suite 862/862 green on the rebased branch, and the extractor unit tests including this PR's |
This was referenced Jul 31, 2026
micheleRP
approved these changes
Jul 31, 2026
micheleRP
left a comment
micheleRP
left a comment
Contributor
There was a problem hiding this comment.
Approving. The version hold is resolved, and I verified the new anchoring commit rather than taking it on faith.
FUNCTION_CALL_PATTERN going from ([a-zA-Z0-9_:]+)\(\) to ([a-zA-Z0-9_:.]+)\(\)$ does more than enable the member-accessor case. Running both patterns side by side:
| Input | Old | New |
|---|---|---|
model::schema_registry_internal_tp.topic() |
no match | model::…topic |
foo() |
foo |
foo |
foo() + bar |
foo |
no match |
a + b.foo() |
no match | no match |
So the anchor also fixes a latent correctness bug: foo() + bar previously resolved to foo's value and silently dropped + bar. Since the call site uses .match(), the $ makes this an effective fullmatch, so compound expressions cannot slip through.
I checked the one edge the anchor introduces, 'foo() ' with trailing whitespace no longer matching, and it is harmless because arg_str.strip() runs first at the top of the same function.
CI is 18 green including test-property-extractor on 3.9 and 3.11. pytest is not installed on my machine, so I exercised the regexes directly instead of running the suite locally.
One sequencing note: #239 carries no version bump and is meant to ride this publish, so merge #239 first and let 5.3.1 here carry both.
JakeSCahill
added a commit
that referenced
this pull request
Aug 3, 2026
Both open questions were answered by rendering real connector data (connect 4.103.0, 244 descriptions) with Asciidoctor: Large-block mitigation: collapsible dropped in favor of passthrough plus reporting. At the 1200-char threshold the collapsible fired on five real connectors, and the worst case (redpanda_migrator, 6637 chars) has a hand-curated published page the migration would never auto-swap anyway. Hiding primary content also depends on browser-specific find and fragment auto-expand behavior in closed details elements, and bodies carrying their own ==== delimiters could never be wrapped (the http_server guard), so behavior diverged silently per page. Heading demotion was rejected with direct evidence: demoting snowflake_put's embedded headings produces six 'section title out of sequence' warnings because the description renders before the page's first == section. Passthrough matches the published pages' structure exactly. The generator now reports long heading-less descriptions during generation (10 current candidates) so structure gets added upstream instead of being hidden by the docs build. Summary placement: the partial now carries two tagged regions in one file. tag=attrs holds a :description: attribute line (flattened to one line via the new summaryAttribute helper: three summaries contain hard breaks) for pages to include in their header, so search snippets and meta descriptions refresh too. tag=body holds the summary, version note, and rendered description for the body include. Verified with Asciidoctor that a page consuming both tags gets the fresh attribute and body with zero warnings. Version 5.4.0: a feature bump that also sidesteps the 5.3.x patch train (#225 takes 5.3.0, #228 re-bumps to 5.3.1).
JakeSCahill
added a commit
that referenced
this pull request
Aug 22, 2026
#227) * feat(rpcn): regenerate a per-connector description partial Adds a `descriptions/<type>/<name>.adoc` partial that is regenerated on every run, mirroring the fields/examples/metadata partials. This is the foundation for fixing description staleness: today a connector's main page is drafted once and never refreshed, so summary/description edits upstream never reach already- published pages. A page that includes this partial always renders the current summary + version + description. - metadata-utils: add descriptionIncludeLine() (mirrors metadataIncludeLine). - helpers/renderConnectDescription: render the description with the == Metadata block replaced by the metadata partial include (de-dup), plus large-block mitigation — a long, heading-less prose wall keeps its first paragraph visible and moves the remainder into an AsciiDoc [%collapsible] block. Descriptions that already carry their own headings pass through unchanged. - templates/descriptions-partials.hbs: summary + version + rendered description behind a "do not edit" banner. - generator: emit the partial for every non-bloblang connector. - tests: 9 new cases (util, helper mitigation branches, generator output). intro.hbs is intentionally left unchanged, so newly drafted pages are byte-for- byte identical and existing generator tests stay green. Wiring pages to include the partial is a deliberate, separate migration (like metadata partials), not an automatic rewrite — see the PR description. Bumps version to 5.2.6. * fix(rpcn): guard collapsible against ==== collision, blank stale description partials Review fixes for the description-partials prototype: - A body containing its own ==== block delimiters (admonition/example blocks) is no longer wrapped in the [%collapsible] block: the nested opening ==== terminated the collapsible early, leaking the rest of the body as top-level blocks and stripping admonition styling (seen with the http_server output's nested CAUTION). Such bodies now pass through unchanged. - When a connector's description disappears upstream, the previously generated description partial is now blanked to a banner-only file and logged, mirroring the stale-metadata handling, instead of silently leaving first-draft text on disk forever. - Description partials are only emitted for connector type dirs (mirrors the handler's connectorTypes list), so the config data key no longer produces 17 unincludable files under descriptions/configs/. - Remove the unused HEADING constant. Adds regression tests for all three, including Asciidoctor render-level assertions that the CAUTION admonition survives intact and that plain collapsible output keeps all content inside the block. * feat(rpcn): resolve description-partial design questions from testing Both open questions were answered by rendering real connector data (connect 4.103.0, 244 descriptions) with Asciidoctor: Large-block mitigation: collapsible dropped in favor of passthrough plus reporting. At the 1200-char threshold the collapsible fired on five real connectors, and the worst case (redpanda_migrator, 6637 chars) has a hand-curated published page the migration would never auto-swap anyway. Hiding primary content also depends on browser-specific find and fragment auto-expand behavior in closed details elements, and bodies carrying their own ==== delimiters could never be wrapped (the http_server guard), so behavior diverged silently per page. Heading demotion was rejected with direct evidence: demoting snowflake_put's embedded headings produces six 'section title out of sequence' warnings because the description renders before the page's first == section. Passthrough matches the published pages' structure exactly. The generator now reports long heading-less descriptions during generation (10 current candidates) so structure gets added upstream instead of being hidden by the docs build. Summary placement: the partial now carries two tagged regions in one file. tag=attrs holds a :description: attribute line (flattened to one line via the new summaryAttribute helper: three summaries contain hard breaks) for pages to include in their header, so search snippets and meta descriptions refresh too. tag=body holds the summary, version note, and rendered description for the body include. Verified with Asciidoctor that a page consuming both tags gets the fresh attribute and body with zero warnings. Version 5.4.0: a feature bump that also sidesteps the 5.3.x patch train (#225 takes 5.3.0, #228 re-bumps to 5.3.1). * fix(rpcn): emit partials for summary-only connectors and never escape the description attribute Two CodeRabbit findings, both real at scale against connect 4.103.0: - The partial template gated everything on description, so the 40 connectors that define a summary without a description rendered an empty partial, which the generator then treated as an upstream removal and blanked. The gate is now (or summary description). - The :description: attribute value used double-stash output, so the 9 summaries containing characters like ampersands or quotes were HTML-escaped inside an AsciiDoc attribute. Triple-stash now matches the body rendering. * fix(rpcn): escape placeholder braces, separate glued headings, report out-of-sequence starts A full-corpus Asciidoctor convert of all 311 generated description partials (connect 4.103.0) surfaced three defect classes: - Literal placeholders like {endpoint} and {api_version} in descriptions (otlp_http, salesforce_graphql) are consumed as attribute references, even inside single-backtick spans, and warn on the published pages today. They are now escaped outside listing blocks. - The protobuf processor's '== Operators' heading is glued to the paragraph above it, so it rendered as literal text and its subsections warned out of sequence. Headings now get the blank line Asciidoctor requires (listing blocks untouched). - aws_dynamodb_cdc (markdown ###) and iceberg (===) start their heading structure below level one, which renders out of sequence and needs an upstream fix: the generator now reports both at build time, and hasStructuralHeadings recognizes markdown headings so the long-headingless report cannot misfire on markdown-structured descriptions. * Report markdown-heading descriptions and surface reports in the PR summary Review findings: - Markdown ## headings counted as structure, so the descriptions that render worst were exactly the ones the long-description report skipped. Structural now means AsciiDoc == only; markdown-heading descriptions get their own report naming the upstream conversion. - The reports went to console.warn, which the workflow buries in a collapsed log block. They now ride the diff object and render as a collapsed section of the PR summary body. * Release 5.4.0 Description partials are a new generator feature. * fix(rpcn): track markdown fences in description scanners so fenced examples pass through untouched The five body scanners in renderConnectDescription.js only tracked AsciiDoc ---- listing delimiters, so the interior of a ```/~~~ fenced example was treated as prose: {token} placeholders gained a literal backslash escape (reproduced: 'url: {endpoint}/v1/logs' rendered as 'url: \{endpoint}/v1/logs') and glued #-comment lines had blank lines pushed into the example. Share metadata-utils' FENCE_DELIMITER and route every scanner through one layered block/fence annotator so fence interiors pass through byte-identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(rpcn): blank description partials orphaned by connectors deleted upstream The in-loop stale handling only covers connectors still present in the dataset: a connector deleted upstream leaves no entry at all, so its description partial kept serving deleted-connector text forever, silently. After the generation loop, sweep the descriptions output root for .adoc files no connector claimed this run, blank them with the same banner-only content as other removed-content partials, and surface them in descriptionReports (and the PR summary). Draft mode skips the sweep: it runs on a filtered dataset where unvisited means already documented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(rpcn): brace-escape summaries so {placeholder} survives the description partial The template emits the summary raw in both the partial body and the :description: attribute, so a {placeholder} in a summary was silently consumed by Asciidoctor as a missing attribute reference. Escape summaries through the same escapePlaceholderBraces path the description body already uses, in the generator that prepares the template data. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(rpcn): add --template-description CLI override for the description partial template Every other partial template has a CLI override flag, but the generator's templateDescription option was unreachable from the CLI: no --template-description flag existed and the handler never passed it. Mirror --template-metadata and pass the option through both generator call sites (partials and draft-missing). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(rpcn): reunite generateMultiVersionPRSummary with its JSDoc renderDescriptionReports was inserted between the multi-version summary JSDoc and its function, orphaning the @PARAM documentation. Move the new function (with its own JSDoc) above the existing comment block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: Auto-update CLI reference documentation (PR #227) * fix(rpcn): refuse the description orphan sweep when it would blank the tree The post-generation sweep blanks every description partial the current dataset did not claim, and an incomplete dataset is indistinguishable from a mass upstream deletion. Measured: a full connect-4.103.1 run writes 316 partials, and a following run whose data carried only the inputs key blanked 233 of them with no dry run, no threshold and no confirmation. Once pages include these partials that is hundreds of published pages losing their body in one pass. Collect the sweep candidates before writing anything and refuse when more than 10 files AND more than 10% of the tree would be blanked, reporting the refusal into the PR summary instead. Small, plausible deletions still sweep unchanged. --prune-orphaned-descriptions is the explicit opt-in for a dataset a human has confirmed complete. * fix(rpcn): write rate-limit partials where their include line points The generator derived the partial directory from the raw dataset key (rate-limits) while descriptionIncludeLine and metadataIncludeLine derive it from item.type (rate_limit -> rate_limits), so every rate-limit page wired to a description partial got an unresolved include. A real Antora build reports "target of include not found" and the page ships with the content silently missing. Make typeDirFor the one place a type directory is derived: it now collapses every rate-limit spelling to rate_limits, and the generator writes the metadata and description partials under typeDirFor(item) rather than the data key. Fields and examples keep the raw data-key spelling because published pages already include them by that path. * fix(rpcn): keep tag regions in a blanked description partial Pages include this partial with [tag=attrs] and [tag=body]. The banner written when a description disappears upstream carried neither region, so Asciidoctor logged "tag 'attrs' not found in include file" and "tag 'body' not found" for every consuming page on every build, which is a build failure wherever warnings are errors. That also contradicted the reason for blanking rather than deleting: keep published includes resolving. Emit empty attrs and body regions in the banner so both tags resolve to nothing. * fix(rpcn): flatten the description attrs region through one flattener summaryAttribute collapsed whitespace only, so the attrs region a page adopts for its :description: shipped raw backticks, xref: and link: macros and bare URL macros straight into <meta name="description"> and, through single-source stubs, into cloud-docs. A real Antora build on a page wired to the attrs include published `xref:guides:bloblang/about.adoc[Bloblang mapping]` verbatim. backfillPageDescriptions already had a flattener that handles all of it. Move that one into metadata-utils as flattenToAttributeValue and call it from both, so the meta description a page publishes does not change when it migrates from the backfilled block to the partial. * feat(rpcn): wire published pages to the description partial The branch wrote 316 description partials per run and nothing read a byte of them: no page carried the include, no template emitted it, and no migration existed, so the whole feature shipped to no consumer. Verified against the real rp-connect-docs, where partial$descriptions had 0 references against 51 for partial$metadata. Three pieces close the loop: - connector.hbs emits the two tagged includes instead of freezing the summary, the "Introduced in version" line and the prose into the page, so new drafts are born wired. The attrs include sits inside the page's // tag::meta[] region, so single-source stubs keep inheriting :description: as a header attribute. Component families with no description partial, and connectors with no summary, keep the inline attribute rather than getting an include that resolves to nothing. - generate migrate-rpcn-descriptions rewires the pages already published. Dry run by default. The body rewire is guarded twice: the page's intro region must be reproduced line for line by the partial, and no content outside it may duplicate the partial body. On rp-connect-docs origin/main that migrates 290 page headers and 30 bodies, and refuses 175 pages whose intro carries prose the generator has never seen plus 103 whose description continues past the config listing, rather than deleting published content. - #246's page-header backfill now recognizes the attrs include, so it stops splicing a competing :description: block into every migrated page. A real Antora build of migrated rp-connect-docs against a build of the same tree unmigrated: 446 pages both times, identical warning and error counts, zero unresolved description includes, zero missing tags, zero changes to rendered body text on 338 component pages, and 3 meta descriptions that changed only by losing raw link markup. * fix(rpcn): warn before a regenerated description partial drops a section The metadata partial has compared old against new headings and pushed lostSectionWarnings since #236, because a section vanishing from a published partial is content loss rather than cleanup. The description partial was overwritten unconditionally: a hand-added section disappeared with no warning, no report and no log line, proven by injecting the same section into both partials and regenerating. Now that pages include these partials the risk is live, so route the description write through the same lostMetadataSections check. * fix(rpcn): collect the draft run's description reports into the PR summary The draft generator call collected draftResult.lostSectionWarnings and silently dropped draftResult.descriptionReports, so the structure reports for newly drafted connectors, the ones most likely to need an upstream fix, never reached the auto-docs PR summary. Deleting either push left the suite fully green because the handler has no coverage at all: it cannot even be required under jest, since it pulls in ESM-only octokit. Both call sites now merge through one collectGeneratorReports in pr-summary-formatter, which is requireable and tested, so a call site can no longer forget a report key. * refactor(rpcn): one verbatim-line annotator for every prose scanner There were three copies of the block/fence state machine and they had already drifted: renderConnectDescription tested the trimmed line while metadata-utils and normalize-metadata tested the raw one, so an indented ---- opened a verbatim region for one scanner and was plain content for another, on the same description in the same pipeline. locateMetadata, which runs before all of them, tracked ---- only and was blind to markdown fences, so a `== Metadata` inside a fenced example was extracted into the metadata partial and the fence was destroyed. Export one layered annotator from metadata-utils and consume it from locateMetadata, sectionHeadings and the five scanners in renderConnectDescription. It tests the raw line, because Asciidoctor only reads a block delimiter at column 0. normalize-metadata imports the shared delimiter rather than declaring a third copy. * refactor(rpcn): one connector-type list, one gate for per-page partials The list of component families existed in eight places and the copies disagreed three ways: config in two of them, rate_limits missing from one, both rate-limit spellings in two. A family added upstream silently gets no partial, no page backfill or no cloud-docs check depending on which copy forgot it. Export CONNECTOR_TYPE_DIRS (page-backed families) and CONNECTOR_DATA_KEYS (those plus config) from metadata-utils and use them in seven of the eight places. The eighth deliberately omits rate_limits and changing it would add cloud-docs findings in a path with no test coverage, so it keeps its literal with a comment saying why. The examples and metadata partials also gated on a denylist naming only the two bloblang keys while the description partial next to them used an allowlist, so both wrote partials under configs/ that no page can include. All three now use the one allowlist. * fix(rpcn): keep version-only connectors, cover the flags, hoist the compiles Three small things the review left open. The description partial's outer guard tested (or summary description), so a connector carrying only a version got no partial at all and its "Introduced in version" line was lost. It now includes version. --template-description, --template-metadata and the new --prune-orphaned-descriptions had no coverage: the flag could be deleted with the whole suite staying green, and the CLI contract check does not notice a documented flag going missing. Tests now assert a stub template reaches the emitted partial and that the CLI still advertises the flags. intro.hbs gets a test too, pinning that the fallback body path applies the same brace escaping and heading separation as the partial. The four one-line partial wrappers were recompiled once per component, and the metadata and description partials were rendered before the type guard, so both were computed and discarded for every bloblang function, method and config entry. Compile once above the loop, render inside the guard. * chore(release): 5.16.0 The branch sat at 5.12.0, which is main's version and is already published on npm, so merging would have shipped nothing: not the partials, not the wiring, not the CLI flags. 5.16.0 is this PR's slot in the agreed merge order and is free on the registry. * docs: Auto-update CLI reference documentation (PR #227) * fix(cli-docs): register the new command and stop stealing another's prose The auto-regeneration workflow deleted the migrate-rpcn-descriptions section from CLI_REFERENCE.adoc on the first push, which was correct: the file is generated, and the section had been hand-written. Two separate defects sat behind that, both introduced here. First, tools/generate-cli-docs.js drives its output from a hardcoded generateSubcommands list and the new command was not in it, so no section was emitted at all. Second, and worse, adding the command's JSDoc broke its neighbour. The generator pairs a comment with the NEXT .command() call it finds, and this change had stacked both JSDoc blocks together with the two command chains after them in the opposite order, so neither comment was adjacent to its own command. The regenerated migrate-rpcn-metadata section fell back to truncated help text, and migrate-rpcn-descriptions was documented with migrate-rpcn-metadata's prose. Each JSDoc block now sits immediately before the command it documents, and the regenerated migrate-rpcn-metadata section is byte-identical to main again. The JSDoc regex is also tempered so a comment body cannot run past its own closing delimiter, which is what let two adjacent blocks merge into one match. Both failure modes are silent, so both are now tested: one check that every registered generate subcommand is known to the generator, one that every JSDoc block is adjacent to its own command. Three commands are already undocumented on main (migrate-property-refs, rpk-plugin-stubs, rpk-overrides); they are listed explicitly so the gap is visible and cannot grow, with a second test failing if an entry stops naming a real command. PR #264 retires these hardcoded lists by deriving the reference from the CLI. * fix(deps): take main's dependency set instead of carrying a stale pin forward This branch never meant to change any dependency, but resolving the package.json conflict from our side kept its own stale pins: @antora/cli and @antora/site-generator at 3.1.4 where main has ^3.1.15. Two ways that hurt, and CI catches neither, because each branch is internally consistent: - On #258 the lockfile and manifest disagreed and `npm ci` failed outright with "Missing: @asciidoctor/core@2.2.8 from lock file", which is what surfaced this. - On the others the branch stayed green while a merge would have DOWNGRADED main's @antora/cli back to 3.1.4. Verified with git merge-tree. main owns dependencies, devDependencies and engines. Both package files are now main's, with only this PR's version stamped and its own exports/scripts additions preserved, and `npm ci --dry-run` passes against the pair. * fix(redpanda-connect): stop dropping the PR summary when there's no diff summaryDiff = masterDiff || diffJson is null on every run with no prior version to diff against or matching versions, and the next line unconditionally set a property on it -- "Cannot set properties of null" -- silently caught by the surrounding try/catch, which meant the entire PR summary (not just descriptionReports) never printed. Reproduced live twice against real fetched Connect data. When there's no diff, lostSectionWarnings/descriptionReports now print on their own via the same renderers generatePRSummary already uses, instead of going through the diff-shaped summary path at all. Also: metadata-utils.js's shared verbatim tracker recognized AsciiDoc `----` listing blocks but not `....` literal blocks, which are equally substitution-free -- confirmed via a real Asciidoctor convert() that a `{token}` inside a .... block was getting corrupted by the brace-escaper. Added a matching LITERAL_DELIMITER state, symmetric with the existing one, plus 4 regression tests. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The v26.1.14 property
log_eviction_exempt_topicsleaked raw C++ into its rendered default:[model::schema_registry_internal_tp.topic()]instead of["_schemas"](visible in redpanda-data/docs#1824, patched there with a manualdefaultoverride).Root cause
Two gaps in the constant resolver:
FUNCTION_CALL_PATTERN(([a-zA-Z0-9_:]+)\(\)) doesn't admit., so a member call likemodel::schema_registry_internal_tp.topic()never reachedresolve_cpp_function_callat all — the existing machinery only handles free zero-arg functions (model::kafka_audit_logging_topic()).ConstexprCachehas no pattern for inlinemodel::topic_partitionconstants, so there was nothing to resolve against (src/v/model/namespace.h:inline const model::topic_partition schema_registry_internal_tp{model::topic{"_schemas"}, ...}).Fix
FUNCTION_CALL_PATTERNto admit dots (calls with arguments still don't match).<variable>.topic → literalfor inlinetopic_partitiondeclarations, under both qualified (model::x.topic) and unqualified names.Testing
New
test_constant_resolution.py(5 tests: qualified/unqualified resolution against a syntheticnamespace.h, member-call and free-function pattern matching, args-rejection). Also verified against the real redpanda checkout:resolve_cpp_function_call('model::schema_registry_internal_tp.topic')→_schemas, andmodel::kafka_audit_logging_topicstill resolves to_redpanda.audit_log.Version bumped to 5.2.6 (adjust on merge if #225's 5.3.0 lands first).