Skip to content

Commit 4a44980

Browse files
Merge pull request #38 from MarcSkovMadsen/fix/holoviz-configuration
Fix/holoviz configuration
2 parents cac012f + 8ab87af commit 4a44980

8 files changed

Lines changed: 212 additions & 67 deletions

File tree

.github/prompts/holoviz.prompt.md

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
# HoloViz Development Guidelines
2+
3+
## Overview
4+
5+
Use the HoloViz ecosystem including Panel, Param, and hvPlot for building interactive data applications following intermediate to expert patterns.
6+
7+
## Panel Application Development
8+
9+
### Core Architecture Principles
10+
11+
**Parameter-Driven Design**
12+
13+
- Create applications as `param.Parameterized` or `pn.Viewable` classes
14+
- Let Parameters drive application state, not widgets directly
15+
- Structure code so user interactions can be tested using Parameterized classes
16+
- Use a source `data` parameter to drive your app - structure code so app state resets when source data changes
17+
18+
**Widget and Display Patterns**
19+
20+
- Create widgets from parameters: `pn.widgets.Select.from_param(state.param.parameter_name, ...)`
21+
- Display Parameter objects in panes: `pn.pane.HoloViews(state.param.parameter_name, ...)`
22+
- Prefer `pn.bind` or `@param.depends` without `watch=True` for reactive updates
23+
- Use `.on_click` for Button interactions over `watch=True` patterns
24+
- Avoid `pn.bind` or `@pn.depends` with `watch=True`, `.watch`, or `.link` methods as they make apps harder to reason about
25+
26+
**other**
27+
28+
- prefer using `.servable()` over `.show()` for serving applications
29+
- use `pn.state.served:` to check if the app is being served instead of `if __name__ == "__main__"
30+
31+
### Code Organization
32+
33+
**Module Structure**
34+
35+
- Put data extractions and transformations in `data.py` - keep clean and reusable without HoloViz dependencies
36+
- Put plot functions in `plots.py` - keep clean and reusable without Panel code
37+
- Separate business logic from UI concerns
38+
39+
**Component Selection**
40+
41+
- Use `panel-graphic-walker` package for Tableau-like data exploration components
42+
- Use `panel-material-ui` components for new projects or projects already using this package
43+
- Continue using standard Panel components in existing projects that already use them
44+
45+
### Testing Strategy
46+
47+
**Testable Architecture**
48+
49+
- Structure code so user interactions can be tested through Parameterized classes
50+
- Separate UI logic from business logic to enable unit testing
51+
- Use parameter watchers and dependencies for reactive behavior that can be tested
52+
53+
## Best Practices
54+
55+
### Reactive Programming
56+
57+
- Prefer declarative reactive patterns over imperative event handling
58+
- Use `@param.depends` decorators to create reactive methods
59+
- Leverage parameter watchers for automatic state management
60+
61+
### Performance Considerations
62+
63+
- Use `sizing_mode="stretch_width"` for responsive layouts
64+
- Avoid unnecessary parameter watchers that could cause performance issues
65+
- Structure data flows to minimize redundant computations
66+
67+
### Error Handling
68+
69+
- Implement graceful handling of missing or invalid data
70+
- Provide meaningful feedback to users when operations fail
71+
- Use safe data access patterns for robust applications
72+
73+
## Example Patterns
74+
75+
### Parameter-Driven Widget Creation
76+
```python
77+
# Good: Widget driven by parameter
78+
select_widget = pn.widgets.Select.from_param(
79+
self.param.model_type,
80+
name="Model Type"
81+
)
82+
83+
# Avoid: Manual widget management
84+
select_widget = pn.widgets.Select(
85+
options=['Option1', 'Option2'],
86+
value='Option1'
87+
)
88+
```
89+
90+
### Reactive Display Updates
91+
92+
```python
93+
# Best: Depends functions and methods
94+
@param.depends('model_results')
95+
def create_plot(self):
96+
return create_performance_plot(self.model_results)
97+
98+
plot_pane = pn.pane.Matplotlib(
99+
self.create_plot
100+
)
101+
102+
# Good: Bound functions and methods
103+
def create_plot(self):
104+
return create_performance_plot(self.model_results)
105+
106+
plot_pane = pn.pane.Matplotlib(
107+
pn.bind(self.create_plot)
108+
)
109+
110+
# Avoid: Manual updates with watchers
111+
def update_plot(self):
112+
self.plot_pane.object = create_performance_plot(self.model_results)
113+
114+
self.param.watch(self.update_plot, 'model_results')
115+
```
116+
117+
### Data-Driven Architecture
118+
119+
```python
120+
class DataApp(param.Parameterized):
121+
data = param.DataFrame(default=pd.DataFrame())
122+
123+
@param.depends('data', watch=True)
124+
def _reset_app_state(self):
125+
"""Reset all app state when source data changes."""
126+
# Reset or update parameters but not widgets directly
127+
...
128+
129+
@param.depends('data')
130+
def _get_xyz(self):
131+
"""Return some transformed object like a DataFrame or a Plot/ Figure."""
132+
# Keep this method short by using imported method from data or plots module
133+
...
134+
```
135+
136+
## Resources
137+
138+
- [Panel Intermediate Tutorials](https://panel.holoviz.org/tutorials/intermediate/index.html)
139+
- [Panel Expert Tutorials](https://panel.holoviz.org/tutorials/expert/index.html)
140+
- [Param Documentation](https://param.holoviz.org/)
141+
- [Panel Material UI Components](https://panel-material-ui.holoviz.org/)
142+
- [Panel Graphic Walker](https://panel-graphic-walker.holoviz.org/)
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
When fixing ruff issues, please follow these guidelines below.
2+
3+
Please fix one issue at a time and test the change before progressing to next issue.
4+
Please fix ruff issues in the 'src' and 'tests' folders only.
5+
Please fix easy to fix issues before more complex ones.
6+
For complex ones feel free to ask the user for guidance or help.
7+
8+
## Specific Rules
9+
10+
### D405
11+
12+
Please ensure the docstring change is meaningful to end users and Google style.
13+
If the summary extends to multiple lines please reformulate instead of breaking the summary into two distinct lines.
14+
15+
### ARG001
16+
17+
If the argument is an unused pytest fixture please change it to a marker, e.g. to `@pytest.mark.usefixtures("name_of_fixture")`.

src/holoviz_mcp/apps/configuration_viewer.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ def __init__(self, **params):
4141
Loads environment config and config loader.
4242
"""
4343
super().__init__(**params)
44-
self._env_config = HoloVizMCPConfig.from_environment()
44+
self._env_config = HoloVizMCPConfig()
4545
self._loader = get_config_loader()
4646

4747
@param.depends("config_source")

src/holoviz_mcp/apps/search.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ class SearchApp(pn.viewable.Viewer):
258258
- Integration with HoloViz MCP docs_search tool
259259
"""
260260

261+
title = param.String(default="HoloViz MCP Search Tool", doc="Title of the search app")
261262
config = param.ClassSelector(class_=SearchConfiguration, doc="Configuration for the search app")
262263

263264
def __init__(self, **params):
@@ -297,7 +298,7 @@ def __panel__(self):
297298
github_button.js_on_click(code=js_code_to_open_holoviz_mcp)
298299

299300
return pmui.Page(
300-
title="HoloViz MCP Search Tool",
301+
title=self.title,
301302
site_url="./",
302303
sidebar=[self._config, menu],
303304
sidebar_width=400,

src/holoviz_mcp/config/loader.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def load_config(self) -> HoloVizMCPConfig:
5252
if self._env_config is not None:
5353
env_config = self._env_config
5454
else:
55-
env_config = HoloVizMCPConfig.from_environment()
55+
env_config = HoloVizMCPConfig()
5656

5757
# Start with default configuration dict
5858
config_dict = self._get_default_config()

src/holoviz_mcp/config/models.py

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@
1616
from pydantic import field_validator
1717

1818

19+
def _holoviz_mcp_user_dir() -> Path:
20+
"""Get the default user directory for HoloViz MCP."""
21+
return Path(os.environ.get("HOLOVIZ_MCP_USER_DIR", Path.home() / ".holoviz-mcp"))
22+
23+
24+
def _holoviz_mcp_default_dir() -> Path:
25+
"""Get the default configuration directory for HoloViz MCP."""
26+
return Path(os.environ.get("HOLOVIZ_MCP_DEFAULT_DIR", Path(__file__).parent.parent / "config"))
27+
28+
1929
class FolderConfig(BaseModel):
2030
"""Configuration for a folder within a repository."""
2131

@@ -144,21 +154,9 @@ class ServerConfig(BaseModel):
144154
anonymized_telemetry: bool = Field(default=False, description="Enable anonymized telemetry")
145155
jupyter_server_proxy_url: str = Field(default="", description="Jupyter server proxy URL for Panel app integration")
146156
security: SecurityConfig = Field(default_factory=SecurityConfig, description="Security configuration")
147-
vector_db_path: Path = Field(default_factory=lambda: Path.home() / ".holoviz-mcp" / "vector_db" / "chroma", description="Path to the Chroma vector database.")
148-
149-
def resolve_vector_db_path(self, user_dir: Path) -> Path:
150-
"""Resolve the path to the Chroma vector database.
151-
152-
This method ensures the vector_db_path is properly expanded.
153-
154-
Args:
155-
user_dir: User directory (for compatibility, but not used since vector_db_path is now always set)
156-
157-
Returns
158-
-------
159-
Resolved path to the vector database
160-
"""
161-
return Path(self.vector_db_path).expanduser()
157+
vector_db_path: Path = Field(
158+
default_factory=lambda: (_holoviz_mcp_user_dir() / "vector_db" / "chroma").expanduser(), description="Path to the Chroma vector database."
159+
)
162160

163161

164162
class HoloVizMCPConfig(BaseModel):
@@ -170,29 +168,12 @@ class HoloVizMCPConfig(BaseModel):
170168
prompts: PromptConfig = Field(default_factory=PromptConfig)
171169

172170
# Environment paths - merged from EnvironmentConfig with defaults
173-
user_dir: Path = Field(default_factory=lambda: Path.home() / ".holoviz-mcp", description="User configuration directory")
174-
default_dir: Path = Field(default_factory=lambda: Path(__file__).parent.parent / "config", description="Default configuration directory")
175-
repos_dir: Path = Field(default_factory=lambda: Path.home() / ".holoviz-mcp" / "repos", description="Repository download directory")
171+
user_dir: Path = Field(default_factory=_holoviz_mcp_user_dir, description="User configuration directory")
172+
default_dir: Path = Field(default_factory=_holoviz_mcp_default_dir, description="Default configuration directory")
173+
repos_dir: Path = Field(default_factory=lambda: _holoviz_mcp_user_dir() / "repos", description="Repository download directory")
176174

177175
model_config = ConfigDict(extra="forbid", validate_assignment=True)
178176

179-
@classmethod
180-
def from_environment(cls) -> HoloVizMCPConfig:
181-
"""Create configuration from environment variables with defaults."""
182-
user_dir = Path(os.environ.get("HOLOVIZ_MCP_USER_DIR", Path.home() / ".holoviz-mcp"))
183-
default_dir = Path(os.environ.get("HOLOVIZ_MCP_DEFAULT_DIR", Path(__file__).parent.parent / "config"))
184-
repos_dir = Path(os.environ.get("HOLOVIZ_MCP_REPOS_DIR", user_dir / "repos"))
185-
186-
return cls(
187-
server=ServerConfig(),
188-
docs=DocsConfig(),
189-
resources=ResourceConfig(),
190-
prompts=PromptConfig(),
191-
user_dir=user_dir,
192-
default_dir=default_dir,
193-
repos_dir=repos_dir,
194-
)
195-
196177
def config_file_path(self, location: Literal["user", "default"] = "user") -> Path:
197178
"""Get the path to the configuration file.
198179

src/holoviz_mcp/docs_mcp/data.py

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -204,12 +204,13 @@ def convert_path_to_url(path: Path, remove_first_part: bool = True, url_transfor
204204
class DocumentationIndexer:
205205
"""Handles cloning, processing, and indexing of documentation."""
206206

207-
def __init__(self, data_dir: Optional[Path] = None, repos_dir: Optional[Path] = None):
207+
def __init__(self, *, data_dir: Optional[Path] = None, repos_dir: Optional[Path] = None, vector_dir: Optional[Path] = None):
208208
"""Initialize the DocumentationIndexer.
209209
210210
Args:
211211
data_dir: Directory to store index data. Defaults to user config directory.
212212
repos_dir: Directory to store cloned repositories. Defaults to HOLOVIZ_MCP_REPOS_DIR.
213+
vector_dir: Directory to store vector database. Defaults to config.vector_dir
213214
"""
214215
# Use unified config for default paths
215216
config = get_config()
@@ -221,9 +222,8 @@ def __init__(self, data_dir: Optional[Path] = None, repos_dir: Optional[Path] =
221222
self.repos_dir = repos_dir or config.repos_dir
222223
self.repos_dir.mkdir(parents=True, exist_ok=True)
223224

224-
# Use config logic to resolve vector DB path
225-
config = get_config()
226-
vector_db_path = config.server.resolve_vector_db_path(config.user_dir)
225+
# Use configurable directory for vector database path
226+
vector_db_path = vector_dir or config.server.vector_db_path
227227
vector_db_path.parent.mkdir(parents=True, exist_ok=True)
228228

229229
# Disable ChromaDB telemetry based on config
@@ -577,7 +577,6 @@ async def extract_docs_from_repo(self, repo_path: Path, project: str, ctx: Conte
577577
regular_count = len(docs) - reference_count
578578

579579
await log_info(f" 📄 {project}: {len(docs)} total documents ({regular_count} regular, {reference_count} reference guides)", ctx)
580-
581580
return docs
582581

583582
async def index_documentation(self, ctx: Context | None = None):
@@ -591,7 +590,6 @@ async def index_documentation(self, ctx: Context | None = None):
591590
# Clone/update repositories and extract documentation
592591
for repo_name, repo_config in self.config.repositories.items():
593592
await log_info(f"Processing {repo_name}...", ctx)
594-
595593
repo_path = await self.clone_or_update_repo(repo_name, repo_config)
596594
if repo_path:
597595
docs = await self.extract_docs_from_repo(repo_path, repo_name, ctx)
@@ -934,34 +932,32 @@ async def _log_summary_table(self, ctx: Context | None = None):
934932
except Exception as e:
935933
await log_warning(f"Failed to generate summary table: {e}", ctx)
936934

935+
def run(self):
936+
"""Update the DocumentationIndexer."""
937+
# Configure logging for the CLI
938+
logging.basicConfig(level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s", handlers=[logging.StreamHandler()])
937939

938-
def main():
939-
"""Update the DocumentationIndexer."""
940-
# Configure logging for the CLI
941-
logging.basicConfig(level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s", handlers=[logging.StreamHandler()])
942-
943-
logger.info("🚀 HoloViz MCP Documentation Indexer")
944-
logger.info("=" * 50)
940+
logger.info("🚀 HoloViz MCP Documentation Indexer")
941+
logger.info("=" * 50)
945942

946-
async def run_indexer():
947-
indexer = DocumentationIndexer()
948-
logger.info(f"📁 Repository directory: {indexer.repos_dir}")
949-
logger.info(f"💾 Vector database: {indexer.data_dir / 'chroma'}")
950-
logger.info(f"🔧 Configured repositories: {len(indexer.config.repositories)}")
951-
logger.info("")
943+
async def run_indexer(indexer=self):
944+
logger.info(f"📁 Repository directory: {indexer.repos_dir}")
945+
logger.info(f"💾 Vector database: {indexer.data_dir / 'chroma'}")
946+
logger.info(f"🔧 Configured repositories: {len(indexer.config.repositories)}")
947+
logger.info("")
952948

953-
await indexer.index_documentation()
949+
await indexer.index_documentation()
954950

955-
# Final summary
956-
count = indexer.collection.count()
957-
logger.info("")
958-
logger.info("=" * 50)
959-
logger.info("✅ Indexing completed successfully!")
960-
logger.info(f"📊 Total documents in database: {count}")
961-
logger.info("=" * 50)
951+
# Final summary
952+
count = indexer.collection.count()
953+
logger.info("")
954+
logger.info("=" * 50)
955+
logger.info("✅ Indexing completed successfully!")
956+
logger.info(f"📊 Total documents in database: {count}")
957+
logger.info("=" * 50)
962958

963-
asyncio.run(run_indexer())
959+
asyncio.run(run_indexer())
964960

965961

966962
if __name__ == "__main__":
967-
main()
963+
DocumentationIndexer().run()

tests/config/test_models.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,3 +181,11 @@ def test_config_forbids_extra_fields(self):
181181
# We need to construct this in a way that bypasses type checking
182182
config_dict = {"server": {"name": "test"}, "docs": {"repositories": {}}, "resources": {}, "prompts": {}, "extra_field": "not allowed"}
183183
HoloVizMCPConfig(**config_dict)
184+
185+
def test_with_environment_variables(self, monkeypatch):
186+
"""Test configuration with environment variables."""
187+
monkeypatch.setenv("HOLOVIZ_MCP_USER_DIR", "/test/dir")
188+
config = HoloVizMCPConfig()
189+
assert config.user_dir == Path("/test/dir")
190+
assert config.server.vector_db_path == Path("/test/dir") / "vector_db" / "chroma"
191+
assert config.repos_dir == Path("/test/dir") / "repos"

0 commit comments

Comments
 (0)