Expose dashboard format controls#118
Conversation
rajkumarsakthivel
left a comment
rajkumarsakthivel
left a comment
There was a problem hiding this comment.
Request changes — 3 important issues before merge.
Important: _normalize_format_config silently discards user input
When preset is not custom, top_k/max_tokens are overwritten with preset values regardless of what the user typed. A user who picks balanced and types top_k=15 silently gets top_k=10 back — no error, no feedback.
Fix: either reject the request when values diverge from the preset, or auto-switch to custom on the server when they diverge, and return that. Silent overwriting is the worst option.
Important: Two file reads per context_search call
_handle_context_search calls self._load_state() to get the live context_top_k/context_max_tokens, then later _apply_output_compression also calls self._load_state() to pick up the live output_level. That's two synchronous state.json reads on every MCP response.
Consolidate: read state once in _handle_context_search, pass the level down to _apply_output_compression rather than re-reading inside it. Also removes the dead hasattr(self, "_state_path") guard.
Important: list_tools advertises stale defaults
list_tools returns self._default_top_k/self._default_max_tokens (startup values), but _handle_context_search uses live state. An AI assistant that reads the schema to understand defaults sees the wrong values after a dashboard change. Update the schema defaults from live state on each list_tools call, or document the discrepancy explicitly.
Minor issues (can fix in same pass)
loadFormatConfigis called insideloadSavings— wrong trigger point. Call it once in the top-level page init instead.saveFormatConfigswallows errors silently:catch(e) { toast('Failed') }. At least logeto the console._format_config_from_statedouble-fallback foroutput_levelis confusing. Simplify to one validated fallback.hasattr(req, "model_dump") else req.dict()— Pydantic v2 is already required; dead branch.- Missing test: GET
/api/formatafter a POST round-trip to verify persisted values come back correctly. - Missing test:
POST /api/compressionafterPOST /api/format(and vice versa) to confirm state keys aren't clobbered.
There was a problem hiding this comment.
Pull request overview
This PR adds dashboard controls for configuring context-search input presets (top_k/max_tokens) and output verbosity, persists those defaults to state.json via a new /api/format endpoint, and updates the MCP server to pick up the persisted defaults without requiring a restart.
Changes:
- Add
/api/formatGET/POST to read/write persisted format defaults (input preset, top_k/max_tokens, output level). - Add “Input / Output Format” controls to the dashboard page and wire them to the new API.
- Update MCP server to refresh context_search defaults from
state.jsonon each call; add tests covering the new API/UI wiring.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/dashboard/test_server.py | Adds API-level tests for /api/format defaults, preset behavior, and clamping/persistence. |
| tests/dashboard/test_dashboard_smoke.py | Extends smoke tests to validate the new format endpoint and presence of UI hooks. |
| src/context_engine/integration/mcp_server.py | Loads persisted defaults and refreshes context_search defaults from state.json at call time; adjusts tool schema defaults. |
| src/context_engine/dashboard/server.py | Implements /api/format and includes format_config in /api/status. |
| src/context_engine/dashboard/_page.py | Adds the dashboard UI panel + JS functions to load/save/reset format settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| lo=500, | ||
| hi=50000, | ||
| ) | ||
| top_k = _clamp_top_k(args.get("top_k", default_top_k)) |
| if hasattr(self, "_state_path"): | ||
| state_level = self._load_state().get("output_level") | ||
| if state_level in LEVELS: | ||
| self._output_level = state_level | ||
|
|
| settings = _normalize_format_config(req) | ||
| state = _read_state() | ||
| state["input_preset"] = settings["input_preset"] | ||
| state["context_top_k"] = settings["top_k"] | ||
| state["context_max_tokens"] = settings["max_tokens"] | ||
| state["output_level"] = settings["output_level"] | ||
| (storage_base / "state.json").write_text(json.dumps(state), encoding="utf-8") | ||
| return settings |
| async function loadFormatConfig() { | ||
| try { | ||
| var r = await fetch(API+'/api/format'); | ||
| var d = await r.json(); | ||
| document.getElementById('fmt-input-preset').value = d.input_preset || 'balanced'; | ||
| document.getElementById('fmt-top-k').value = d.top_k || 10; | ||
| document.getElementById('fmt-max-tokens').value = d.max_tokens || 8000; | ||
| document.getElementById('fmt-output-level').value = d.output_level || currentLevel || 'standard'; | ||
| } catch(e) {} | ||
| } |
| max_tokens = args.get("max_tokens", default_max_tokens) | ||
| try: | ||
| max_tokens = int(max_tokens) | ||
| except (TypeError, ValueError): |
rajkumarsakthivel
left a comment
rajkumarsakthivel
left a comment
There was a problem hiding this comment.
Good feature, clean design. Two issues preventing merge.
Blocking
1. POST /api/format uses non-atomic write — regression
set_format_config does:
(storage_base / "state.json").write_text(json.dumps(state), encoding="utf-8")set_compression was just fixed in #123 to use atomic_write_text for exactly this reason (MCP server reads the same file, race causes corruption). The new endpoint needs the same treatment.
2. CI failure — _state_path missing in test mocks
_handle_context_search now calls self._load_state() unconditionally to read context_top_k/context_max_tokens. _load_state accesses self._state_path internally. The test fixtures added in #123 (test_context_search_appends_omitted_note_when_drops_reported, test_context_search_no_note_when_nothing_dropped) construct ContextEngineMCP without going through __init__, so _state_path is absent and the call blows up.
The guard in _apply_output_compression:
if hasattr(self, "_state_path"):…does not help because _load_state itself accesses the attribute. The same guard (or a try/except) needs to wrap the _load_state() call in _handle_context_search.
Minor
The hasattr(self, "_state_path") guard in _apply_output_compression is a smell — if a real ContextEngineMCP ever lacks _state_path that's a deeper bug. Once the CI fix is in place it can be removed.
loadFormatConfig() is called inside loadSavings(), coupling format config loading to the savings refresh timer. Not wrong, but it means the format panel re-reads from the server on every stats poll. A one-time load on DOMContentLoaded or on panel open would be cleaner.
|
Superseded by #133, which includes all the same changes with three fixes: atomic write in |
Summary
Fixes #100
Tests