feat: Add Explore Data page with interactive dashboard#15
Merged
Conversation
…it codes, Redis) (#13) * fix: Correct import paths in migration scripts (d4bl.database → d4bl.infra.database) * fix: Fix remaining broken import paths in test_db_connection and add_trace_id scripts * fix: Correct import path in crew.py (d4bl.tools → d4bl.agents.tools) * fix: Return True from archive_and_wipe_db.py main() on success so exit code is 0 * fix: Log original db_host before overwriting it in Docker detection * fix: Authenticate redis-cli in healthcheck when requirepass is set * fix: Preserve sequential task order when filtering by selected_agents in crew.py * fix: Create d4bl.evals.runner with Langfuse-backed run_evals_and_log so run_evals.py imports correctly * fix: Address CodeRabbit PR #13 review issues - Fix import-time binding in test_db_connection.py by accessing async_session_maker via module reference after init_db() runs - Wrap synchronous run_comprehensive_evaluation in asyncio.to_thread to avoid blocking the event loop in evals/runner.py - Add return_exceptions=True to asyncio.gather and log per-job failures - Remove spurious f-prefix from non-interpolated print in database.py - Remove unnecessary # noqa: E402 comments from migration scripts - Move sys.path.insert and D4Bl import to module level in test file * fix: Address CodeRabbit PR #13 second-round review issues - Refactor evals/runner.py: hoist sem and _evaluate_job outside the session block, replace async-for-get_db()+break with direct async_session_maker context manager (fixes Ruff B023 closure capture) - Fix max_rows=0 treated as "no limit": use `if max_rows is not None:` - Add strict=True to zip(jobs, results) for invariant enforcement - Simplify database.py guard: replace redundant 3-part boolean with `db_host in ("localhost", "127.0.0.1")` - Break long warning print strings to stay within 100-char line limit
… interactive dashboard Implement the full /explore feature: three new API endpoints (indicators, policies, states), Pydantic schemas, Census ACS and OpenStates ingestion scripts, and a Next.js dashboard with StateMap, RacialGapChart, MetricFilterPanel, and PolicyTable components. Add CodeRabbit config for automated PR reviews.
Contributor
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. 📝 WalkthroughWalkthroughAdds an Explore Data feature: new ORM models and indexes, three /api/explore endpoints and Pydantic schemas, Next.js explore page and UI components, two ingestion scripts (Census ACS, OpenStates), import path shifts to Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExplorePage as Explore Page<br/>(Next.js)
participant API as Backend API<br/>(/api/explore/*)
participant DB as PostgreSQL<br/>Database
User->>ExplorePage: Visit /explore
activate ExplorePage
ExplorePage->>API: GET /api/explore/states
activate API
API->>DB: Aggregate metrics & bill counts
DB-->>API: StateSummaryItem[]
API-->>ExplorePage: StateSummaryItem[]
deactivate API
ExplorePage->>ExplorePage: Render map + filters
User->>ExplorePage: Select metric/race/year
ExplorePage->>API: GET /api/explore/indicators (filters)
activate API
API->>DB: Query census_indicators (filtered)
DB-->>API: IndicatorItem[]
API-->>ExplorePage: IndicatorItem[]
deactivate API
ExplorePage->>ExplorePage: Update map colors
User->>ExplorePage: Click state
ExplorePage->>API: GET /api/explore/policies (state)
activate API
API->>DB: Query policy_bills (by state)
DB-->>API: PolicyBillItem[]
API-->>ExplorePage: PolicyBillItem[]
deactivate API
ExplorePage->>ExplorePage: Render policy table + chart
deactivate ExplorePage
sequenceDiagram
participant Script as Ingest Script
participant ExtAPI as External API
participant DB as PostgreSQL<br/>Database
Script->>Script: Parse CLI (--year, --state, --dry-run)
Script->>DB: init_db / create_tables
alt Census ACS ingest
Script->>ExtAPI: Fetch ACS variables
ExtAPI-->>Script: Rows
Script->>Script: compute_rate / transform
else OpenStates ingest
Script->>ExtAPI: GraphQL bills (paginated)
ExtAPI-->>Script: Bill nodes
Script->>Script: map_status / dedupe
end
Script->>DB: Upsert records (on_conflict_do_update)
DB-->>Script: ACK
alt dry-run
Script->>Script: Skip commit
else
Script->>DB: Commit
end
Script->>Script: Print summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (11)
tests/test_crew_task_ordering.py (1)
36-39: Remove redundant in-test imports/path mutation.
sys.path.insertandD4Blimport are already done at module scope; repeating them here adds noise and side effects.🧹 Proposed cleanup
def test_all_agents_full_canonical_order(): - import sys - sys.path.insert(0, 'src') - from d4bl.agents.crew import D4Bl all_agents = list(D4Bl.AGENT_TASK_MAP.keys()) names = _get_ordered_task_names(all_agents) assert names == D4Bl.TASK_ORDER🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_crew_task_ordering.py` around lines 36 - 39, Remove the redundant in-test imports and path mutation by deleting the repeated sys.path.insert(0, 'src') and the duplicate from d4bl.agents.crew import D4Bl in the test block; rely on the module-scope imports already present and use the existing D4Bl symbol to compute all_agents = list(D4Bl.AGENT_TASK_MAP.keys()) without re-importing or mutating sys.path in the test body.src/d4bl/agents/crew.py (1)
65-74: MakeTASK_ORDERimmutable and typed.Using a mutable class-level list here makes accidental runtime mutation possible and can break determinism across tests/runs. Convert to an immutable tuple with explicit
Finaltype hint.♻️ Proposed refactor
-from typing import List, Optional +from typing import Final, List, Optional @@ - TASK_ORDER = [ + TASK_ORDER: Final[tuple[str, ...]] = ( "research_task", "analysis_task", "writing_task", "fact_checker_task", "citation_task", "bias_detection_task", "editor_task", "data_visualization_task", - ] + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/d4bl/agents/crew.py` around lines 65 - 74, TASK_ORDER is currently a mutable class-level list; change it to an immutable tuple and add an explicit Final type hint to prevent runtime mutation and signal intent. Replace the list literal assigned to TASK_ORDER with a tuple literal (use parentheses) and annotate it with typing.Final (e.g., from typing import Final) so the symbol TASK_ORDER is typed as a Final[tuple[str, ...]] (or Final[Tuple[str, ...]] depending on imports). Ensure imports are updated to include Final (and Tuple if needed).tests/test_explore_models.py (1)
61-72: Align the test name with the assertedtopic_tagsbehavior.Line 61 says “defaults to empty list”, but Line 72 allows
None.
Please either rename the test to nullable behavior or assert one exact default.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_explore_models.py` around lines 61 - 72, The test test_topic_tags_defaults_to_empty_list is misnamed because the assertion allows PolicyBill.topic_tags to be None or a list; rename the test to reflect nullable behavior (e.g., test_topic_tags_is_nullable_or_list) or change the assertion to enforce a single expectation; update either the test function name or the assertion around PolicyBill(topic_tags) so the name and behavior match (reference: test_topic_tags_defaults_to_empty_list, PolicyBill, topic_tags).ui-nextjs/components/explore/PolicyTable.tsx (2)
51-73: Expose pressed state on topic toggle buttons.These controls behave as toggles but currently do not announce selected state to assistive tech.
♿ Suggested fix
<button onClick={() => setActiveTopic(null)} + aria-pressed={activeTopic === null} className={`px-3 py-1 rounded-full text-xs font-medium transition-colors ${ activeTopic === null ? 'bg-[`#00ff32`] text-black' : 'bg-[`#2a2a2a`] text-gray-400 border border-[`#404040`] hover:border-[`#00ff32`]' }`} > @@ <button key={topic} onClick={() => setActiveTopic(activeTopic === topic ? null : topic)} + aria-pressed={activeTopic === topic} className={`px-3 py-1 rounded-full text-xs font-medium capitalize transition-colors ${ activeTopic === topic ? 'bg-[`#00ff32`] text-black' : 'bg-[`#2a2a2a`] text-gray-400 border border-[`#404040`] hover:border-[`#00ff32`]' }`} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-nextjs/components/explore/PolicyTable.tsx` around lines 51 - 73, The topic buttons (including the "All" button) are toggle controls but do not expose their pressed/selected state to assistive tech; update the buttons in PolicyTable (the button rendering that uses activeTopic, setActiveTopic, and ALL_TOPICS) to include an explicit ARIA state such as aria-pressed={activeTopic === topic} for each topic button and aria-pressed={activeTopic === null} for the "All" button (or aria-current="true"/"false" if you prefer current semantics) so screen readers can announce which toggle is selected; ensure the attribute value changes when onClick updates activeTopic.
83-85: Use a stable key for bill rows instead of array index.Line 84 uses
key={i}, which can cause stale row reconciliation when filters change.♻️ Suggested fix
- {filtered.map((bill, i) => ( - <div key={i} className="py-3 flex items-start justify-between gap-4"> + {filtered.map((bill) => ( + <div key={`${bill.state}-${bill.bill_number}`} className="py-3 flex items-start justify-between gap-4">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-nextjs/components/explore/PolicyTable.tsx` around lines 83 - 85, The mapped rows in PolicyTable use the array index (key={i}) which can break React reconciliation when filtering; replace the unstable index key with a stable unique identifier from each bill (e.g., bill.id, bill.slug, bill.billId or a deterministic composite like `${bill.year}-${bill.number}`) in the map over filtered so the <div key=...> uses that stable property instead of i; ensure the chosen property is unique and present on the bill objects returned to avoid fallback to indexing.ui-nextjs/components/explore/RacialGapChart.tsx (1)
91-93: Use a stable key for chart cells.Line 92 uses array index as key. Prefer a deterministic key from the mapped data item.
♻️ Suggested fix
<Bar dataKey="value" radius={[4, 4, 0, 0]} barSize={40}> {data.map((entry, index) => ( - <Cell key={index} fill={entry.fill} /> + <Cell key={entry.race} fill={entry.fill} /> ))} </Bar>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-nextjs/components/explore/RacialGapChart.tsx` around lines 91 - 93, The Cell elements in the data.map inside RacialGapChart use the array index as the key; replace the index key with a stable, deterministic identifier from each entry (e.g., entry.id, entry.label, or a composed key like `${entry.label}-${entry.fill}`), or add a unique id to the data items before mapping, and set that property as the key on <Cell> instead of using the index.scripts/ingest_census_acs.py (3)
74-75: Addstrict=Truetozip()to catch length mismatches.If the Census API returns malformed data where row lengths don't match header length, this would silently truncate data. Adding
strict=Truewill raise aValueErrorif lengths differ, making debugging easier.🛡️ Suggested fix
headers = rows[0] - return [dict(zip(headers, row)) for row in rows[1:]] + return [dict(zip(headers, row, strict=True)) for row in rows[1:]]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ingest_census_acs.py` around lines 74 - 75, The list comprehension that builds records from headers and rows uses zip(headers, row) which silently truncates when lengths mismatch; change the zip call in the return expression to zip(headers, row, strict=True) so a ValueError is raised on length mismatches, and wrap the comprehension (or the calling function) in a try/except to catch ValueError, log the offending headers and row for debugging, and re-raise or handle appropriately; update the symbols headers, rows and the zip(...) usage in ingest_census_acs.py.
104-110: Clarify the tuple order comment.The comment says "(denominator, numerator)" but
varsis defined as(total, owner-occupied)in METRIC_VARS. The code correctly passesvars[1](numerator) andvars[0](denominator) tocompute_rate, so the logic is correct, but the comment could be clearer.📝 Suggested clarification
- # For rate metrics, vars is a tuple (denominator, numerator) + # For rate metrics, vars is a tuple (denominator, numerator) where + # vars[0] = denominator (e.g., total households), vars[1] = numerator (e.g., owner-occupied)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ingest_census_acs.py` around lines 104 - 110, The inline comment about tuple order is misleading; clarify that for rate metrics the tuple in METRIC_VARS is (denominator, numerator) but defined as (total, owner-occupied) so the code intentionally uses vars[1] as numerator and vars[0] as denominator when calling compute_rate; update the comment above the tuple handling in the block that calls fetch_acs and compute_rate (referencing vars, METRIC_VARS, fetch_acs, compute_rate, rows, fips) to explicitly state the tuple element meaning and that compute_rate receives (numerator, denominator) as compute_rate(vars[1], vars[0]) to avoid confusion.
64-64: Consider using unpacking syntax for list concatenation.Using spread operator is more idiomatic and slightly cleaner.
✨ Suggested refactor
- get_str = ",".join(["NAME"] + vars) + get_str = ",".join(["NAME", *vars])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ingest_census_acs.py` at line 64, Replace the explicit list concatenation when building get_str with list unpacking for clarity: instead of creating a new list via ["NAME"] + vars inside the ",".join call, construct the list using unpacking (e.g., ["NAME", *vars]) before joining; update the expression that assigns get_str to use this unpacked list form to achieve the same output more idiomatically.src/d4bl/infra/database.py (1)
9-9: Line exceeds 100 character limit.This import line is 103 characters. Consider splitting it across multiple lines for readability.
As per coding guidelines: "Enforce 100 character maximum line length"
✨ Suggested refactor
-from sqlalchemy import JSON, Text, Column, String, DateTime, Float, Integer, Date, Index, UniqueConstraint +from sqlalchemy import ( + JSON, Text, Column, String, DateTime, Float, Integer, Date, Index, UniqueConstraint +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/d4bl/infra/database.py` at line 9, The long import statement importing JSON, Text, Column, String, DateTime, Float, Integer, Date, Index, UniqueConstraint exceeds the 100-character limit; break this single line into multiple shorter import lines (for example grouping related symbols or one-per-line) so the line length is under 100 characters while keeping the same imported names (JSON, Text, Column, String, DateTime, Float, Integer, Date, Index, UniqueConstraint).src/d4bl/app/api.py (1)
549-553: Topic filter may produce unexpected matches.The JSON-to-string cast with
contains()could match partial strings within the JSON serialization (e.g., searching for "health" could match["mental health", "public health"]but also potentially match keys or other JSON artifacts). Consider using PostgreSQL's native JSON containment operator (@>) if precision is critical.💡 Alternative using JSON containment (PostgreSQL-specific)
# For exact array element matching: from sqlalchemy.dialects.postgresql import JSONB # If topic_tags were JSONB: # query = query.where(PolicyBill.topic_tags.contains([topic])) # Current approach works for substring matching which may be acceptable # for the use case (matching "housing" in ["affordable housing"])For this dashboard use case, the current approach is likely sufficient since topic strings are controlled via FOCUS_SUBJECTS in the ingestion script.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/d4bl/app/api.py` around lines 549 - 553, The current filter uses PolicyBill.topic_tags.cast(String).contains(topic) which can produce partial/false matches; replace it with PostgreSQL JSON containment to match exact array elements by using PolicyBill.topic_tags.contains([topic]) (ensure topic_tags is JSONB or cast to postgresql.JSONB) and import JSONB from sqlalchemy.dialects.postgresql as needed; keep a fallback to the substring approach only if JSONB is not available or portability is required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.coderabbit.yaml:
- Line 11: The setting auto_incremental_review is currently at top-level and
must be moved under the reviews.auto_review object to match the documented
schema; update the .coderabbit.yaml by removing the top-level
auto_incremental_review entry and adding it as reviews:\n auto_review:\n
auto_incremental_review: <value> so the tool will pick up the option (refer to
the auto_incremental_review and reviews.auto_review keys to locate where to
change).
In `@docs/plans/2026-02-19-explore-data-design.md`:
- Around line 134-160: The fenced ASCII layout block (the NAV/HERO/CHOROPLETH
map diagram) lacks a language tag and triggers MD040; fix it by adding a
language identifier (e.g., "text") after the opening triple backticks of that
fenced code block so it reads ```text, keeping the block content unchanged;
locate the block by its unique ASCII header (the "NAV: D4BL | Research |
Explore Data" line) or the choropleth/map diagram to update the fence.
In `@scripts/ingest_openstates.py`:
- Around line 212-240: The upsert for PolicyBill (the stmt using
insert(PolicyBill) and .on_conflict_do_update) currently omits the model's
introduced_date and last_action_date columns; update the .values(...) and the
.on_conflict_do_update(..., set={...}) maps to include these two fields (e.g.,
mapping introduced_date to bill.get("createdAt") and last_action_date to
bill.get("updatedAt") or whatever keys the GraphQL response provides), or if
those keys don't exist add them to the GraphQL query and then use bill.get(...)
to populate both the initial insert and the conflict update.
In `@src/d4bl/app/schemas.py`:
- Around line 85-116: Add short class-level docstrings for the public Pydantic
models IndicatorItem, PolicyBillItem, and StateSummaryItem in schemas.py: for
each class (IndicatorItem, PolicyBillItem, StateSummaryItem) add a one- or
two-sentence docstring at top of the class describing its purpose and the key
fields it represents (e.g., IndicatorItem represents a single indicator
observation with fips_code, year, metric, value; PolicyBillItem represents a
legislative bill summary with state, bill_number, title, status;
StateSummaryItem summarizes per-state metadata like state_fips,
available_metrics, bill_count, latest_year). Ensure docstrings are simple,
placed immediately under the class definition, and follow existing docstring
style used in the codebase.
In `@src/d4bl/evals/runner.py`:
- Around line 43-45: The code currently does research_output =
str(result_dict.get("result") or "") which stringifies the entire result object;
instead, extract the actual answer text from the result payload (e.g., check
result_dict.get("result") for common keys like "answer", "text", "output" or
nested result["response"] and use that value) and assign that string to
research_output so evaluators get the human-readable answer; replace the
str(...) usage in the block that defines research_output (and the similar block
around lines 56-60) to prefer result_dict["result"]["answer"] or fall back to
other text keys, handling missing keys by using an empty string.
- Around line 61-62: The current call in runner.py sets trace_id=str(job.job_id)
which ignores an existing ResearchJob.trace_id and breaks correlation; update
the code that constructs the trace (where trace_id is passed—look for the block
using trace_id=str(job.job_id)) to use job.trace_id when present, e.g. set
trace_id = str(job.trace_id) if job.trace_id else str(job.job_id) (or equivalent
null-coalescing) so evaluations use persisted trace IDs while falling back to
job_id.
In `@tests/test_crew_task_ordering.py`:
- Around line 8-16: The current helper _get_ordered_task_names merely recomputes
order from D4Bl.AGENT_TASK_MAP and D4Bl.TASK_ORDER; update the tests to exercise
D4Bl.crew() directly: instantiate D4Bl, call its crew(...) method with
selected_agents set, then assert the returned crew object's crew.tasks (or
.tasks attribute on the object returned by D4Bl.crew) yields the expected task
order (compare task.name or str representation) instead of using
AGENT_TASK_MAP/TASK_ORDER directly; keep the helper only for expected-name
computation if needed but switch assertions to verify D4Bl.crew() behavior
(referencing D4Bl.crew, crew.tasks, _get_ordered_task_names,
D4Bl.AGENT_TASK_MAP, D4Bl.TASK_ORDER).
In `@tests/test_explore_schemas.py`:
- Around line 28-33: Update the test_metric_required unit test so it only
validates the absence of the metric field: instantiate IndicatorItem with valid
values for all other required attributes (e.g., fips_code, geography_name, and
any other mandatory fields on the model) but intentionally omit metric, then
assert that creating IndicatorItem raises a pydantic.ValidationError; modify the
test function name/test body (test_metric_required and the IndicatorItem
constructor call) to pass concrete valid values for all required fields except
metric so the failure isolates metric being required.
In `@ui-nextjs/app/explore/page.tsx`:
- Around line 75-77: Replace the permissive catch typing "catch (e: any)" with
"catch (e: unknown)" and narrow the type before using e.message; for example in
the async blocks around setError in app/explore/page.tsx (the catch blocks that
currently call setError(e.message)), check "if (e instanceof Error)
setError(e.message) else setError(String(e))" (or similar explicit narrowing) so
you handle non-Error throwables safely; apply the same change to the other two
catch sites referenced (lines ~96–98 and ~117–119).
- Around line 64-128: Old async fetches (fetchMapData, fetchChartData,
fetchBills) can be overwritten by stale responses and use catch(e: any) which
breaks strict TS; fix by making each fetch use an AbortController (create a
controller inside each fetch callback and pass controller.signal to fetch) and
ensure responses only update setMapIndicators, setChartIndicators, setBills,
setError if the request was not aborted, and wire a cleanup in the useEffect (or
return a cancel function) to abort pending controllers when filters change; also
change all catch blocks to catch(e: unknown) and convert to a safe message (e
instanceof Error ? e.message : String(e)) before calling setError.
In `@ui-nextjs/app/layout.tsx`:
- Around line 18-19: The metadata description string in layout.tsx (the
description property of the metadata object) exceeds the 100-character line
limit; split the long description into multiple shorter string literals (for
example by concatenating two strings or using an array.join) so the resulting
code for the description property stays under 100 characters per line while
preserving the exact text, updating the description property in the metadata
object accordingly.
In `@ui-nextjs/components/explore/StateMap.tsx`:
- Around line 54-72: The Geography elements are not keyboard-accessible: update
the Geography component usage (the element with props geography, fill, stroke,
onClick, onMouseEnter/onMouseLeave) to include tabIndex={0}, role="button", and
a descriptive aria-label (e.g., using geo.properties.name and value), add an
onKeyDown handler that triggers onSelectState(fips, geo.properties.name) when
Enter or Space is pressed, and remove outline: 'none' from the default style so
focused elements get a visible focus outline; ensure existing tooltip behavior
(setTooltip) remains on focus/blur or via keyboard activation as appropriate.
---
Nitpick comments:
In `@scripts/ingest_census_acs.py`:
- Around line 74-75: The list comprehension that builds records from headers and
rows uses zip(headers, row) which silently truncates when lengths mismatch;
change the zip call in the return expression to zip(headers, row, strict=True)
so a ValueError is raised on length mismatches, and wrap the comprehension (or
the calling function) in a try/except to catch ValueError, log the offending
headers and row for debugging, and re-raise or handle appropriately; update the
symbols headers, rows and the zip(...) usage in ingest_census_acs.py.
- Around line 104-110: The inline comment about tuple order is misleading;
clarify that for rate metrics the tuple in METRIC_VARS is (denominator,
numerator) but defined as (total, owner-occupied) so the code intentionally uses
vars[1] as numerator and vars[0] as denominator when calling compute_rate;
update the comment above the tuple handling in the block that calls fetch_acs
and compute_rate (referencing vars, METRIC_VARS, fetch_acs, compute_rate, rows,
fips) to explicitly state the tuple element meaning and that compute_rate
receives (numerator, denominator) as compute_rate(vars[1], vars[0]) to avoid
confusion.
- Line 64: Replace the explicit list concatenation when building get_str with
list unpacking for clarity: instead of creating a new list via ["NAME"] + vars
inside the ",".join call, construct the list using unpacking (e.g., ["NAME",
*vars]) before joining; update the expression that assigns get_str to use this
unpacked list form to achieve the same output more idiomatically.
In `@src/d4bl/agents/crew.py`:
- Around line 65-74: TASK_ORDER is currently a mutable class-level list; change
it to an immutable tuple and add an explicit Final type hint to prevent runtime
mutation and signal intent. Replace the list literal assigned to TASK_ORDER with
a tuple literal (use parentheses) and annotate it with typing.Final (e.g., from
typing import Final) so the symbol TASK_ORDER is typed as a Final[tuple[str,
...]] (or Final[Tuple[str, ...]] depending on imports). Ensure imports are
updated to include Final (and Tuple if needed).
In `@src/d4bl/app/api.py`:
- Around line 549-553: The current filter uses
PolicyBill.topic_tags.cast(String).contains(topic) which can produce
partial/false matches; replace it with PostgreSQL JSON containment to match
exact array elements by using PolicyBill.topic_tags.contains([topic]) (ensure
topic_tags is JSONB or cast to postgresql.JSONB) and import JSONB from
sqlalchemy.dialects.postgresql as needed; keep a fallback to the substring
approach only if JSONB is not available or portability is required.
In `@src/d4bl/infra/database.py`:
- Line 9: The long import statement importing JSON, Text, Column, String,
DateTime, Float, Integer, Date, Index, UniqueConstraint exceeds the
100-character limit; break this single line into multiple shorter import lines
(for example grouping related symbols or one-per-line) so the line length is
under 100 characters while keeping the same imported names (JSON, Text, Column,
String, DateTime, Float, Integer, Date, Index, UniqueConstraint).
In `@tests/test_crew_task_ordering.py`:
- Around line 36-39: Remove the redundant in-test imports and path mutation by
deleting the repeated sys.path.insert(0, 'src') and the duplicate from
d4bl.agents.crew import D4Bl in the test block; rely on the module-scope imports
already present and use the existing D4Bl symbol to compute all_agents =
list(D4Bl.AGENT_TASK_MAP.keys()) without re-importing or mutating sys.path in
the test body.
In `@tests/test_explore_models.py`:
- Around line 61-72: The test test_topic_tags_defaults_to_empty_list is misnamed
because the assertion allows PolicyBill.topic_tags to be None or a list; rename
the test to reflect nullable behavior (e.g.,
test_topic_tags_is_nullable_or_list) or change the assertion to enforce a single
expectation; update either the test function name or the assertion around
PolicyBill(topic_tags) so the name and behavior match (reference:
test_topic_tags_defaults_to_empty_list, PolicyBill, topic_tags).
In `@ui-nextjs/components/explore/PolicyTable.tsx`:
- Around line 51-73: The topic buttons (including the "All" button) are toggle
controls but do not expose their pressed/selected state to assistive tech;
update the buttons in PolicyTable (the button rendering that uses activeTopic,
setActiveTopic, and ALL_TOPICS) to include an explicit ARIA state such as
aria-pressed={activeTopic === topic} for each topic button and
aria-pressed={activeTopic === null} for the "All" button (or
aria-current="true"/"false" if you prefer current semantics) so screen readers
can announce which toggle is selected; ensure the attribute value changes when
onClick updates activeTopic.
- Around line 83-85: The mapped rows in PolicyTable use the array index
(key={i}) which can break React reconciliation when filtering; replace the
unstable index key with a stable unique identifier from each bill (e.g.,
bill.id, bill.slug, bill.billId or a deterministic composite like
`${bill.year}-${bill.number}`) in the map over filtered so the <div key=...>
uses that stable property instead of i; ensure the chosen property is unique and
present on the bill objects returned to avoid fallback to indexing.
In `@ui-nextjs/components/explore/RacialGapChart.tsx`:
- Around line 91-93: The Cell elements in the data.map inside RacialGapChart use
the array index as the key; replace the index key with a stable, deterministic
identifier from each entry (e.g., entry.id, entry.label, or a composed key like
`${entry.label}-${entry.fill}`), or add a unique id to the data items before
mapping, and set that property as the key on <Cell> instead of using the index.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ui-nextjs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
.coderabbit.yamldocker-compose.observability.ymldocs/plans/2026-02-19-explore-data-design.mddocs/plans/2026-02-19-explore-data-page.mdscripts/add_evaluation_results_table.pyscripts/add_job_id_to_evaluations.pyscripts/add_research_data_column.pyscripts/add_trace_id_to_research_jobs.pyscripts/archive_and_wipe_db.pyscripts/ingest_census_acs.pyscripts/ingest_openstates.pyscripts/init_db.pyscripts/test_db_connection.pysrc/d4bl/agents/crew.pysrc/d4bl/app/__init__.pysrc/d4bl/app/api.pysrc/d4bl/app/schemas.pysrc/d4bl/crew.pysrc/d4bl/evals/__init__.pysrc/d4bl/evals/runner.pysrc/d4bl/infra/database.pysrc/d4bl/services/langfuse/client.pytests/test_crew_task_ordering.pytests/test_explore_api.pytests/test_explore_models.pytests/test_explore_schemas.pyui-nextjs/app/explore/page.tsxui-nextjs/app/layout.tsxui-nextjs/components/explore/MetricFilterPanel.tsxui-nextjs/components/explore/PolicyTable.tsxui-nextjs/components/explore/RacialGapChart.tsxui-nextjs/components/explore/StateMap.tsxui-nextjs/package.json
- Fix .coderabbit.yaml nesting (auto_incremental_review under auto_review) - Add AbortController to explore page fetches to prevent stale responses - Replace catch(e: any) with catch(e: unknown) and proper type narrowing - Add keyboard accessibility to StateMap (tabIndex, role, onKeyDown) - Add docstrings to IndicatorItem, PolicyBillItem, StateSummaryItem - Map introduced_date and last_action_date in OpenStates ingestion upsert - Isolate metric-required test to omit only the metric field - Add language tag to fenced code block in design doc - Split long metadata description line in layout.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ui-nextjs/app/layout.tsx (1)
42-58: Well-structured navigation with good accessibility baseline.The nav uses semantic HTML (
<nav>element), proper Link components for client-side routing, and Tailwind CSS styling as per guidelines.Optional enhancement: Consider adding visual indication for the active route using
usePathname()fromnext/navigation. This improves UX by showing users which page they're currently viewing.♻️ Optional: Add active link state
// At the top of the file, add: "use client"; import { usePathname } from "next/navigation"; // Inside RootLayout, before the return: const pathname = usePathname(); // Then update Link classNames to include active state: <Link href="/" className={`text-sm transition-colors ${ pathname === "/" ? "text-[`#00ff32`]" : "text-gray-300 hover:text-[`#00ff32`]" }`} > Research </Link>Note: This would convert the layout to a Client Component. If you prefer keeping it as a Server Component, extract the nav into a separate
<NavBar />client component instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-nextjs/app/layout.tsx` around lines 42 - 58, Add an active-route visual state to the navigation by using usePathname() from next/navigation: convert the nav into a client component (add "use client") or extract it into a new NavBar client component, import usePathname, call usePathname() inside the component (e.g., in RootLayout or NavBar) and update the Link className logic for "Research" and "Explore Data" so they render the active style (e.g., text-[`#00ff32`]) when pathname matches the link href and the default gray/hover style otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ui-nextjs/app/layout.tsx`:
- Around line 42-58: Add an active-route visual state to the navigation by using
usePathname() from next/navigation: convert the nav into a client component (add
"use client") or extract it into a new NavBar client component, import
usePathname, call usePathname() inside the component (e.g., in RootLayout or
NavBar) and update the Link className logic for "Research" and "Explore Data" so
they render the active style (e.g., text-[`#00ff32`]) when pathname matches the
link href and the default gray/hover style otherwise.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.coderabbit.yamldocs/plans/2026-02-19-explore-data-design.mdscripts/ingest_openstates.pysrc/d4bl/app/schemas.pytests/test_explore_schemas.pyui-nextjs/app/explore/page.tsxui-nextjs/app/layout.tsxui-nextjs/components/explore/StateMap.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- ui-nextjs/app/explore/page.tsx
- .coderabbit.yaml
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
This was referenced Mar 3, 2026
This was referenced Mar 10, 2026
5 tasks
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/explorepage with race-disaggregated Census ACS indicators and state policy bills dashboardGET /api/explore/indicators,GET /api/explore/policies,GET /api/explore/statesStateMap(interactive choropleth),RacialGapChart(recharts bar chart),MetricFilterPanel(indicator/state selectors),PolicyTable(sortable bill listing)ingest_census_acs.py(Census Bureau API) andingest_openstates.py(OpenStates API) with--dry-runsupportCensusIndicatorandPolicyBillSQLAlchemy models (committed earlier on this branch)Test plan
pytest tests/ -v— all 52 tests passcd ui-nextjs && npm run build— TypeScript build clean,/exploreroute generatedpython scripts/ingest_census_acs.py --help— prints usagepython scripts/ingest_openstates.py --help— prints usage/explorepage renders map, filter panel, chart, and policy tableSummary by CodeRabbit
New Features
Documentation
Tests
Chores