Skip to content

feat: integrate platzky plugins with static assets and field rendering - #351

Merged
raven-wing merged 29 commits into
Problematy:mainfrom
raven-wing:better_plugins
May 13, 2026
Merged

raven-wing merged 29 commits into
Problematy:mainfrom
raven-wing:better_plugins

Conversation

@raven-wing

@raven-wing raven-wing commented May 12, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Extensible plugin support: discoverable plugins can supply components, shortcodes, and static assets; app exposes a plugin manifest and serves plugin assets with permissive CORS.
  • Bug Fixes

    • Improved exception logging behavior across several endpoints.
  • Documentation

    • Added plugin developer docs, quickstart data-model section, and updated Sphinx version lookup and docs navigation.
  • Chores

    • Default frontend library updated to v1.6.1; minor tooling/config tweaks.
  • Tests

    • Added and expanded tests for plugins, field handling, admin routes, and model initialization.
  • Refactor

    • Location-model initialization and setup logic reorganized.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds backend plugin discovery (platzky entry points) and per-plugin static blueprints with CORS, threads field-renderer mappings into API handlers, extends the formatter to wrap/encode configured plugin dict fields and drop unconfigured payloads, exposes PLUGIN_MANIFEST to the frontend template, updates the data-model categories API, and adds docs and tests.

Changes

Plugin System Implementation

Layer / File(s) Summary
Foundation: imports, versions, and config
goodmap/goodmap.py, goodmap/config.py, pyproject.toml
Adds imports for entry-point/static discovery and BaseModel typing; bumps default frontend lib URL and updates platzky and Ruff config in pyproject.toml.
Plugin discovery and static registration
goodmap/goodmap.py
Discovers platzky.plugins entry points, attempts to load plugin modules, registers /plugins/<name> blueprints serving on-disk static/ dirs with per-response CORS headers, and accumulates plugin_manifest stored on the app and passed to templates.
Location model setup and validators
goodmap/goodmap.py, goodmap/data_models/location.py
Refactors location-model/category initialization into _setup_location_model(...), changes create_location_model to accept categories={}, and centralizes allowed-value and per-item length validators and JSON schema metadata composition.
API field renderer threading
goodmap/core_api.py
Extends core_pages(...) with field_renderers and passes derived field_renderers into prepare_pin for /location responses; replaces several logger.error(..., exc_info=True) calls with logger.exception(...).
Field plugin wrapping and transformation
goodmap/formatter.py, tests/unit_tests/test_formatter.py
Implements _apply_field_plugin to wrap configured dict fields with scope and base64-encode code strings, drops unconfigured plugin-like dicts, extends prepare_pin to accept field_plugins, and adds unit tests covering wrap, pass-through, and drop behaviors.
Frontend manifest and template changes
goodmap/templates/map.html
Adds window.PLUGIN_MANIFEST to the global config, consolidates LOCATION_SCHEMA into the same script block, and adds crossorigin="anonymous" to the frontend library script tag.
Plugin system documentation and Sphinx config
docs/index.rst, docs/plugins.rst, docs/conf.py, docs/quickstart.rst
Replaces frontend-integration TOC entry with plugins, adds plugins.rst documenting discovery and configuration, adds Data Model section to quickstart, and switches Sphinx version lookup to importlib.metadata.
Tests: plugin registration, config, logging, and DB updates
tests/unit_tests/*
Adds plugin-related tests, admin-route tests, formatter plugin tests, updates _FRONTEND_LIB_URL constant in config tests, adapts core_api logging assertions to use exception, and updates many tests to the new create_location_model(..., {}) signature.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Problematy/goodmap#261: Both PRs update the goodmap_frontend_lib_url configuration default/handling and ensure templates load the frontend JS via that configurable URL.
  • Problematy/goodmap#330: Both modify core_pages(...) signature/behavior and endpoint wiring related to field rendering.

Poem

🐰 I found a plugin by the willow root,
entry-points chimed a tiny route,
blueprints flapped their static flags with glee,
scope-wrapped fields hummed base64 melody,
rabbit hops — load, render, and be free!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 46.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: integrate platzky plugins with static assets and field rendering' directly and comprehensively summarizes the main changes: plugin discovery, static asset serving, and field rendering integration via field_plugins parameter.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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 `@goodmap/formatter.py`:
- Around line 37-40: The merge currently uses result = {"scope":
field_plugins[field], **value} which still allows an attacker-supplied
value["scope"] to override the trusted scope; change the merge so the trusted
scope wins (e.g., merge value first and then set "scope" from field_plugins or
explicitly filter out "scope" from value before merging) in the function in
formatter.py that builds result so the configured scope in field_plugins[field]
cannot be overwritten by payload data.

In `@goodmap/goodmap.py`:
- Around line 102-123: The code currently registers a Blueprint for any plugin
with a static_dir and always appends a plugin_manifest entry even when
remoteEntry.js is missing; change the logic to check for the actual file
(os.path.isfile(os.path.join(static_dir, "remoteEntry.js"))) before appending to
plugin_manifest (keep registering the Blueprint and the _add_cors handler so
other static assets still serve), and only add the manifest entry with "url":
f"/plugins/{ep.name}/static/remoteEntry.js" and "module": "./Button" when that
file exists.
- Around line 124-125: The current blanket except in the plugin discovery block
hides useful error info—change the exception handling around the plugin loading
logic (where ep.load(), inspect.getfile(), and related filesystem ops are
called) to log the actual exception and traceback instead of swallowing it;
update the logger.warning call for plugin '%s' to include exc_info=True (and
optionally log the exception message) so the traceback and error details are
preserved when failures occur.
🪄 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: 8b5bca0d-493e-43e2-9d95-8abb0140d933

📥 Commits

Reviewing files that changed from the base of the PR and between bfbcae0 and 37bc898.

⛔ Files ignored due to path filters (1)
  • poetry.lock is excluded by !**/*.lock
📒 Files selected for processing (9)
  • docs/index.rst
  • docs/plugins.rst
  • goodmap/config.py
  • goodmap/core_api.py
  • goodmap/formatter.py
  • goodmap/goodmap.py
  • goodmap/templates/map.html
  • pyproject.toml
  • tests/unit_tests/test_formatter.py

Comment thread goodmap/formatter.py Outdated
Comment thread goodmap/goodmap.py Outdated
Comment thread goodmap/goodmap.py Outdated
@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: bad1483

📊 E2E Stress Test Performance

Status: PASSED (13510.61ms max < 25000ms limit)

Metric Value
Average Time 12199.87ms
Minimum Time 11346.43ms
Maximum Time 13510.61ms
Completed Runs 5/5
Avg Markers Loaded 71
📈 Individual Run Times
Run Time (ms) Markers
Run 1 11762.71ms 71
Run 2 11346.43ms 71
Run 3 12431.9ms 71
Run 4 11947.68ms 71
Run 5 13510.61ms 71

@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: 7e06531

📊 E2E Stress Test Performance

Status: PASSED (12786.5ms max < 25000ms limit)

Metric Value
Average Time 11097.3ms
Minimum Time 10009.49ms
Maximum Time 12786.5ms
Completed Runs 5/5
Avg Markers Loaded 71
📈 Individual Run Times
Run Time (ms) Markers
Run 1 10009.49ms 71
Run 2 10809.75ms 71
Run 3 10750.16ms 71
Run 4 11130.58ms 71
Run 5 12786.5ms 71

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@tests/unit_tests/test_goodmap.py`:
- Around line 129-130: This file fails the project's Black formatting check; run
the code formatter and commit the changes so CI passes: run `black` (or your
repo's configured Black command) on tests/unit_tests/test_goodmap.py, fix
formatting differences flagged around the regions previously noted (near the
blocks around lines referenced 129-130 and 197-198), stage the updated file, and
push the commit; no functional changes are needed—only reformatting via Black
for functions/classes in that file.
🪄 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: 978e3a37-6090-4a87-b59c-73d7539269c0

📥 Commits

Reviewing files that changed from the base of the PR and between 37bc898 and 4cd3599.

📒 Files selected for processing (7)
  • goodmap/config.py
  • goodmap/data_models/location.py
  • goodmap/goodmap.py
  • pyproject.toml
  • tests/unit_tests/test_config.py
  • tests/unit_tests/test_formatter.py
  • tests/unit_tests/test_goodmap.py
✅ Files skipped from review due to trivial changes (1)
  • goodmap/data_models/location.py
🚧 Files skipped from review as they are similar to previous changes (3)
  • goodmap/config.py
  • pyproject.toml
  • tests/unit_tests/test_formatter.py

Comment thread tests/unit_tests/test_goodmap.py
Comment thread docs/conf.py Fixed
@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: 361cf42

⚠️ E2E Stress Test Results

Performance data not found. See workflow logs for details.

@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: b90ac35

📊 E2E Stress Test Performance

Status: PASSED (11004.14ms max < 25000ms limit)

Metric Value
Average Time 10213.93ms
Minimum Time 9586.32ms
Maximum Time 11004.14ms
Completed Runs 5/5
Avg Markers Loaded 71
📈 Individual Run Times
Run Time (ms) Markers
Run 1 10103.12ms 71
Run 2 10374.19ms 71
Run 3 9586.32ms 71
Run 4 10001.86ms 71
Run 5 11004.14ms 71

@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: 6770fd9

📊 E2E Stress Test Performance

Status: PASSED (11000.28ms max < 25000ms limit)

Metric Value
Average Time 10420.56ms
Minimum Time 9958.64ms
Maximum Time 11000.28ms
Completed Runs 5/5
Avg Markers Loaded 71
📈 Individual Run Times
Run Time (ms) Markers
Run 1 10359.35ms 71
Run 2 9958.64ms 71
Run 3 10524.55ms 71
Run 4 10259.98ms 71
Run 5 11000.28ms 71

@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: 2b948c6

📊 E2E Stress Test Performance

Status: PASSED (14359.95ms max < 25000ms limit)

Metric Value
Average Time 12306.35ms
Minimum Time 11333.84ms
Maximum Time 14359.95ms
Completed Runs 5/5
Avg Markers Loaded 71
📈 Individual Run Times
Run Time (ms) Markers
Run 1 11333.84ms 71
Run 2 12173.22ms 71
Run 3 11922.69ms 71
Run 4 11742.06ms 71
Run 5 14359.95ms 71

@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: d3b1c71

📊 E2E Stress Test Performance

Status: PASSED (11586.51ms max < 25000ms limit)

Metric Value
Average Time 10583.59ms
Minimum Time 9967.36ms
Maximum Time 11586.51ms
Completed Runs 5/5
Avg Markers Loaded 71
📈 Individual Run Times
Run Time (ms) Markers
Run 1 9967.36ms 71
Run 2 10449.95ms 71
Run 3 10444.89ms 71
Run 4 10469.25ms 71
Run 5 11586.51ms 71

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tests/unit_tests/test_goodmap.py (1)

136-152: ⚡ Quick win

Use a real module object in make_mock_entry_point instead of MagicMock.

inspect.getfile() raises TypeError when passed a MagicMock object. Without patching, test_register_plugin_static_resources_no_static_dir exits via the exception handler instead of testing the intended "no static directory" code path.

Proposed fix
+import types
+
 def make_mock_entry_point(name: str, module_path: str):
@@
-    mock_module = mock.MagicMock()
-    mock_module.__file__ = init_file
-    spec.load.return_value = mock_module
+    module = types.ModuleType(name)
+    module.__file__ = init_file
+    spec.load.return_value = module
     return spec
🤖 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/unit_tests/test_goodmap.py` around lines 136 - 152, The test helper
make_mock_entry_point currently returns a MagicMock as the module which causes
inspect.getfile() to raise TypeError; change it to create and return a real
module object (e.g., types.ModuleType(name)) and set its __file__ to the created
init_file so inspect.getfile and importlib behavior work; keep spec as the
mocked EntryPoint and set spec.load.return_value to that real module (and
optionally set __package__ or other attrs if needed) so
test_register_plugin_static_resources_no_static_dir follows the intended code
path.
🤖 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 `@tests/unit_tests/test_goodmap.py`:
- Around line 136-152: The test helper make_mock_entry_point currently returns a
MagicMock as the module which causes inspect.getfile() to raise TypeError;
change it to create and return a real module object (e.g.,
types.ModuleType(name)) and set its __file__ to the created init_file so
inspect.getfile and importlib behavior work; keep spec as the mocked EntryPoint
and set spec.load.return_value to that real module (and optionally set
__package__ or other attrs if needed) so
test_register_plugin_static_resources_no_static_dir follows the intended code
path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a9c260f-615c-4d6e-abe9-d7c376c0b606

📥 Commits

Reviewing files that changed from the base of the PR and between 4cd3599 and d3b1c71.

⛔ Files ignored due to path filters (1)
  • poetry.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • docs/conf.py
  • goodmap/core_api.py
  • goodmap/goodmap.py
  • pyproject.toml
  • tests/unit_tests/test_core_api.py
  • tests/unit_tests/test_goodmap.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • goodmap/core_api.py

@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: f568877

📊 E2E Stress Test Performance

Status: PASSED (15018.22ms max < 25000ms limit)

Metric Value
Average Time 13099.94ms
Minimum Time 12301.79ms
Maximum Time 15018.22ms
Completed Runs 5/5
Avg Markers Loaded 71
📈 Individual Run Times
Run Time (ms) Markers
Run 1 12898.59ms 71
Run 2 12538.57ms 71
Run 3 15018.22ms 71
Run 4 12301.79ms 71
Run 5 12742.55ms 71

@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: cca1096

📊 E2E Stress Test Performance

Status: PASSED (11851.99ms max < 25000ms limit)

Metric Value
Average Time 10953.56ms
Minimum Time 10093.74ms
Maximum Time 11851.99ms
Completed Runs 5/5
Avg Markers Loaded 71
📈 Individual Run Times
Run Time (ms) Markers
Run 1 10592.18ms 71
Run 2 11851.99ms 71
Run 3 11090.93ms 71
Run 4 11138.95ms 71
Run 5 10093.74ms 71

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
goodmap/data_models/location.py (1)

1-225: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

CI lint failure: Black wants to reformat this file.

The lint job in the Testing pipeline is failing because goodmap/data_models/location.py is not Black-formatted. Please run poetry run black . (or poetry run black goodmap/data_models/location.py) and commit the result before merging.

🤖 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 `@goodmap/data_models/location.py` around lines 1 - 225, The file is failing CI
because it is not Black-formatted; run the code formatter and commit the result:
run "poetry run black ." (or "poetry run black goodmap/data_models/location.py")
to reformat goodmap/data_models/location.py, verify the changes include the
LocationBase class and create_location_model function signatures/spacing are
updated, then add and commit the formatted file so the lint job passes.
🧹 Nitpick comments (1)
docs/plugins.rst (1)

50-52: ⚡ Quick win

Clarify "silently dropped" wording.

The phrase "silently dropped" on line 51 contradicts the following sentence stating "A debug message is logged." The behavior is silent from the API response perspective but not from the logging perspective.

📝 Suggested rewording
 If a plugin is removed from the configuration while a location still has
-fields referencing it, those fields are silently dropped from the API
-response. A debug message is logged:
+fields referencing it, those fields are dropped from the API response
+with a debug message logged:
🤖 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 `@docs/plugins.rst` around lines 50 - 52, The wording "silently dropped" is
misleading because the next sentence says "A debug message is logged"; update
the sentence containing the phrase "silently dropped" to clarify that the fields
are removed from the API response (i.e., no longer returned) while a debug
message is still emitted; specifically replace or rephrase the phrase "silently
dropped" and keep the following sentence "A debug message is logged" so readers
understand the silence is only from the API-response perspective, not from
logging.
🤖 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 `@docs/plugins.rst`:
- Line 21: The docs mention visible_data but never defines it—add a concise
definition and example where the term appears (the paragraph containing
"visible_data and the plugin is configured, the API wraps the field value")
explaining that visible_data is a flag/field in Goodmap's data model indicating
which record fields are exposed to plugins and API responses; include one short
sentence about its possible values (e.g., boolean or list of field names) and a
tiny example of its effect on an API response so readers immediately understand
how it controls visible fields.

In `@goodmap/data_models/location.py`:
- Around line 187-190: The function signature for create_location_model uses a
mutable default for categories ({}), which triggers B006 and breaks callers that
previously passed None; change the parameter to accept None (categories:
dict[str, list[str]] | None = None) and normalize at the start of
create_location_model with a local assignment (e.g., categories = categories or
{}) so callers passing None still work, no mutable default is used, and existing
uses of categories.get(...) remain valid; update any type hints/docstrings
accordingly.

---

Outside diff comments:
In `@goodmap/data_models/location.py`:
- Around line 1-225: The file is failing CI because it is not Black-formatted;
run the code formatter and commit the result: run "poetry run black ." (or
"poetry run black goodmap/data_models/location.py") to reformat
goodmap/data_models/location.py, verify the changes include the LocationBase
class and create_location_model function signatures/spacing are updated, then
add and commit the formatted file so the lint job passes.

---

Nitpick comments:
In `@docs/plugins.rst`:
- Around line 50-52: The wording "silently dropped" is misleading because the
next sentence says "A debug message is logged"; update the sentence containing
the phrase "silently dropped" to clarify that the fields are removed from the
API response (i.e., no longer returned) while a debug message is still emitted;
specifically replace or rephrase the phrase "silently dropped" and keep the
following sentence "A debug message is logged" so readers understand the silence
is only from the API-response perspective, not from logging.
🪄 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: 8976d273-ea58-4a21-bb32-927a98a37206

📥 Commits

Reviewing files that changed from the base of the PR and between d3b1c71 and 577e63d.

📒 Files selected for processing (4)
  • docs/conf.py
  • docs/plugins.rst
  • goodmap/data_models/location.py
  • goodmap/formatter.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/conf.py
  • goodmap/formatter.py

Comment thread docs/plugins.rst Outdated
Comment thread goodmap/data_models/location.py
@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: b6a3cb4

📊 E2E Stress Test Performance

Status: PASSED (9958.3ms max < 25000ms limit)

Metric Value
Average Time 9381.92ms
Minimum Time 8996.14ms
Maximum Time 9958.3ms
Completed Runs 5/5
Avg Markers Loaded 71
📈 Individual Run Times
Run Time (ms) Markers
Run 1 9387.46ms 71
Run 2 9200.55ms 71
Run 3 9958.3ms 71
Run 4 8996.14ms 71
Run 5 9367.15ms 71

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
goodmap/core_api.py (1)

95-105: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align parameter name field_renderers with formatter signature; pass field_plugins as keyword argument for clarity.

The parameter field_renderers introduced at line 104 is passed to prepare_pin() in goodmap/formatter.py:47, which names it field_plugins. The misleading name ("renderers" suggests render functions, but it actually maps field names to plugin scope names as documented in the formatter). Since only one call site exists (goodmap/goodmap.py:177), rename field_renderers to field_plugins throughout core_api.py (function signature, parameter storage, and the prepare_pin call at line 376) for consistency. Additionally, pass field_plugins as a keyword argument when calling prepare_pin to improve clarity.

🤖 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 `@goodmap/core_api.py` around lines 95 - 105, Rename the core_pages parameter
field_renderers to field_plugins in the core_pages function signature and all
uses within core_api.py (ensure any local storage/assignment uses the new name),
and update the prepare_pin call to pass it explicitly as the keyword argument
field_plugins=field_plugins (prepare_pin in goodmap/formatter.py expects
field_plugins); update the one call site in goodmap/goodmap.py if referenced
here and replace any remaining references to field_renderers with field_plugins
for consistent naming.
🧹 Nitpick comments (1)
goodmap/core_api.py (1)

376-378: ⚡ Quick win

Use keyword argument field_plugins=field_renderers for clarity and to expose the naming inconsistency.

The function parameter is named field_plugins while the variable being passed is field_renderers. Passing it positionally hides this mismatch. Using a keyword argument makes the intent explicit and surfaces the naming divergence so you can decide whether to align them.

♻️ Suggested clarification
-        formatted_data = prepare_pin(
-            location.model_dump(), visible_data, meta_data, field_renderers
-        )
+        formatted_data = prepare_pin(
+            location.model_dump(),
+            visible_data,
+            meta_data,
+            field_plugins=field_renderers,
+        )
🤖 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 `@goodmap/core_api.py` around lines 376 - 378, Call to prepare_pin passes
field_renderers positionally which hides the parameter name mismatch; change the
call to use the keyword argument field_plugins=field_renderers so the invocation
reads prepare_pin(location.model_dump(), visible_data, meta_data,
field_plugins=field_renderers) and thus makes the naming inconsistency explicit
(prepare_pin, field_renderers, field_plugins).
🤖 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.

Outside diff comments:
In `@goodmap/core_api.py`:
- Around line 95-105: Rename the core_pages parameter field_renderers to
field_plugins in the core_pages function signature and all uses within
core_api.py (ensure any local storage/assignment uses the new name), and update
the prepare_pin call to pass it explicitly as the keyword argument
field_plugins=field_plugins (prepare_pin in goodmap/formatter.py expects
field_plugins); update the one call site in goodmap/goodmap.py if referenced
here and replace any remaining references to field_renderers with field_plugins
for consistent naming.

---

Nitpick comments:
In `@goodmap/core_api.py`:
- Around line 376-378: Call to prepare_pin passes field_renderers positionally
which hides the parameter name mismatch; change the call to use the keyword
argument field_plugins=field_renderers so the invocation reads
prepare_pin(location.model_dump(), visible_data, meta_data,
field_plugins=field_renderers) and thus makes the naming inconsistency explicit
(prepare_pin, field_renderers, field_plugins).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: aa3a7151-c1ca-481d-bfcf-692c9774d695

📥 Commits

Reviewing files that changed from the base of the PR and between 577e63d and 5508fff.

📒 Files selected for processing (5)
  • goodmap/core_api.py
  • goodmap/data_models/location.py
  • tests/unit_tests/data_models/test_location.py
  • tests/unit_tests/test_db.py
  • tests/unit_tests/test_goodmap.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • goodmap/data_models/location.py

@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: 5508fff

📊 E2E Stress Test Performance

Status: PASSED (11335.41ms max < 25000ms limit)

Metric Value
Average Time 10807.52ms
Minimum Time 10279.36ms
Maximum Time 11335.41ms
Completed Runs 5/5
Avg Markers Loaded 71
📈 Individual Run Times
Run Time (ms) Markers
Run 1 10279.36ms 71
Run 2 10351.24ms 71
Run 3 11310.29ms 71
Run 4 10761.28ms 71
Run 5 11335.41ms 71

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: e0ef8c1

⚠️ E2E Stress Test Results

Performance data not found. See workflow logs for details.

@raven-wing raven-wing changed the title feat: add platkzy plugin shotcodes to be handled in locations feat: integrate platzky plugins with static assets and field rendering May 13, 2026
@github-actions

Copy link
Copy Markdown

🧪 E2E Test Results

📊 View full workflow run
🔗 Commit: 21fafa7

📊 E2E Stress Test Performance

Status: PASSED (11340.73ms max < 25000ms limit)

Metric Value
Average Time 10684.74ms
Minimum Time 10277.94ms
Maximum Time 11340.73ms
Completed Runs 5/5
Avg Markers Loaded 71
📈 Individual Run Times
Run Time (ms) Markers
Run 1 10277.94ms 71
Run 2 10410.79ms 71
Run 3 11340.73ms 71
Run 4 10576.26ms 71
Run 5 10817.96ms 71

@raven-wing

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@raven-wing

Copy link
Copy Markdown
Collaborator Author

@coderabbitai can you rereview?

@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

@raven-wing Sure! Triggering a full re-review of PR #351 now.

✅ Actions performed

Full review triggered.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
tests/unit_tests/test_goodmap.py (2)

275-275: ⚡ Quick win

Use context manager for file operations.

Same issue as Line 141: opening a file without a context manager is not idiomatic Python.

♻️ Refactor to use context manager
-        # Create a test file in the static dir
-        open(os.path.join(static_dir, "test.js"), "w").close()
+        # Create a test file in the static dir
+        with open(os.path.join(static_dir, "test.js"), "w"):
+            pass
🤖 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/unit_tests/test_goodmap.py` at line 275, The test currently creates an
empty file by calling open(os.path.join(static_dir, "test.js"), "w").close()
without a context manager; change this to use a with-context to open the same
path (constructed from static_dir and "test.js") for writing so the file is
closed automatically when the block exits, ensuring proper resource handling in
the test_goodmap test.

141-141: ⚡ Quick win

Use context manager for file operations.

Opening a file without a context manager is not idiomatic Python. Although .close() is called immediately, a context manager ensures cleanup even if an exception occurs.

♻️ Refactor to use context manager
-    if not os.path.exists(init_file):
-        open(init_file, "w").close()
+    if not os.path.exists(init_file):
+        with open(init_file, "w"):
+            pass
🤖 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/unit_tests/test_goodmap.py` at line 141, Replace the bare
open(init_file, "w").close() call with a context-manager-based file open so the
file handle is always cleaned up; locate the spot referencing the init_file
variable in the test (in test_goodmap) and use a with-statement to open it for
writing and let the context manager close it automatically.
🤖 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 `@goodmap/config.py`:
- Line 14: The default jsDelivr URL currently set to
"https://cdn.jsdelivr.net/npm/@problematy/goodmap@1.6.1" points to a
non-existent npm release; update that default to an existing published version
(for example replace "@1.6.1" with "@1.5.0") or otherwise revert to a lower
published tag, or add a comment and fallback logic to wait for the front-end
package to be published; locate the string
"https://cdn.jsdelivr.net/npm/@problematy/goodmap@1.6.1" in the config and
change it accordingly.

In `@pyproject.toml`:
- Line 22: The dependency entry platzky in pyproject.toml is pinned to an alpha
release (2.0.0a0); change that version to a stable release (e.g., the latest 1.x
stable or the official 2.0.0 if released) by editing the platzky version string
in pyproject.toml, then regenerate your lockfile (poetry lock / pip-tools) and
run the test suite; also check platzky's changelog/compat notes for breaking
changes before moving to 2.0.0.

---

Nitpick comments:
In `@tests/unit_tests/test_goodmap.py`:
- Line 275: The test currently creates an empty file by calling
open(os.path.join(static_dir, "test.js"), "w").close() without a context
manager; change this to use a with-context to open the same path (constructed
from static_dir and "test.js") for writing so the file is closed automatically
when the block exits, ensuring proper resource handling in the test_goodmap
test.
- Line 141: Replace the bare open(init_file, "w").close() call with a
context-manager-based file open so the file handle is always cleaned up; locate
the spot referencing the init_file variable in the test (in test_goodmap) and
use a with-statement to open it for writing and let the context manager close it
automatically.
🪄 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: 5e7379ce-0254-430c-a642-d200be2df96a

📥 Commits

Reviewing files that changed from the base of the PR and between bfbcae0 and 21fafa7.

⛔ Files ignored due to path filters (1)
  • poetry.lock is excluded by !**/*.lock
📒 Files selected for processing (17)
  • docs/conf.py
  • docs/index.rst
  • docs/plugins.rst
  • docs/quickstart.rst
  • goodmap/config.py
  • goodmap/core_api.py
  • goodmap/data_models/location.py
  • goodmap/formatter.py
  • goodmap/goodmap.py
  • goodmap/templates/map.html
  • pyproject.toml
  • tests/unit_tests/data_models/test_location.py
  • tests/unit_tests/test_config.py
  • tests/unit_tests/test_core_api.py
  • tests/unit_tests/test_db.py
  • tests/unit_tests/test_formatter.py
  • tests/unit_tests/test_goodmap.py

Comment thread goodmap/config.py
Comment thread pyproject.toml
aiohttp = "^3.8.4"
pydantic = "^2.12.0"
platzky = "^1.4.10"
platzky = "2.0.0a0"

@coderabbitai coderabbitai Bot May 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Consider using a stable release instead of an alpha version.

The platzky dependency has been updated to 2.0.0a0, which is a pre-release/alpha version. Alpha versions may contain:

  • Unstable APIs subject to breaking changes
  • Undiscovered bugs
  • Missing features or documentation

Unless this PR specifically requires alpha features, consider waiting for a stable 2.0.0 release or using the latest stable 1.x version.

🤖 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 `@pyproject.toml` at line 22, The dependency entry platzky in pyproject.toml is
pinned to an alpha release (2.0.0a0); change that version to a stable release
(e.g., the latest 1.x stable or the official 2.0.0 if released) by editing the
platzky version string in pyproject.toml, then regenerate your lockfile (poetry
lock / pip-tools) and run the test suite; also check platzky's changelog/compat
notes for breaking changes before moving to 2.0.0.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes we reqire this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@raven-wing, understood! If platzky 2.0.0a0 is required for the new plugin features in this PR, that makes sense. Just be mindful that the alpha version may receive breaking API changes before the final 2.0.0 stable release, so you may need to update the pin (or loosen it) once a stable version ships.


✏️ Learnings added
Learnt from: raven-wing
Repo: Problematy/goodmap PR: 351
File: pyproject.toml:22-22
Timestamp: 2026-05-13T14:55:48.782Z
Learning: In the goodmap repository (pyproject.toml), the platzky dependency is intentionally pinned to the alpha pre-release version `2.0.0a0` because the platzky plugin integration features in the `better_plugins` branch require this specific version. Do not flag this as an issue.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@raven-wing
raven-wing merged commit e666c3e into Problematy:main May 13, 2026
11 checks passed
@raven-wing
raven-wing deleted the better_plugins branch May 13, 2026 14:56
@coderabbitai coderabbitai Bot mentioned this pull request Aug 3, 2026
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