docs(planning): spec + impl plan for 5 MVP2 idea-stage items#413
Conversation
Take five idea-only MVP2 folders from idea → cross-model-reviewed feature spec + implementation plan (idea-preflight → spec-gen → impl-plan-gen), each with mandatory GPT-5.5 review cycles. No source code or migrations — planning artifacts only. - chore_studies_post_arq_spy_fixture — arq_pool_spy fixture asserting no-enqueue on studies-POST rejection paths (2 stories, no migration) - bug_judgment_header_omits_click_bucket — render the click (UBI) source bucket in the judgment-list header (2 stories, frontend-only) - bug_baseline_phase_test_isolation — fix suite-ordering dependency in baseline-phase unit tests (lazy settings read + os.environ leak root cause; 2 stories, regression test) - chore_template_library_expansion — curated 6-template library + 3 per-engine tunable-params cheatsheets (8 stories, no migration) - chore_ubi_reader_search_after_pagination — exact full-traffic UBI aggregation via unified scan_all (ES/OS PIT+search_after, Solr cursorMark) (5 stories, SearchAdapter Protocol surface, no migration) Includes the dashboard regen for the 5 folders advancing to Plan stage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
There was a problem hiding this comment.
Code Review
This pull request updates the MVP2 dashboards and introduces comprehensive ideas, feature specifications, implementation plans, and pipeline statuses for several planned features, chores, and bug fixes, including baseline phase test isolation, judgment header click bucket rendering, an Arq pool spy fixture, template library expansion, and UBI reader pagination. The reviewer feedback highlights several issues in the newly added specifications and implementation plans: in the template library spec, the Solr edismax_basic.j2 template uses ps without defining pf, rendering it ineffective; in the UBI reader pagination implementation plan, the code sketch directly accesses settings and a hardcoded UBI_EVENTS_INDEX constant, which would cause NameErrors; and the _chunk helper's byte-length calculation needs to account for serialization overhead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ### FR-2: Locked template set (Solr) | ||
|
|
||
| - Requirement: | ||
| - The system **MUST** ship the following Solr templates under `samples/templates/solr/`, each rendering to a valid flat Solr request-parameter dict: |
There was a problem hiding this comment.
In Solr's edismax query parser, the ps (Phrase Slop) parameter only applies to phrase queries generated from the pf (Phrase Fields) parameter. Since pf is not listed under the baked-in literals or declared parameters for edismax_basic.j2, ps will have no effect. Consider baking in pf or adding it to the template so that ps functions correctly.
| # _chunk bounds each batch by BOTH id-count (settings.ubi_query_id_batch_size) | ||
| # AND encoded byte-length (so a Solr {!terms} fq / ES terms list stays under | ||
| # engine limits regardless of id length) — P1-B1. | ||
| for batch in _chunk(query_ids, settings.ubi_query_id_batch_size): |
There was a problem hiding this comment.
In the _scan_ubi_events implementation sketch, settings.ubi_query_id_batch_size is accessed directly. However, in UbiReader, settings are typically retrieved via get_settings() or stored on self during initialization (e.g., self._settings). Accessing settings directly without importing or resolving it first will raise a NameError. Please ensure the implementation plan specifies accessing this via self._settings or get_settings().
| # _chunk bounds each batch by BOTH id-count (settings.ubi_query_id_batch_size) | ||
| # AND encoded byte-length (so a Solr {!terms} fq / ES terms list stays under | ||
| # engine limits regardless of id length) — P1-B1. | ||
| for batch in _chunk(query_ids, settings.ubi_query_id_batch_size): |
There was a problem hiding this comment.
When implementing the _chunk helper to bound batches by encoded byte-length (Settings.ubi_query_id_batch_max_bytes), ensure the calculation accounts for the serialization overhead (such as commas or query syntax like {!terms f=query_id}) rather than just summing the raw string lengths of individual IDs. This is especially important for Solr where the entire query string is sent in the request body/URL.
| page_size=min(ES_MAX_RESULT_WINDOW, remaining), | ||
| cursor=cursor, fl=..., request_id=request_id, | ||
| ) | ||
| cursor = page.cursor # assign the LATEST cursor IMMEDIATELY after await, |
There was a problem hiding this comment.
The scan_all call in the sketch uses UBI_EVENTS_INDEX as the first argument. In UbiReader, the index name is typically retrieved dynamically from settings or stored as an instance variable (e.g., self._events_index). Using a hardcoded UBI_EVENTS_INDEX constant might lead to a NameError if it is not defined at the module level. Consider updating the sketch to use self._events_index or settings.ubi_events_index for consistency and correctness.
…ected) - template spec: bake `pf` into edismax_basic.j2 so the declared-tunable `ps` (phrase slop) is not a no-op (edismax `ps` only applies to phrase queries from `pf`) [accepted] - ubi plan: fix the _scan_ubi_events sketch's out-of-scope `settings.X` reference — UbiReader is decoupled from Settings, so ceilings are injected via __init__ (mirroring position_bias_prior), not read via get_settings() inside the reader [accepted; Gemini remedy adjusted] - ubi plan: clarify byte-length chunking measures the fully-serialized filter fragment (wrapper + separators), not summed raw id lengths [accepted] - ubi plan finding #4 (UBI_EVENTS_INDEX NameError) rejected: it is a module-level constant at ubi_reader.py:75, already used directly at L515 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
Gemini Code Assist — adjudication summaryAll 4 line-level findings adjudicated (against
These are planning artifacts (specs + plans), not code — no source changes. Merging on the docs fast lane (heavy |
Prepend the PR #413 one-liner to state.md Last-5-merges (drop PR #348 to state_history), refresh Last-updated, and move the 5 new Plan-stage pairs into the In-flight 'impl-execute-ready' list. Full narrative appended to state_history.md. Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Takes five idea-only MVP2 folders from idea → cross-model-reviewed feature spec + implementation plan (the
idea-preflight → spec-gen → impl-plan-genpipeline). Each spec and plan went through mandatory GPT-5.5 cross-model review cycles with full adjudication logs in the artifacts. No source code, no migrations — planning artifacts only. Mirrors the precedent set by PR #364 (6 spec/plan pairs landed in one docs PR).Folders advanced to Plan stage
chore_studies_post_arq_spy_fixturearq_pool_spyfixture asserting no-enqueue on studies-POST rejection pathsbug_judgment_header_omits_click_bucketclick(UBI) source bucket in the judgment-list headerbug_baseline_phase_test_isolationos.environleak)chore_template_library_expansionchore_ubi_reader_search_after_paginationscan_all(ES/OS PIT+search_after, Solr cursorMark)Notable decisions locked during planning
LifespanManageragainst CI Redis — the spy replaces it), records flattened(name, *args)tuples.get_settings()read in_compute_baseline_wait_s(only when no explicittrial_timeout_s); thetest_main_lifespan.pyos.environleak is documented but left out of scope.declared_params == search_spaceequality check. Three engines only — no Lucidworks Fusion.SearchAdapter.scan_all+close_scanreturning an opaque round-tripped cursor (ES/OS one impl with internal PIT-endpoint branch; Solr cursorMark viaPOST /select) — keeps Absolute Rule infra(foundation): bootstrap MVP1 stack — Docker + FastAPI + /healthz + Alembic + CI (#3) #4 intact.Not included
Implementation of any of these plans (each is
/impl-execute-ready as a follow-up). Two were left untouched because they are design-ahead-gated or smaller in scope.🤖 Generated with Claude Code