refactor: dashboard/settings redesign + keychain API key storage#99
Conversation
- PaperCard: collapsible evidence quotes section below judge scores - SavedTab: export dropdown (BibTeX/RIS/Markdown) triggering browser download - TopicWorkflowDashboard: Download .md button for daily paper reports - SearchResults: add papers_cool and hf_daily to SOURCE_OPTIONS Closes #96
…ion test
- Add GET /api/research/metrics/evidence-coverage endpoint that returns
coverage_rate, total_claims, total_with_evidence, and daily trend data
- Add unit test verifying saved papers rank higher than skipped papers
in list_track_feed
- Enhance POST /api/model-endpoints/{id}/test to return latency_ms,
success, and error fields
- Add unit tests for all new functionality
Closes #92
Closes #95
Closes #97
- Add utils/secret.py with encrypt/decrypt using Fernet (AES-128-CBC + HMAC-SHA256) - Encryption key from PAPERBOT_SECRET_KEY env var, fallback key with warning - model_endpoint_store: encrypt on write, decrypt on read - Graceful migration: non-Fernet values (legacy plaintext) pass through on decrypt - Fix pre-existing test bugs (unquoted f-string path expressions) Closes #77
…gine - Remove paper_searcher parameter from ContextEngine.__init__ - Remove legacy SemanticScholarSearch import and direct API call path - ContextEngine now exclusively uses PaperSearchService for paper search - Fallback: logs warning and returns empty results if no search_service provided - Remove unused PaperMeta import Closes #78
- Replace direct harvester calls (ArxivHarvester, SemanticScholarHarvester, OpenAlexHarvester) with PaperSearchService.search() - PaperSearchService handles dedup and persistence internally, removing the need for separate dedup and store phases - Remove unused imports (HarvestedPaper, HarvestResult, HarvestRunResult, PaperDeduplicator, HarvesterPort, individual harvesters) - Keep pipeline orchestration: keyword expansion, venue recommendation, harvest run records, progress events Closes #79
Delete PapersCoolTopicSearchWorkflow, TopicSearchSourceRegistry, SemanticScholarSearch and their test files. Update all test mocks to patch _run_topic_search / run_unified_topic_search instead of the removed classes. Fix pre-existing SSE event assertions that expected per-paper judge events no longer emitted by EnrichmentPipeline. Closes #80
Rewrite dashboard with clean 2/3+1/3 grid (StatsBar, ActivityTimeline, LLMUsageChart, SavedPapers, DeadlineRadar), delete 9 deprecated components. Rebuild settings page with ccswitch-style provider cards and Dialog form. Add KeychainStore for macOS Keychain API key storage with DB encryption fallback. Add usage proxy route.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Summary of ChangesHello @jerry609, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a comprehensive overhaul of the user interface for the Dashboard and Settings pages, enhancing usability and visual consistency. Concurrently, it introduces a significant security upgrade by integrating Keychain for API key storage, ensuring sensitive data is handled more robustly. These changes are supported by underlying data model refactorings and API adjustments to streamline data flow and improve overall system architecture. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR delivers a breaking redesign of the Dashboard and Settings UI, alongside backend support for secure API key storage (Keychain-first with encrypted-DB fallback) and several related API/type updates.
Changes:
- Redesign Dashboard layout (StatsBar + ActivityTimeline + right-rail cards) and remove deprecated dashboard/feed components.
- Redesign Settings providers UX (card list + add/edit dialog) and surface Keychain status/masked key display.
- Add backend utilities/endpoints for Keychain/secret handling plus new export + metrics endpoints, with updated tests.
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/lib/types.ts | Update frontend types (TimelineItem/SavedPaper) and expand Paper status. |
| web/src/lib/api.ts | Update dashboard-related fetchers and add SSR/CSR usage fetching behavior. |
| web/src/components/research/TopicWorkflowDashboard.tsx | Add client-side markdown download for DailyPaper report. |
| web/src/components/research/SearchResults.tsx | Add new search source options. |
| web/src/components/research/SavedTab.tsx | Add export dropdown for saved papers. |
| web/src/components/research/PaperCard.tsx | Add evidence quotes UI for judge results. |
| web/src/components/dashboard/feed/NewPaperCard.tsx | Remove deprecated feed card component. |
| web/src/components/dashboard/feed/MilestoneCard.tsx | Remove deprecated feed card component. |
| web/src/components/dashboard/feed/ConferenceCard.tsx | Remove deprecated feed card component. |
| web/src/components/dashboard/StatsCard.tsx | Remove deprecated stats card component. |
| web/src/components/dashboard/StatsBar.tsx | Add compact stats header component. |
| web/src/components/dashboard/SavedPapers.tsx | Add dashboard card for saved papers list. |
| web/src/components/dashboard/ReadingQueue.tsx | Remove deprecated reading queue component. |
| web/src/components/dashboard/QuickFilters.tsx | Remove deprecated quick filters component. |
| web/src/components/dashboard/QuickActions.tsx | Remove deprecated quick actions component. |
| web/src/components/dashboard/PipelineStatus.tsx | Remove deprecated pipeline status component. |
| web/src/components/dashboard/LLMUsageChart.tsx | Simplify chart header + empty state behavior. |
| web/src/components/dashboard/DeadlineRadar.tsx | Tweak radar layout and limit displayed items. |
| web/src/components/dashboard/ActivityTimeline.tsx | Add new timeline component for recent activity. |
| web/src/components/dashboard/ActivityFeed.tsx | Remove deprecated activity feed component. |
| web/src/app/settings/page.tsx | Redesign providers settings page with dialog-based editing + keychain indicator. |
| web/src/app/dashboard/page.tsx | Switch dashboard to new layout and new data sources. |
| web/src/app/api/model-endpoints/usage/route.ts | Add Next.js proxy route for usage endpoint. |
| tests/unit/test_topic_search_sources.py | Remove tests for removed legacy topic search sources module. |
| tests/unit/test_paperscool_topic_search.py | Remove tests for removed legacy paperscool topic search workflow. |
| tests/unit/test_paperscool_route.py | Update route tests to monkeypatch unified search runner. |
| tests/unit/test_paperscool_cli.py | Update CLI tests to use unified search runner mock. |
| tests/unit/test_paper_judge.py | Add evidence_quotes parsing/serialization tests. |
| tests/unit/test_model_endpoint_store.py | Fix sqlite tmp_path string interpolation in tests. |
| tests/unit/test_feed_ranking.py | Add ranking test asserting saved beats skipped. |
| tests/unit/test_export.py | Add export formatting tests (BibTeX/RIS/Markdown). |
| tests/unit/test_evidence_coverage_route.py | Add tests for evidence coverage metrics endpoint. |
| tests/unit/test_connection_test_endpoint.py | Add tests for model endpoint connection test response. |
| tests/unit/test_arq_daily_papers.py | Update daily job test to mock unified search runner. |
| tests/test_framework.py | Remove search module tests tied to removed legacy search utilities. |
| src/paperbot/utils/secret.py | Add Fernet-based secret encryption helper. |
| src/paperbot/utils/search.py | Remove legacy Semantic Scholar search utility module. |
| src/paperbot/infrastructure/stores/paper_store.py | Adjust library sorting key usage. |
| src/paperbot/infrastructure/stores/model_endpoint_store.py | Implement keychain-backed API key storage with encrypted-DB fallback. |
| src/paperbot/infrastructure/stores/keychain.py | Add keyring wrapper for OS keychain storage. |
| src/paperbot/context_engine/engine.py | Remove legacy direct SemanticScholarSearch fallback path. |
| src/paperbot/application/workflows/topic_search_sources.py | Remove legacy topic search sources module. |
| src/paperbot/application/workflows/paperscool_topic_search.py | Remove legacy papers.cool topic search workflow. |
| src/paperbot/application/workflows/harvest_pipeline.py | Switch harvesting to unified PaperSearchService. |
| src/paperbot/application/workflows/analysis/paper_judge.py | Add evidence_quotes to judgment model + parsing. |
| src/paperbot/application/workflows/analysis/judge_prompts.py | Extend judge prompt schema to include evidence_quotes. |
| src/paperbot/api/routes/research.py | Add evidence coverage endpoint + paper export endpoint and helpers. |
| src/paperbot/api/routes/paperscool.py | Count judge evidence_quotes as evidence in metrics. |
| src/paperbot/api/routes/model_endpoints.py | Add latency/success fields to endpoint test response. |
| pyproject.toml | Add keyring + cryptography dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| row.name = name | ||
| row.vendor = vendor | ||
| row.base_url = str(payload.get("base_url") or row.base_url or "").strip() or None | ||
| row.api_key_env = ( | ||
| str(payload.get("api_key_env") or row.api_key_env or "OPENAI_API_KEY").strip() | ||
| or "OPENAI_API_KEY" | ||
| ) | ||
| if "api_key" in payload: | ||
| api_key_text = str(payload.get("api_key") or "").strip() | ||
| if not api_key_text: | ||
| row.api_key_value = None | ||
| KeychainStore.delete_key(name) | ||
| elif not api_key_text.startswith("***"): | ||
| row.api_key_value = api_key_text | ||
| if KeychainStore.store_key(name, api_key_text): | ||
| row.api_key_value = _KEYCHAIN_MARKER | ||
| else: | ||
| row.api_key_value = _encrypt_secret(api_key_text) |
There was a problem hiding this comment.
Keychain entries are keyed by endpoint_name (KeychainStore.store_key(name, ...) / get_key(row.name)). If an endpoint is renamed without re-entering the API key, the stored secret becomes unreachable (and delete_key will also target the new name, leaving the old entry behind). Prefer a stable identifier for the keyring username (e.g., endpoint id) or add explicit rename/migration logic (copy old key -> new name, then delete old).
| from datetime import datetime, timezone | ||
| from typing import Any, Dict, List, Optional | ||
|
|
||
| from loguru import logger |
There was a problem hiding this comment.
logger is imported but never used in this module. Remove the import to avoid lint/type-check noise.
| from loguru import logger |
| raw = os.getenv(_ENV_KEY, "").strip() | ||
| if raw: | ||
| # Accept either a raw 32-byte-urlsafe-b64 Fernet key or an | ||
| # arbitrary passphrase that we hash into one. | ||
| try: | ||
| Fernet(raw.encode()) | ||
| key = raw.encode() | ||
| except Exception: | ||
| key = base64.urlsafe_b64encode( | ||
| hashlib.sha256(raw.encode()).digest() | ||
| ) | ||
| else: | ||
| logger.warning( | ||
| f"{_ENV_KEY} is not set — using deterministic fallback key. " | ||
| "Set this variable in production to secure API keys at rest." | ||
| ) | ||
| key = base64.urlsafe_b64encode( | ||
| hashlib.sha256(b"paperbot-default-insecure-key").digest() | ||
| ) |
There was a problem hiding this comment.
When PAPERBOT_SECRET_KEY is missing, _get_fernet() derives a deterministic fallback key from a constant string. That makes DB-encrypted API keys decryptable by anyone with code access and can give a false sense of “encryption at rest”. Consider failing closed (raise) when attempting to encrypt/decrypt secrets without a configured key, or generate a random key once and require it to be persisted/managed explicitly (and gate the relaxed behavior behind a clear dev-only flag).
| // Use Next.js proxy route for SSR compatibility | ||
| const url = typeof window === "undefined" | ||
| ? `${API_BASE_URL}/model-endpoints/usage?${qs.toString()}` | ||
| : `/api/model-endpoints/usage?${qs.toString()}` | ||
| const res = await fetch(url, { cache: "no-store" }) |
There was a problem hiding this comment.
PAPERBOT_API_BASE_URL is treated as including /api here (default is ...:8000/api), but Next.js proxy helpers (apiBaseUrl() in app/api/**) treat the same env var as the server root (default ...:8000). With a real env value, this can easily lead to double-missing or double-/api prefixes depending on where it’s used. Consider standardizing the env var to always be the root origin (no /api) and have lib/api.ts append /api in its path joins (or rename the env vars to make the contract unambiguous).
| for k in keys: | ||
| count = seen.get(k, 0) | ||
| seen[k] = count + 1 | ||
| result.append(k if count == 0 else f"{k}{chr(ord('a') + count)}") |
There was a problem hiding this comment.
_dedup_citation_keys says it appends a/b/c suffixes, but the implementation appends b for the first collision (because count starts at 1 for the first duplicate). Either change the algorithm to start with a on the first collision (more standard for BibTeX) or update the docstring/tests to match the intended suffix scheme.
| result.append(k if count == 0 else f"{k}{chr(ord('a') + count)}") | |
| result.append(k if count == 0 else f"{k}{chr(ord('a') + count - 1)}") |
| error=None, | ||
| ) | ||
| except Exception as exc: | ||
| latency_ms = int((time.monotonic() - t0) * 1000) |
There was a problem hiding this comment.
Variable latency_ms is not used.
| latency_ms = int((time.monotonic() - t0) * 1000) |
| title="Skipped Paper on Machine Learning Inference", | ||
| keywords=["machine learning", "inference"], | ||
| ) | ||
| neutral_pid = _insert_paper( |
There was a problem hiding this comment.
Variable neutral_pid is not used.
| neutral_pid = _insert_paper( | |
| _insert_paper( |
| import pytest | ||
|
|
There was a problem hiding this comment.
Import of 'pytest' is not used.
| import pytest |
| Base, | ||
| PaperFeedbackModel, | ||
| PaperModel, | ||
| ResearchTrackModel, |
There was a problem hiding this comment.
Import of 'Base' is not used.
Import of 'ResearchTrackModel' is not used.
| Base, | |
| PaperFeedbackModel, | |
| PaperModel, | |
| ResearchTrackModel, | |
| PaperFeedbackModel, | |
| PaperModel, |
| except Exception: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| pass | |
| except Exception as exc: | |
| logger.warning(f"keychain delete failed for {endpoint_name}: {exc}") |
| @@ -66,10 +67,13 @@ class EndpointTestRequest(BaseModel): | |||
|
|
|||
| class EndpointTestResponse(BaseModel): | |||
| ok: bool | |||
| def _dedup_citation_keys(keys: List[str]) -> List[str]: | ||
| """Append a/b/c suffixes when keys collide.""" | ||
| seen: Dict[str, int] = {} | ||
| result: List[str] = [] | ||
| for k in keys: | ||
| count = seen.get(k, 0) | ||
| seen[k] = count + 1 | ||
| result.append(k if count == 0 else f"{k}{chr(ord('a') + count)}") | ||
| return result |
There was a problem hiding this comment.
当前用于通过附加字符后缀(chr(ord('a') + count))来去重引用键的实现,在处理超过25个相同基本键的冲突时存在问题。如果 count 达到26或更高,它将开始附加非字母字符(例如 {, |),这可能导致无效的BibTeX键。虽然这是一个边缘情况,但更稳健的做法是处理它。例如,在'z'之后切换到数字后缀,或者使用类似Excel列的 a, b, ..., z, aa, ab, ... 风格的后缀。以下建议实现了后者,它能优雅地处理任意数量的冲突。此建议还将后缀序列更改为从 'a' 开始,这更为常规。
| def _dedup_citation_keys(keys: List[str]) -> List[str]: | |
| """Append a/b/c suffixes when keys collide.""" | |
| seen: Dict[str, int] = {} | |
| result: List[str] = [] | |
| for k in keys: | |
| count = seen.get(k, 0) | |
| seen[k] = count + 1 | |
| result.append(k if count == 0 else f"{k}{chr(ord('a') + count)}") | |
| return result | |
| def _dedup_citation_keys(keys: List[str]) -> List[str]: | |
| """Append a/b/c suffixes when keys collide.""" | |
| seen: Dict[str, int] = {} | |
| result: List[str] = [] | |
| for k in keys: | |
| count = seen.get(k, 0) | |
| seen[k] = count + 1 | |
| if count == 0: | |
| result.append(k) | |
| continue | |
| # Generate suffixes like a, b, ..., z, aa, ab, ... | |
| suffix = "" | |
| temp_count = count | |
| while temp_count > 0: | |
| temp_count, remainder = divmod(temp_count - 1, 26) | |
| suffix = chr(ord('a') + remainder) + suffix | |
| result.append(f"{k}{suffix}") | |
| return result |
Summary
Dashboard + Settings 破坏性重构,附带后端 Keychain 安全存储。
Dashboard
/api/model-endpoints/usage)Settings
Backend
KeychainStore:封装 keyring 库,API key 存入 macOS Keychainmodel_endpoint_store.py:upsert 走 Keychain(DB 存__keychain__标记),delete 清理 Keychain,_to_dict从 Keychain 读取Types/API
TimelineItem/SavedPaper类型,删除Activity/PipelineTask/ReadingQueueItemfetchActivities→ 真实 paper 数据转 timelinefetchSavedPapers替代fetchReadingQueuefetchLLMUsageSSR 走后端直连,CSR 走 Next.js proxy