Fix: src layout without package by volfpeter · Pull Request #42 · volfpeter/holm

volfpeter · 2025-12-22T22:48:45Z

Fixes #41

Summary by CodeRabbit

New Features
- Better support for non-packaged Python applications (improved module import behavior).
Bug Fixes
- More tolerant import handling with graceful failure logging.
Chores
- Package version bumped to 0.7.2.
- New configuration default to disable automatic issue enrichment.
Documentation
- Examples and guides updated to use a unified, generic component return type.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-22T22:48:54Z

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a CodeRabbit config, bumps package version to 0.7.2, generalizes many examples/docs to use ComponentType for component return/parameter annotations, and makes import resolution in holm/_model.py tolerant of non-packaged app roots and import failures (logs and returns None).

Changes

Cohort / File(s)	Summary
Configuration `\.coderabbit\.yaml`	Adds `issue_enrichment.auto_enrich.enabled: false`.
Version `holm/__init__.py`	Bumps `__version__` from `"0.7.1"` to `"0.7.2"`.
Import resolution & error handling `holm/_model.py`	Introduces `_no_app_package_roots = {"", "."}`; uses membership test when resolving import names for non-packaged apps; catches import exceptions, logs a warning with traceback, and returns `None` instead of raising.
Docs / Guides `docs/guides/...`	Replaces concrete `htmy.html.*` return types with `ComponentType` and adds/imports `ComponentType` where needed across guides (`forms.md`, `path-parameters.md`, `in-a-hurry.md`).
Examples `examples/forms/*`, `examples/path-parameters/...`	Updates example component type hints from `html.*` to `ComponentType`; updates imports to include `ComponentType`.
Tests / Test app example `test_app/user/_user_id_/api.py`	Adds `ComponentType` to imports and changes `user_card` return annotation from `html.div` to `ComponentType`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Possibly related PRs

Fix: src layout without package #42 — Overlaps changes in holm/_model.py (non-packaged/src layout handling) and the .coderabbit.yaml change.
Docs: path parameters / dynamic routes guide #15 — Touches path-parameters guides and example files; related to ComponentType return-type updates.
Bump patch version, prepare new release #39 — Also bumps __version__ in holm/__init__.py; related to the version change.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	Several changes appear out of scope: version bump to 0.7.2, multiple unrelated return type updates from concrete HTML types to ComponentType across examples and documentation, and a new .coderabbit.yaml configuration file.	Remove the version bump, revert all ComponentType return type changes in examples/docs (keep only the core _model.py fix), and exclude the .coderabbit.yaml configuration to stay focused on the src layout issue.
Docstring Coverage	⚠️ Warning	Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix: src layout without package' directly relates to the main objective of supporting src layout projects, especially those without a Python package wrapper, which aligns with issue #41.
Linked Issues check	✅ Passed	The code changes successfully address issue #41 by modifying the import resolution logic to handle non-packaged applications using src layout through the new _no_app_package_roots constant and enhanced error handling.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 94ebc90 and 4df765b.

📒 Files selected for processing (1)

holm/_model.py

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

Actionable comments posted: 0

🧹 Nitpick comments (1)

holm/_model.py (1)
142-159: Update or remove the outdated comment.

The comment at Lines 142-144 states that catching import exceptions is "undesired because it would be hard for users to find why an API is not registered." However, the new code at Lines 152-159 now catches all exceptions and logs a warning instead of propagating them.

This creates a documentation inconsistency. Consider either:

Updating the comment to explain the trade-off and why the new approach is preferred for src layout support

Making the exception handling more specific (e.g., only catch ImportError or ModuleNotFoundError related to configuration issues)

The current approach of catching all exceptions (including SyntaxError, AttributeError, etc.) and logging warnings could hide legitimate bugs in user code. While the full traceback is logged, warnings are easier to miss than exceptions during development.
🔎 Suggested comment update
-        # Check if the module exists. Trying to import it and catching the exception would hide possible
-        # import error that occur in within the module, which is undesired because it would be hard for
-        # users to find why an API is not registered.
+        # Check if the module file exists before attempting import.
         if not (self.package_dir / f"{name}.py").is_file():
             return None
 
         # Support applications that are not wrapped in a Python package.
         import_name = name if self.package_name in _no_app_package_roots else f"{self.package_name}.{name}"
         try:
             module = import_module(import_name)
         except Exception:
             import traceback
 
-            # Handle potential misconfigurations with a simple warning, instead of an exception.
+            # For src/flat layouts, import path misconfigurations are possible. Log the full
+            # traceback as a warning rather than crashing, allowing the application to continue
+            # with other modules. Users should monitor logs for these warnings during development.
             logger = logging.getLogger("holm")
             logger.warning(f"Failed to import module {name} at: {import_name}")
             logger.warning(traceback.format_exc())
             return None

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f4bd73 and edaf294.

📒 Files selected for processing (3)

.coderabbit.yaml
holm/__init__.py
holm/_model.py

🔇 Additional comments (4)

.coderabbit.yaml (1)

1-3: LGTM!

The configuration correctly disables automatic issue enrichment per the schema. This is a valid preference setting with no impact on the codebase functionality.

holm/__init__.py (1)

1-1: LGTM!

Appropriate version bump to reflect the src layout support feature.

holm/_model.py (2)

149-149: LGTM!

The change from equality check to membership test correctly extends support to both "" (src layout) and "." (flat layout), aligning with the PR objectives.

88-94: Constant definition looks good, but layout mapping needs confirmation.

The flat layout organizes configuration and import packages in the top-level directory, while the src layout moves importable code into a subdirectory typically named src/. The constant uses a set for efficient membership testing, which is appropriate.

However, the docstring's mapping of "" to src layout and "." to flat layout should be verified against the actual import_name values that Flask/Werkzeug assigns in these scenarios, as this specific detail is not documented in standard Python packaging guidance.

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between edaf294 and 94ebc90.

📒 Files selected for processing (7)

docs/guides/forms.md
docs/guides/path-parameters.md
docs/in-a-hurry.md
examples/forms/layout.py
examples/forms/page.py
examples/path-parameters/user/_id_/page.py
test_app/user/_user_id_/api.py

🧰 Additional context used

🧬 Code graph analysis (2)

test_app/user/_user_id_/api.py (1)

test_app/user/_user_id_/page.py (1)

htmy (19-24)

examples/path-parameters/user/_id_/page.py (3)

examples/forms/page.py (1)

page (70-72)

examples/path-parameters/user/page.py (1)

page (8-19)

examples/path-parameters/page.py (1)

page (7-17)

🔇 Additional comments (7)

examples/forms/page.py (1)

4-4: Import addition looks good.

ComponentType is imported to support the generalized return type annotation.

docs/guides/path-parameters.md (1)

206-206: Documentation accurately reflects the ComponentType refactoring.

The updated import and return type annotation in the documentation example align with the actual code changes in examples/path-parameters/user/_id_/page.py.

Also applies to: 238-238

examples/forms/layout.py (1)

8-8: Public API return types appropriately generalized.

Both head() and search_form() return types have been updated to ComponentType, making these helper components more flexible. The implementations still return concrete html elements (html.head and html.form), which should be compatible with the generic type.

Also applies to: 28-28

test_app/user/_user_id_/api.py (1)

3-3: Test app updated consistently with examples.

The user_card function return type has been generalized to ComponentType, matching the pattern applied across examples and maintaining consistency in the codebase.

Also applies to: 25-25

examples/path-parameters/user/_id_/page.py (1)

4-4: Example updated with generalized return type.

The page() function now returns ComponentType instead of a specific HTML element type, aligning with the broader refactoring effort across the codebase.

Also applies to: 36-36

docs/guides/forms.md (1)

109-109: Documentation examples updated to reflect ComponentType refactoring.

All function signatures in the guide now use ComponentType for return types, accurately reflecting the changes made to the corresponding example code in examples/forms/.

Also applies to: 133-133, 203-214

docs/in-a-hurry.md (1)

47-61: Introduction guide updated with ComponentType pattern.

The documentation examples now consistently demonstrate using ComponentType for function parameters and return types, providing clear guidance for users on the recommended approach for building holm applications.

Also applies to: 72-100

enabled by default

coderabbitai Bot reviewed Dec 22, 2025

View reviewed changes

volfpeter merged commit 9883cce into main Dec 22, 2025
2 checks passed

volfpeter deleted the fix/src-layout-without-package branch December 22, 2025 23:00

coderabbitai Bot reviewed Dec 22, 2025

View reviewed changes

Comment thread examples/forms/page.py

volfpeter added 5 commits December 23, 2025 00:33

fix: support src layouts where the app root is not in a package, fixes

7e3aaa8

chore: don't create a set every time, add docs

c354728

chore: disable coderabbit.ai issue enrichment, it's super annoying to be

edaf294

enabled by default

chore: update examples and docs to work with htmy v0.10+

94ebc90

docs: fix comment

4df765b

This was referenced Dec 27, 2025

Add support for src layout with editable install and run script #44

Merged

Support application layouts where the holm app is in a subpackage #45

Merged

coderabbitai Bot mentioned this pull request Jan 18, 2026

Add support for HTML layouts with Snippet, Slots, and a default text processor #53

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix: src layout without package#42

Fix: src layout without package#42
volfpeter merged 5 commits into
mainfrom
fix/src-layout-without-package

volfpeter commented Dec 22, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Dec 22, 2025 •

edited

Loading

Review failed

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

volfpeter commented Dec 22, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Pre-merge checks and finishing touches

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

volfpeter commented Dec 22, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Dec 22, 2025 •

edited

Loading