fix(connectors): notion/readwise parsers handle single-page + empty + batched shapes#83
Closed
KooshaPari wants to merge 1 commit into
Closed
fix(connectors): notion/readwise parsers handle single-page + empty + batched shapes#83KooshaPari wants to merge 1 commit into
KooshaPari wants to merge 1 commit into
Conversation
… batched shapes (FR-NOTION-API-002/003/004/006, FR-READWISE-API-002/003/004/006) NotionPage/NotionTask and Article/Highlight parsers now accept: - vendor page-shaped single objects (not just `results:[]` batches) - empty / wrong-shape payloads as empty list (no panic, no silent drops) - any unknown `created_at`/`updated_at`/`url` field becomes empty string instead of panicking on `?` This unblocks the autograder that previously failed with `assertion failed: !pages.is_empty()`. Also adds a journey manifest and traceability doc so the FR↔test mapping is explicit and the user story is recorded. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Review limit reached
More reviews will be available in 44 minutes and 5 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
codeant-ai
Bot
added
the
size:L
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+48
to
+52
| last_edited_time: page | ||
| .get("last_edited_time") | ||
| .and_then(|t| t.as_str()) | ||
| .unwrap_or_default() | ||
| .into(), |
There was a problem hiding this comment.
Suggestion: Defaulting a missing page edit timestamp to an empty string breaks the downstream event contract: map_pages parses this field as RFC3339 and falls back to Utc::now() on failure, so the dedupe key changes every sync and duplicate page_updated events are emitted for the same page. Keep a stable timestamp fallback (for example, reuse created_time or skip emitting pages without a valid edit time) instead of "". [api mismatch]
Severity Level: Major ⚠️
- ❌ Notion page_updated events re-emitted for unchanged pages.
- ⚠️ Dedupe keys unstable when page edit time missing.
- ⚠️ Downstream consumers may see duplicate page updates.Steps of Reproduction ✅
1. Trigger a Notion sync via `NotionConnector::sync` in
`crates/connector-notion/src/lib.rs:2-30`, which constructs a `NotionClient` and
`NotionEventMapper` and calls `client.get_pages().await`.
2. In `NotionClient::get_pages` (`crates/connector-notion/src/api.rs:11-32`), have the
HTTP response body deserialize into a `serde_json::Value` representing at least one page
whose `last_edited_time` field is missing or null (this can be reproduced directly in
tests by constructing such a JSON and calling `NotionPage::from_notion_json`).
3. `NotionPage::from_notion_json` in `crates/connector-notion/src/models.rs:17-60` builds
a `NotionPage` where `last_edited_time` is set via
`page.get("last_edited_time").and_then(|t| t.as_str()).unwrap_or_default().into()`, so the
missing/invalid field yields `last_edited_time == ""`.
4. The connector then calls `mapper.map_pages(pages)` in
`crates/connector-notion/src/lib.rs:9-12`, which executes `map_pages` in
`crates/connector-notion/src/events.rs:5-41`; there `edited_at` is parsed with
`DateTime::parse_from_rfc3339(&p.last_edited_time).unwrap_or_else(|_| Utc::now())`, so
`""` causes a parse error and substitutes `Utc::now()`, and `EventFactory::new_dedupe_key`
in `crates/focus-events/src/lib.rs:160-165` builds a dedupe key from this changing
timestamp, resulting in a different dedupe key (and a new `page_updated` event) on every
sync for the same page.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/connector-notion/src/models.rs
**Line:** 48:52
**Comment:**
*Api Mismatch: Defaulting a missing page edit timestamp to an empty string breaks the downstream event contract: `map_pages` parses this field as RFC3339 and falls back to `Utc::now()` on failure, so the dedupe key changes every sync and duplicate `page_updated` events are emitted for the same page. Keep a stable timestamp fallback (for example, reuse `created_time` or skip emitting pages without a valid edit time) instead of `""`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+127
to
+131
| last_edited_time: task | ||
| .get("last_edited_time") | ||
| .and_then(|t| t.as_str()) | ||
| .unwrap_or_default() | ||
| .into(), |
There was a problem hiding this comment.
Suggestion: Defaulting task last_edited_time to an empty string causes map_tasks to fail RFC3339 parsing and substitute Utc::now(), which makes dedupe keys unstable across polls and can repeatedly re-emit the same completed task. Use a stable fallback timestamp (or avoid emitting tasks missing a parseable edit time) rather than an empty string. [api mismatch]
Severity Level: Major ⚠️
- ❌ Notion task_completed events duplicated for unchanged tasks.
- ⚠️ Dedupe keys unstable when task edit time missing.
- ⚠️ Task-based automations may trigger multiple times.Steps of Reproduction ✅
1. Trigger a Notion sync via `NotionConnector::sync` in
`crates/connector-notion/src/lib.rs:2-30`, which calls `client.get_tasks().await` and then
`mapper.map_tasks(tasks)`.
2. In `NotionClient::get_tasks` (`crates/connector-notion/src/api.rs:45-75`), deserialize
the HTTP response into JSON where a task page lacks a valid `last_edited_time` field
(missing, null, or non-string); this can be reproduced by constructing such JSON and
passing it to `NotionTask::from_notion_json`.
3. `NotionTask::from_notion_json` in `crates/connector-notion/src/models.rs:91-135` sets
`last_edited_time` using `task.get("last_edited_time").and_then(|t|
t.as_str()).unwrap_or_default().into()`, so invalid/missing values become the empty string
on the `NotionTask`.
4. `map_tasks` in `crates/connector-notion/src/events.rs:45-79` filters to completed tasks
and then computes `edited_at` via
`DateTime::parse_from_rfc3339(&t.last_edited_time).unwrap_or_else(|_| Utc::now())`; for
`last_edited_time == ""` this falls back to `Utc::now()`, and
`EventFactory::new_dedupe_key` (`crates/focus-events/src/lib.rs:160-165`) incorporates
this changing timestamp, so the same completed task with no edit time produces different
dedupe keys and repeated `task_completed` events across syncs.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/connector-notion/src/models.rs
**Line:** 127:131
**Comment:**
*Api Mismatch: Defaulting task `last_edited_time` to an empty string causes `map_tasks` to fail RFC3339 parsing and substitute `Utc::now()`, which makes dedupe keys unstable across polls and can repeatedly re-emit the same completed task. Use a stable fallback timestamp (or avoid emitting tasks missing a parseable edit time) rather than an empty string.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+36
to
+40
| updated_at: doc | ||
| .get("updated_at") | ||
| .and_then(|t| t.as_str()) | ||
| .unwrap_or_default() | ||
| .into(), |
There was a problem hiding this comment.
Suggestion: Falling back updated_at to "" for articles breaks downstream assumptions in map_articles, which parses it as RFC3339 and otherwise uses Utc::now(); this creates a different dedupe key on each sync for the same article. Use a stable fallback like created_at/added_at or skip records lacking a parseable update timestamp. [api mismatch]
Severity Level: Major ⚠️
- ❌ Readwise article_read events duplicated for unchanged articles.
- ⚠️ Dedupe keys unstable when article updated_at missing.
- ⚠️ Analytics on article reads can be inflated.Steps of Reproduction ✅
1. Trigger a Readwise sync via the connector in
`crates/connector-readwise/src/lib.rs:118-21`, which calls `client.get_articles().await`
and then `mapper.map_articles(articles)`.
2. In `ReadwiseClient::get_articles` (`crates/connector-readwise/src/api.rs:3-28`),
deserialize the `/documents` response into JSON where an article lacks a valid
`updated_at` string (missing, null, or wrong type); this can be reproduced by constructing
such JSON and passing it to `Article::from_readwise_json`.
3. `Article::from_readwise_json` in `crates/connector-readwise/src/models.rs:18-44` sets
`updated_at` using `doc.get("updated_at").and_then(|t|
t.as_str()).unwrap_or_default().into()`, so the model's `updated_at` is `""` when the
source timestamp is absent or invalid.
4. `map_articles` in `crates/connector-readwise/src/events.rs:45-80` parses `a.updated_at`
with `DateTime::parse_from_rfc3339(&a.updated_at).unwrap_or_else(|_| Utc::now())`; for
`""` it falls back to `Utc::now()`, and `EventFactory::new_dedupe_key`
(`crates/focus-events/src/lib.rs:160-165`) uses this timestamp, so each sync produces a
new dedupe key and additional `readwise:article_read` events for the same logical article.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/connector-readwise/src/models.rs
**Line:** 36:40
**Comment:**
*Api Mismatch: Falling back `updated_at` to `""` for articles breaks downstream assumptions in `map_articles`, which parses it as RFC3339 and otherwise uses `Utc::now()`; this creates a different dedupe key on each sync for the same article. Use a stable fallback like `created_at`/`added_at` or skip records lacking a parseable update timestamp.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+86
to
+90
| created_at: h | ||
| .get("created_at") | ||
| .and_then(|t| t.as_str()) | ||
| .unwrap_or_default() | ||
| .into(), |
There was a problem hiding this comment.
Suggestion: Defaulting highlight created_at to an empty string causes map_highlights to fall back to Utc::now() during timestamp parsing, so identical highlights get new dedupe keys on each run and can be re-ingested repeatedly. Use a stable timestamp fallback (for example updated_at) or drop highlights without a valid creation timestamp. [api mismatch]
Severity Level: Major ⚠️
- ❌ Readwise highlight_created events duplicated for same highlight.
- ⚠️ Dedupe keys unstable when highlight created_at missing.
- ⚠️ Downstream consumers may reprocess identical highlights.Steps of Reproduction ✅
1. Trigger a Readwise sync via the connector in
`crates/connector-readwise/src/lib.rs:118-21`, which first calls
`client.get_highlights().await` and then `mapper.map_highlights(highlights)`.
2. In `ReadwiseClient::get_highlights` (`crates/connector-readwise/src/api.rs:31-59`),
deserialize the `/highlights` response into JSON where a highlight has no valid
`created_at` string; this can also be reproduced in tests by constructing such JSON and
calling `Highlight::from_readwise_json`.
3. `Highlight::from_readwise_json` in `crates/connector-readwise/src/models.rs:71-99` sets
`created_at` via `h.get("created_at").and_then(|t|
t.as_str()).unwrap_or_default().into()`, so missing/invalid timestamps become the empty
string on the `Highlight`.
4. `map_highlights` in `crates/connector-readwise/src/events.rs:5-41` computes
`created_at` for the event with
`DateTime::parse_from_rfc3339(&h.created_at).unwrap_or_else(|_| Utc::now())`; when
`h.created_at == ""`, it uses the current time, and `EventFactory::new_dedupe_key`
(`crates/focus-events/src/lib.rs:160-165`) uses this time, so repeated syncs over the same
highlight with missing `created_at` generate different dedupe keys and multiple
`readwise:highlight_created` events.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/connector-readwise/src/models.rs
**Line:** 86:90
**Comment:**
*Api Mismatch: Defaulting highlight `created_at` to an empty string causes `map_highlights` to fall back to `Utc::now()` during timestamp parsing, so identical highlights get new dedupe keys on each run and can be re-ingested repeatedly. Use a stable timestamp fallback (for example `updated_at`) or drop highlights without a valid creation timestamp.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
3 tasks
Owner
Author
|
CI status update: the |
Owner
Author
|
Closing as redundant: PR #82 ( |
|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
KooshaPari
added a commit
that referenced
this pull request
Jun 13, 2026
* Add root STATUS.md * chore: add CITATION.cff for GitHub citation support Enables "Cite this repository" feature on GitHub and provides machine-readable metadata for academic/tooling use. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(Planify): add standard bug_report.md issue template GitHub's new-issue chooser only matches canonical filenames (bug_report.md / feature_request.md). The existing --bug-report.yaml files in .github/ISSUE_TEMPLATE/ are silently ignored, so contributors land on a blank form. Adds .github/ISSUE_TEMPLATE/bug_report.md so the chooser offers a structured bug report form. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Revert "chore(Planify): add standard bug_report.md issue template" This reverts commit 71ca6dd. * chore: add OpenSSF Scorecard workflow Adds .github/scorecard.yml running ossf/scorecard-action weekly and on main pushes, uploading SARIF to code-scanning for supply-chain security visibility. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore: clean up audit artifacts and deleted files * chore: clean up audit artifacts and deleted files * fix(repos): restore corrupted root STATUS.md The previous root STATUS.md contained a single junk line that was a mixture of a `wtmp` system log and a relative link: wtmp begins Mon Jun 16 08:38:50 MST 2025phenotype-org-governance/SUPERSEDED.md Replace it with a proper monorepo status index: header, last-updated date, a 3-bullet summary of recent work, and links into the sub-project groups (apps, shared libraries, services, tooling, worktrees). * chore(audit-2026-06-10): V3 DAG and audits * chore: record l5-91 stash cleanup * docs(findings): add MERGE_DASHBOARD_2026_06_11.md with batch-merge results * docs(plans): update 100-task DAG v3 with resume progress + 100+ PR merge results * merge(2026-06-11): unify focus repo state — all L1-L5 agent branches merged Sequential --no-ff merges in dependency order (L2-21/22/24/25 → L2-28..35 → L3-41..45 → L4-61..71 → L5-83..92) of 55 chore/l* branches across 5 focus repos. Conflicts on shared .editorconfig / .github/* files resolved with -X theirs (later agents had more complete content). PhenoCompose: 11 branches were pre-consolidation (the 2026-06-08 commit 1936a4c dropped 3,373 LOC of duplicate Go). Cherry-picked the new artifacts (dependabot.yml, 9 .github/workflows/*.yml, gitleaks.toml, trufflehog.yml, .pre-commit-config.yaml, renovate.json5) and reverted the initial L2-33 cherry-pick that re-introduced deleted Go code. Final state per repo: - AgilePlus (main, fe033adf): 12 branches merged, 1 stale (chore/license) - PlayCua (master, 65ccfc4): 15/15 merged (incl. L4-70 bare-cua→playcua rename) - nanovms (main, 0fd3307): 11/11 merged - PhenoCompose (main, 82f579c): 3 merged + 7 cherry-pick commits, 11 pre-consolidation - BytePort (main, 61a9497): 14/14 merged V3_EXECUTION_LOG_2026_06_10.md updated with full Phase 2 report (inventory, method, conflict analysis, tooling findings, PhenoCompose consolidation handling, key learnings for ANSI/gitconfig issues). * merge(2026-06-11): finalize focus repo state with submodule pointer updates - AgilePlus: 2 new commits on main (L2 #31 SHA pin + L3 #45 docs) - 386493084 SHA-pin 14 workflow files (fixes -X theirs that stripped pins) - 958629185 add CARGO-WORKSPACE.md + 22 per-crate README public API indexes - nanovms: 1 new commit (cherry-pick L2 #31 SHA pin) - V3_EXECUTION_LOG: updated Phase 2 report Other 105+ submodule modifications left in working tree (pre-existing unmerged work from 2026-06-08 sessions). To be unified in a separate pass per the V3 plan: 'project-first, then cross-project libification and SOTA sweep.' * chore(repos): update AgilePlus pointer to include recovery script commit Recovery Script 1106ef5c9 (docs: add cargo workspace API index) added an empty placeholder commit on AgilePlus main. The substantive docs work was already in 958629185 (CARGO-WORKSPACE.md + per-crate README public API indexes); this is a downstream noop marker. * chore(exec-log-2026-06-11): Phase 3 build verification report All 5 focus repos verified buildable post-merge: - AgilePlus: cargo check OK (38.6s, 22 crates) - PlayCua: cargo check OK (36.3s, 54 dead-code warnings from L4-70 hex port declarations - intentional SOTA declare-then-implement pattern) - nanovms: go build OK - PhenoCompose: VitePress docs site (consolidation absorbed Go code) - BytePort: cargo check OK (28.5s, Tauri+Electron) * chore(report-2026-06-11): final session report Comprehensive 163-line session report documenting: - 50 agent branches merged across 5 focus repos - 17 merge conflicts resolved (mostly -X theirs for shared CI files) - PhenoCompose pre-consolidation special case (11 stale branches + 7 cherry-pick commits) - Build verification (4 cargo check OK, 1 go build OK, 1 N/A VitePress) - 5 SOTA patterns observed (hex arch, public-API READMEs, SHA-pinned actions, etc.) - Tooling findings (ANSI stripping workarounds, gitconfig fix, conflict strategy) - All files created/updated across root and focus repos - Next phase: SOTA sweep (cancelled due to codex exec credit exhaustion) * chore(worklog-2026-06-11): fix worklog schema gap + add agileplus-worklog wrapper Phase 4 of the V3 unification plan. The 9 agent-produced worklog JSON files did not match the canonical 8-field schema in WORKLOG_SCHEMA_2026_06_10.md — fields like 'task' (vs task_id), 'files' (vs files_changed), 'branch' (vs commit_sha) were used inconsistently across L2-029, L2-033, and L4-070 worklogs. Fix: - worklog-converter.py: normalizes any worklog JSON to canonical schema - /Users/kooshapari/bin/agileplus-worklog: 4 subcommands (validate, convert, schema, list) — temporary wrapper until the Rust CLI gets a native 'worklog' subcommand - 9 canonical worklog-*-canonical.json files created across AgilePlus (2), PlayCua (3), nanovms (2), BytePort (2) - V3_EXECUTION_LOG updated with Phase 4 report * feat(pheno-errors): new crate with AppError + 5 variants (L3 #46) V3 DAG L3 #46 — author the canonical pheno-errors Rust crate consolidating the 5 most-common error patterns observed across the L1/L2 fleet audit (2026-06-10). Variants (exact L3 DAG spec): - AppError::Domain(String) - AppError::NotFound { entity: String, id: String } - AppError::Conflict(String) - AppError::Validation(String) - AppError::Storage(String) Pattern: thiserror derive + anyhow::Context interop. From impls for std::io::Error (-> Storage), anyhow::Error (-> Domain with full chain walk), &str/String (-> Domain). No blanket From<E: Error> impl because that conflicts with the concrete std::io::Error impl under Rust's coherence rules. Standalone package via empty [workspace] table in its own Cargo.toml so the new crate does NOT pull the 56+ focus/connector crates in the root FocalPoint/Phenotype workspace into the build. Consumers add it to their own workspace or depend on it via a path/git dep. 14/14 tests pass (8 inline + 6 smoke). Clippy clean. rustfmt clean. Branch: chore/l3-46-pheno-errors-2026-06-11 (local-only, not pushed, per task directive). Consumed by L5 #81-85 across the pheno-* fleet. * chore(repos): absorb worklog gap fix and canonical worklog commits Pointer updates for 4 focus repos (AgilePlus, PlayCua, nanovms, BytePort): - AgilePlus 41a98f44: feat(cli) add worklog subcommand - PlayCua b25d148: chore(worklog) add canonical-form worklogs (3) - nanovms d74fae7: chore(worklog) add canonical-form worklogs (2) - BytePort bb16e29: chore(worklog) add canonical-form worklogs (2) PhenoCompose unchanged (no worklogs were produced for it; the agent work went into the L2-29..L2-35 cherry-pick commits directly). * chore(exec-log-2026-06-11): Phase 5 - native worklog subcommand + push status Phase 5: native Rust subcommand 'agileplus worklog' added to agileplus-cli (replaces the wrapper script at /Users/kooshapari/bin/agileplus-worklog). 4 subcommands: schema, list, validate, convert. 9 canonical worklogs validated across 5 focus repos. Push status documented (3 options for next session: LFS fetch, allow-incomplete-push, fresh fork). * feat(pheno-tracing): new crate with init/init_json/init_with_file (L3 #47) V3 DAG L3 task #47: author the canonical pheno-tracing crate that consolidates the tracing-subscriber + EnvFilter + tracing-appender init patterns previously duplicated across focus-observability and other consumers into a one-liner: `pheno_tracing::init()`. Public API: - pheno_tracing::init() - pretty console + RUST_LOG (default info) - pheno_tracing::init_json() - structured JSON output - pheno_tracing::init_with_file - daily-rotated log file appender All three are process-level idempotent (try_init) and read RUST_LOG for the EnvFilter directive. Consumed by L5 #81-85 (focus-repo integration). Workspace membership registered in root Cargo.toml. cargo test -p pheno-tracing passes 9/9 (2 unit + 6 integration + 1 doctest). Refs: FLEET_100TASK_DAG_V3.md L3 #47 * chore(l3-47): canonical worklog + V3 exec log entry L3 subagent #47: ship the canonical worklog for the pheno-tracing crate authoring task and the matching V3_EXECUTION_LOG entry. Worklog: worklogs/l3-47-pheno-tracing-2026-06-11.json (16 fields: task_id, date, title, status, branch, commit, author, refs, summary, scope, public_api, dependencies, test_results, test_coverage, idempotency, consolidation_targets, downstream_consumers, verification, notes, no_touch). V3 exec log: appends the L3 subagent #47 Updates summary plus a detailed L3 #47 section mirroring the L3 #46 template (branch, crate layout, public API, internal helper, workspace registration, idempotency, test isolation, verification, files table, constraints, downstream). Companion to commit 3aecb78. * chore(monorepo): harvest 6 V4 launch agent outputs as audit files * docs(scaffold-kit): end-to-end smoke test against 2 pheno-* repos * feat(pheno-config): author canonical config loader (L3 #48) V3 DAG L3 #48 — author the canonical pheno-config Rust crate consolidating the env-var / JSON-file / programmatic config loading patterns previously duplicated across the L1/L2 fleet into a single typed Config { url, port, log_level, db_path, feature_flags }. Public API (3 entry points): - pheno_config::load_from_env(prefix) — reads <PREFIX>_* env vars (URL, DB_PATH required; PORT, LOG_LEVEL, FEATURE_FLAGS optional with sensible defaults: port=8080, log_level="info", flags empty). Unrelated env vars (no prefix match) are filtered out, verified by load_from_env_with_prefix_filters_unrelated_vars. - pheno_config::load_from_file(path) — JSON via serde_json. IoError on read failure, ParseError on malformed JSON or type mismatch, MissingField on absent required keys (best-effort extracted from serde_json's "missing field `name`" diagnostic). - pheno_config::ConfigBuilder — programmatic with defaults port=8080, log_level="info", feature_flags=Vec::new(). ConfigError is a closed 3-variant enum (thiserror derive): MissingField(String), ParseError{field, message}, IoError(#[from] std::io::Error). Deliberately closed (no #[non_exhaustive]) so downstream match exhaustiveness checks are useful. No blanket From<E: Error> impl to avoid Rust coherence conflicts with the concrete io::Error impl. 12/12 tests pass (10 integration + 2 doctest). The 6 L3 #48 spec-named tests are all present in tests/config_test.rs verbatim: load_from_env_with_prefix_filters_unrelated_vars, load_from_env_defaults_port_8080, load_from_file_valid_json, load_from_file_missing_file_returns_io_error, builder_sets_defaults, missing_required_field_returns_missing_field_error. 4 additional tests round out the contract: load_from_file_missing_required_field_returns_missing_field_error, load_from_env_invalid_port_returns_parse_error, load_from_file_malformed_json_returns_parse_error, config_error_display_is_informative. Clippy clean (no warnings). Standalone package via empty [workspace] table in its own Cargo.toml so the new crate does NOT pull the 57+ focus/connector crates in the root FocalPoint/Phenotype workspace into the build. Mirrors the L3 #46 (pheno-errors) pattern. Consumed by L5 #81–85. Branch chore/l3-48-pheno-config-2026-06-11, off chore/l3-47-pheno-tracing-2026-06-11. Per task directive: local-only, NOT pushed. Refs: FLEET_100TASK_DAG_V3.md#L3-#48 Downstream: L5-#81, L5-#82, L5-#83, L5-#84, L5-#85 * chore(l3-48): canonical worklog + V3 exec log entry L3 subagent #48: ship the canonical worklog for the pheno-config crate authoring task and the matching V3_EXECUTION_LOG entry. Worklog: worklogs/l3-48-pheno-config-2026-06-11.json matching the schema in WORKLOG_SCHEMA_2026_06_10.md (8 required top-level fields: status, task_id, agent_id, files_changed, commit_sha, verification_result, started_at, completed_at — plus branch and notes). verification_result.commands lists the two exact commands run for green-build evidence: `cargo test --manifest-path pheno-config/Cargo.toml --offline` and `cargo clippy --manifest-path pheno-config/Cargo.toml --offline --all-targets -- -D warnings`. V3 exec log: appends the L3 subagent #48 Updates summary plus a detailed L3-#48 (pheno-config) section mirroring the L3-#47 template (branch, crate layout, public API, ConfigError variants, test coverage table, constraints respected, downstream). The section reflects the canonical spec layout: tests in `tests/config_test.rs` (per the L3 #48 spec directive) rather than inline in `src/lib.rs`. The `commit_sha` references the feat commit `4ff33e7d7f`. Companion to commit 4ff33e7. * feat(pheno-config): commit FLEET_DAG.db from worktree (L3 #48) --------- Co-authored-by: DAG-Audit <audit@phenotype.local> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Phenotype Agent <agent@phenotype.local> Co-authored-by: Forge <forge@phenotype.local> Co-authored-by: Koosha Pari <koosha@phenotype.com>
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.
User description
Problem
cargo test -p connector-notionwas failing 3 tests withassertion failed: !pages.is_empty()because the NotionPage/NotionTask parsers only handledresults:[]batches and required all timestamp fields to be present. Same shape issue existed inconnector-readwisefor Article/Highlight.Fix
results:[]batches.created_at/updated_at/urlfall back to empty string instead of?-panicking.readwise_items/notion_titlehelpers so the four parsers stay consistent.Coverage (FR → test, autograder table)
connector_notion::api::tests::parse_page_responseconnector_notion::api::tests::parse_task_responseconnector_notion::api::tests::parse_multiple_pagesconnector_notion::api::tests::parse_empty_pagesconnector_readwise::api::tests::parse_article_responseconnector_readwise::api::tests::parse_highlight_responseconnector_readwise::api::tests::parse_multiple_articlesconnector_readwise::api::tests::parse_empty_articlesCI Gate
This is the autograder for the user story. See
docs/reference/connectors-notion-readwise-parse.mdand the new journey manifest atdocs/journeys/manifests/connectors-notion-readwise-parse.jsonfor full traceability.Test Plan
cargo test -p connector-notion -p connector-readwise→ 26 passed, 0 failedcrates/.../models.rsandcrates/.../api.rstrace comments🤖 Generated with Claude Code
Note
Medium Risk
Changes sit on the connector ingestion boundary used by all downstream sync; behavior shifts (empty strings vs dropped rows) could affect data completeness but is covered by targeted tests.
Overview
Fixes Notion and Readwise connector JSON parsing so sync ingestion accepts vendor single-object responses as well as
results:[]batches, instead of returning empty lists when tests (and some API calls) pass a lone page or article.Notion (
NotionPage/NotionTask): Normalizes input via sharednotion_title, which scans any title-type property and readsplain_textor nestedtext.content. Single pages are detected whenobject == "page". Timestamps andurldefault to empty strings when missing so rows are not dropped by?infilter_map.Readwise (
Article/Highlight): Sharedreadwise_itemstreats a top-level JSON object as one item when there is noresultsarray.created_atfalls back toadded_at;document_idand timestamps use defaults instead of failing the whole record.Adds a journey manifest and reference doc tying FR IDs to the existing
connector_*::api::testsautograder and CI gatecargo test -p connector-notion -p connector-readwise.Reviewed by Cursor Bugbot for commit d9ea52e. Bugbot is set up for automated code reviews on this repo. Configure here.
CodeAnt-AI Description
Handle single-item and batched Notion and Readwise payloads without failing
What Changed
resultslists.added_atwhencreated_atis missing.Impact
✅ Fewer connector parse crashes✅ Successful syncs for single-item API responses✅ Clearer test coverage for Notion and Readwise ingestion💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.