Skip to content

feat: mxcli search --exclude-marketplace leaves out Marketplace modules - #1493

Open
stagknee wants to merge 2 commits into
mendixlabs:mainfrom
stagknee:feat/search-exclude-marketplace
Open

stagknee wants to merge 2 commits into
mendixlabs:mainfrom
stagknee:feat/search-exclude-marketplace

Conversation

@stagknee

@stagknee stagknee commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Part of #1477. It was split out of #1478 at review. #1478 is now merged, so this PR is rebased on main. It has two commits: the flag, and the search --help wording suggested in review of #1478 ("the documentation of every element the catalog records" instead of a hand-written list that would drift).

What it does

mxcli search … --exclude-marketplace leaves out hits inside modules downloaded from the Marketplace. An audit of the app's own text usually wants this, because the vendor's documentation isn't yours to edit. The flag is opt-in: search is read-only, and existing callers must see the same results. The SEARCH statement is unchanged.

How

  • SearchOptions{ExcludeMarketplace bool} and Executor.SearchWith add the filter to both the strings and the source query, in table, names and JSON formats.
  • marketplaceModuleFilter reads modules for the current snapshot only, using the idiom from AddGraphAnalysis: SnapshotId = (select SnapshotId from snapshots order by rowid desc limit 1). So a module that is Marketplace-sourced in another snapshot is not excluded here.
    • One limit remains: the strings/source FTS tables have no SnapshotId, so the hits themselves are not snapshot-scoped, as before.
  • Docs: a "Marketplace modules" section in docs-site/src/tools/search.md, plus the help text, the syntax entry and a CHANGELOG entry under Added.

Evidence

  • Tests:
    • TestSearchWith_ExcludeMarketplace, which includes a documented Marketplace module row, the case from the review;
    • TestSearchWith_ExcludeMarketplace_AllFormats;
    • TestSearchWith_ExcludeMarketplace_UsesTheCurrentSnapshot, with two snapshots in both directions.
    • Removing the snapshot scope fails the snapshot test, and disabling the filter fails all three.
  • Real app (Mendix 11.12.5, with 7 Marketplace modules):
    • search "keyboard focus" returns WebActions.FocusNext and WebActions.FocusPrevious (JAVASCRIPT_ACTION, documentation).
    • With --exclude-marketplace it returns No matches found.
    • The app's own documented constant is still found with the flag.
  • Windows: the failing tests across mdl/catalog, mdl/executor and cmd/mxcli are the same set as on main.
  • Linux (golang:1.26.6-bookworm, ANTLR 4.13.2, as in CI): make grammar build test check-mdl check-findings check-conformance check-migration-reference lint-go all pass.

--fail-on-match (#1492) also adds a field to SearchOptions. Whichever PR lands second will need a small rebase, and I'll do it.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

  1. In mdl/executor/cmd_catalog.go, the marketplaceModuleFilter constant uses string concatenation that could be formatted as a single string literal for readability. However, this is a minor style preference and does not affect functionality.

  2. The buildDocumentationStrings function in mdl/catalog/builder_strings.go contains a long SQL query. While acceptable, consider extracting repetitive patterns (like the CASE WHEN ObjectType = 'MODULE' THEN Name ELSE ModuleName END logic) into a helper if similar patterns emerge elsewhere. Not required for this change.

  3. The searchCacheFixture refactor in mdl/executor/cmd_search_source_index_test.go introduces a new helper function. This is beneficial for test flexibility but slightly increases surface area. The change is justified by the need to seed documentation rows for tests.

What Looks Good

  • Comprehensive test coverage: New unit tests cover documentation indexing, exclude-marketplace functionality across all formats, and snapshot scoping. Real-app validation confirms behavior matches expectations.
  • Documentation updates: All relevant documentation files (user-facing, reference, internal schema, and skills) are updated to reflect the new search behavior and excluded marketplace modules.
  • Schema version bump: Correctly increments CatalogSchemaVersion to 25 to invalidate caches due to the new documentation rows in the strings table.
  • Snapshot-aware filtering: The exclude-marketplace logic correctly uses only the current snapshot via (select SnapshotId from snapshots order by rowid desc limit 1), preventing incorrect exclusion of modules that changed sources across snapshots.
  • Documentation indexing completeness: Indexes documentation for all relevant element types (entities, attributes, constants, pages, etc.) while avoiding duplicates (microflow/workflow documentation) and irrelevant types (enumeration values, contract entities).
  • Log message indexing: Correctly extends existing log node name indexing to include log message text, closing a gap where words in log messages were undiscoverable.
  • CLI integration: The --exclude-marketplace flag is properly added to the search command with clear help text and examples.
  • Atomic changes: Each modification serves a single purpose (catalog schema, documentation indexing, marketplace filtering, CLI flag, documentation).
  • Backward compatibility: Default behavior unchanged; existing callers see identical results unless opting in via the flag.
  • Consistency with patterns: Uses established patterns like SearchOptions (similar to other executor options) and follows the existing code style.

Recommendation

Approve the PR. The changes are well-tested, documented, and address the stated requirements without introducing regressions or violating project conventions. The feature is ready for merging.


Automated review via OpenRouter (Nemotron Super 120B) — workflow source

@stagknee
stagknee force-pushed the feat/search-exclude-marketplace branch from 5d8c44c to f6c4412 Compare October 14, 2026 23:07
@ako

ako commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

The PR has merge conflicts

@ako ako closed this Oct 15, 2026
@ako ako reopened this Oct 15, 2026
@github-actions

Copy link
Copy Markdown

AI Code Review

What Looks Good

  • Comprehensive testing: New test file (mdl/executor/cmd_search_exclude_marketplace_test.go) covers basic functionality, all output formats, and snapshot scoping with multiple test cases verifying correct behavior.
  • Real-world validation: Tested against a Mendix 11.12.5 app with 7 Marketplace modules, showing the flag correctly excludes Marketplace hits while preserving app-owned hits.
  • Documentation updates:
    • Added --exclude-marketplace to CLI help text
    • Updated docs-site/src/tools/search.md with examples and explanation
    • Updated docs-site/src/reference/query/search.md to reference the CLI flag
    • Added CHANGELOG entry under "Added"
  • Correct snapshot scoping: Uses the same idiom as AddGraphAnalysis to get current snapshot (SnapshotId = (select SnapshotId from snapshots order by rowid desc limit 1)), verified by snapshot tests.
  • Consistent implementation: Applies filter to both strings and source FTS tables across all output formats (table, names, JSON).
  • Atomic commits: First commit implements the feature, second commits the help wording improvement from review - each does one thing.
  • No regressions: Windows shows same test failures as main; Linux build/test/lint passes.

Recommendation

Approve the PR. The feature is well-implemented, thoroughly tested, properly documented, and follows project conventions. It addresses a real user need (auditing app-owned text by excluding vendor documentation) while maintaining backward compatibility (opt-in flag). The snapshot scoping limitation is correctly implemented and tested. No changes needed.


Automated review via OpenRouter (Nemotron Super 120B) — workflow source

stagknee and others added 2 commits October 15, 2026 12:19
…es (mendixlabs#1477)

An audit of the app's own text reported a vendor module's text beside the
project's. The flag drops hits whose module was downloaded from the
Marketplace, a module's own documentation included. It is off by default and
the SEARCH statement is unchanged. Which modules are Marketplace modules is
read from the current snapshot only, so a module that was one in an older
snapshot is not left out of every later search.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… a list

The hand-written list of document types would drift as the objects view
grows; "every element the catalog records" stays true. Suggested in review
of mendixlabs#1478.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@stagknee

Copy link
Copy Markdown
Contributor Author

Rebased on main now that #1492 has landed. The conflicts were the expected ones: both PRs added a field to SearchOptions and edited the same help, syntax and docs lines.

The exclude-marketplace tests now use #1492's (int, error) return. Both test sets pass together:

  • TestSearchWith_ExcludeMarketplace*;
  • TestSearchWith_ReturnsTheNumberOfHits and TestSearchWith_StrictReportsAQueryTheIndexRejects.

On Linux (golang:1.26.6-bookworm, ANTLR 4.13.2), make grammar build test check-mdl check-findings check-conformance check-migration-reference lint-go all pass.

@stagknee
stagknee force-pushed the feat/search-exclude-marketplace branch from f6c4412 to e425fa4 Compare October 15, 2026 19:31
@github-actions

Copy link
Copy Markdown

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

None found.

What Looks Good

  • Feature implementation: The --exclude-marketplace flag is correctly implemented as an opt-in addition to mxcli search, leaving existing behavior unchanged by default. The flag correctly filters out Marketplace modules based on the current snapshot only, as intended.
  • Test coverage: Comprehensive unit tests validate the flag's behavior across all output formats (table, names, JSON) and snapshot scenarios, including verification that the filter uses the current snapshot and that excluding Marketplace modules works correctly for both module documentation and domain model documentation.
  • Documentation: All relevant documentation is updated:
    • CLI help text now includes the new flag
    • tools/search.md adds a dedicated "Marketplace modules" section
    • reference/query/search.md notes the CLI extension
    • CHANGELOG.md provides a clear entry under Added
  • Code quality:
    • Changes are minimal and focused, following existing patterns (e.g., extending SearchOptions and reusing the snapshot selection idiom from AddGraphAnalysis)
    • The marketplaceModuleFilter constant is well-defined and correctly scoped to the current snapshot
    • No modifications to the SEARCH statement or MDL syntax (as intended)
    • Generated parser files are appropriately noted as noise and skipped in review

Recommendation

Approve. The PR is ready for merge. It cleanly implements the requested feature with appropriate test coverage, documentation, and adherence to project conventions. The opt-in design preserves backward compatibility, and the snapshot-scoped filtering logic is sound for the stated use case (auditing app-owned text). No changes are needed.


Automated review via OpenRouter (Nemotron Super 120B) — workflow source

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants