feat(gui): registry-based page registration with frame injection

[olivermeyer]

The current implementation requires services to define a GUINamespace singleton which is referenced when adding a page (e.g. @gui.authenticated). The GUINamespace then extends all pages to use the same frame_func for consistent layout. Without a utils module, there is no natural place to define this singleton. This PR removes the need for it by implementing a page registry; registering a page such as @page_authenticated simply adds the page to the registry; services must then make an additional call to gui_register_pages() with an optional frame func to apply that func to the pages.

This relies on the same auto-discovery mechanism (locate_subclasses) as other components (API, CLI). See the ADR for more details.

[codecov]

Codecov Report

❌ Patch coverage is 90.16393% with 6 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/aignostics_foundry_core/gui/auth.py	89.47%	6 Missing ⚠️

Files with missing lines	Coverage Δ
src/aignostics_foundry_core/gui/__init__.py	100.00% <ø> (ø)
src/aignostics_foundry_core/gui/core.py	98.00% <100.00%> (+0.08%)	⬆️
src/aignostics_foundry_core/gui/auth.py	66.17% <89.47%> (-6.33%)	⬇️

[claude]

Claude finished @olivermeyer's task in 4m 38s —— View job

---

Code Review Complete ✅

I've completed a thorough review of PR #39. The implementation is well-designed and follows the ADR, but there are blocking issues that must be addressed before merge.

---

🚨 Blocking Issues

1. SonarQube S1192 Violation - String Duplication in Docstrings ⚠️

Location: src/aignostics_foundry_core/gui/auth.py:411-556

Issue: Multiple docstring fragments are duplicated exactly 5 times across the page_public, page_authenticated, page_admin, page_internal, and page_internal_admin functions. This violates SonarQube rule S1192 which requires strings duplicated 3+ times to be extracted as constants.

Duplicated strings (each appears 5 times):

• "The route is NOT registered with NiceGUI immediately. Call"
• "A decorator that records the page function in the registry and returns"
• "it unchanged (sync or async). The function will be awaited automatically"
• "when the page renders if it is a coroutine function."

Why this is blocking: Per CLAUDE.md:

S1192 (String Duplication): Extract repeated string literals (duplicated 3+ times) into module-level constants. This applies to test files as well - tests are production code and must meet the same quality standards.

Fix: Extract the repeated docstring fragments into module-level constants at the top of auth.py, after the existing constants like MSG_403_FORBIDDEN. For example:

# Docstring fragments for registry decorators
_DOCSTRING_DEFERRED_REGISTRATION = (
    "The route is NOT registered with NiceGUI immediately. Call "
    "``gui_register_pages(frame_func=...)`` to actualize all registered pages "
    "with the appropriate frame."
)
_DOCSTRING_DECORATOR_BEHAVIOR = (
    "A decorator that records the page function in the registry and returns "
    "it unchanged (sync or async). The function will be awaited automatically "
    "when the page renders if it is a coroutine function."
)

Then use f-strings in the docstrings:

def page_public(path: str, title: str = "") -> Callable[..., Any]:
    f"""Decorator that registers a public page in the global registry.

    {_DOCSTRING_DEFERRED_REGISTRATION}

    Args:
        path: The URL path for the page.
        title: The title passed to the frame function when the page is actualized.

    Returns:
        {_DOCSTRING_DECORATOR_BEHAVIOR}
    ...

Fix this →

---

2. Missing Test Coverage - 6 Lines Uncovered

Location: src/aignostics_foundry_core/gui/auth.py

Issue: CodeCov reports 90.16% patch coverage with 6 lines missing coverage in auth.py. The target is at least 85%, so while technically passing, we should aim for 100% coverage per CLAUDE.md.

Investigation needed:

• Identify which 6 lines are missing coverage (CodeCov link in PR shows details)
• Most likely candidates: error branches in _actualize_admin, _actualize_internal, _actualize_internal_admin where 403 Forbidden is rendered

Suggested tests to add:

1. Test that page_admin shows 403 when user is authenticated but not admin
2. Test that page_internal shows 403 when user is authenticated but not internal org
3. Test that page_internal_admin shows 403 for all failure cases (non-internal, non-admin, both)

Add coverage tests →

---

💡 Suggestions (Non-Blocking)

3. Test Helper Method Could Reduce Duplication

Location: tests/aignostics_foundry_core/gui/gui_test.py:666-719

Observation: The pattern page_recorder.assert_not_called() appears 5 times in the TestPageRegistryDecorators class, and four test methods have very similar structure testing that different decorators don't register immediately.

Suggestion: Consider extracting a parameterized test or helper method:

@pytest.mark.parametrize("decorator_name,decorator_func", [
    ("page_public", page_public),
    ("page_authenticated", page_authenticated),
    ("page_admin", page_admin),
    ("page_internal", page_internal),
    ("page_internal_admin", page_internal_admin),
])
def test_registry_decorator_defers_registration(
    self, decorator_name: str, decorator_func: Callable
) -> None:
    """Registry decorators do NOT call ui.page immediately; route deferred until gui_register_pages."""
    nicegui_mock, page_recorder = _make_nicegui_mock()
    
    with patch.dict(sys.modules, {"nicegui": nicegui_mock}):
        decorator_func(_TEST_PATH)(lambda user: None)
    
    page_recorder.assert_not_called()

This is just a suggestion - the current approach is also valid and explicit.

---

4. Consider Adding Integration Test for Full Flow

Observation: Tests cover the registry pattern well, but there's no integration test that exercises the full flow: decorator → registry → gui_run() → actualization with frame.

Suggestion: Add an integration test (marked @pytest.mark.integration) that:

1. Uses real decorators to register multiple pages at different access levels
2. Calls gui_run() with a real frame function
3. Verifies all pages are registered with correct paths and frame injection

This would catch any integration issues between components.

---

5. Documentation: ADR Could Mention Testing Strategy

Location: docs/decisions/0005-gui-page-registration.md

Suggestion: The "Consequences → Harder / risks" section mentions that tests must call clear_page_registry(). Consider adding:

**Testing guidance**:
- Test classes that exercise page registration must include an autouse fixture
  calling `clear_page_registry()` in setup and teardown
- See `TestPageRegistryDecorators` for the reference pattern

---

✅ What's Good

1. Architecture: The registry pattern solves the singleton problem elegantly and aligns with the existing locate_subclasses pattern used elsewhere
2. Test coverage: Comprehensive unit tests for the new registry behavior, including async page functions
3. Backward compatibility: GUINamespace continues to work via _actualize_* functions
4. Documentation: ADR clearly explains the problem, alternatives, and trade-offs
5. Commit messages: Follow conventional commits format correctly
6. Test markers: All new tests properly marked with @pytest.mark.unit
7. Type safety: Proper type hints throughout, including TYPE_CHECKING imports

---

Summary

This is a well-designed feature with a clear architectural improvement. The two blocking issues are:

1. String duplication violating SonarQube S1192 (causes CI failure)
2. 6 lines missing test coverage (need tests for 403 error paths)

Once these are resolved, this PR will be ready to merge. The suggestions are optional improvements for consideration.

---

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
90.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud