fix(source-granola): stop dropping notes at incremental slice boundaries + add acceptance tests - #84277
Conversation
Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: bot_apk <apk@cognition.ai>
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
|
Note Autopilot progressive rollouts are not enabled for the following modified connector(s):
This is a courtesy heads-up only — it does not block merge or fail any check. |
|
Deploy preview for airbyte-docs ready!
Deployed with vercel-action |
|
|
Heads-up for reviewers: the newly enabled acceptance tests cannot pass yet because the test credentials this connector's The
Every other check on this PR is green (34 passed / 13 skipped), including connector lint, connector unit tests, metadata checks, format and docs build. Someone with a Granola Enterprise API key and access to the GSM test store needs to populate |
|
Cross-linking for the consolidation pass triaged by Full comparison and my suggestion — keep the unbounded cursor plus this PR's |
What
Related to https://github.com/airbytehq/airbyte-internal-issues/issues/16923:
Two scoped items from the
source-granolacertification epic — criterion I-2 (incremental correctness) and its prerequisite R-1 (standard tests never exercised the read path). Everything else in the epic (error handlers, backoff,airbyte_type, new streams,updated_aftercursor, metadata/support-level, docs, CODEOWNERS) is out of scope and stays with the other sub-issues.I-2 — notes on 30-day slice boundary dates were silently dropped. The
notescursor useddatetime_format: "%Y-%m-%d"withcursor_granularity: P1D, so withstep: P30Dan intermediate slice was emitted ascreated_after=2025-10-12&created_before=2025-11-10and the next slice started at2025-11-11. Granola'screated_before=<bare date>is exclusive with respect to that whole day (verified in the epic: that exact two-parameter probe returns 0 records on a day holding 2 notes), so every boundary day fell into an unqueried gap. Reproduced in the epic:start_date=2025-10-12emitted 133 of 142 notes, and the 9 missing notes fall exactly on the four boundary dates. Syncs reported success while dropping records.R-1 — the connector had no
acceptance-test-config.yml, socheck,discover,basic_readandfail_read_with_bad_catalogall skipped while the suite still exited 0 (2 passed, 6 skipped). That is why the record loss survived 11 releases.How
Cursor bounds become full timestamps at second granularity:
so the first slice now ends at
2025-11-10T23:59:59Zand the next starts at2025-11-11T00:00:00Z— no gap. The API's published OpenAPI spec acceptsdate-timeforcreated_after/created_before(https://docs.granola.ai/api-reference/list-notes).Two compatibility details are deliberate and are what keeps this non-breaking:
start_datetimeis left date-only.start_dateis a user-facing spec fieldpattern-validated asYYYY-MM-DD, andMinMaxDatetimeparses the config value with its owndatetime_format. Existing configs keep working untouched; the slice bounds are formatted with the cursor'sdatetime_format, so nothing is truncated."%Y-%m-%d"is added tocursor_datetime_formats.CustomFormatConcurrentStreamStateConverterbuilds its input parse formats fromcursor_datetime_formats+datetime_formatonly, so without this entry, state persisted by ≤0.2.11 (values like"2026-01-09") would fail withNo format in [...] matching 2026-01-09. Covered by a test.New
acceptance-test-config.ymlfollows the current nested convention used by comparable manifest-only sources (closest template:source-plaid) and enablesspec,connection,discovery,basic_read,incrementalandfull_refresh— nobypass_reason, noempty_streams. Supporting fixtures:invalid_config.json,abnormal_state.json, aconfigured_catalog_incremental.jsonfor the incremental test, and a correctedconfigured_catalog.json(it previously declared onlynoteswithcursor_field: ["updated_at"], a field that is not in the declarednotesschema; it now covers both streams). CI can fetchSECRET_SOURCE-GRANOLA__CREDSpermetadata.yaml, so the acceptance tests run there; they cannot run locally (no secret,airbyte-ciunavailable in the dev box).Declarative-First Evaluation
Fully declarative — the change is four lines inside the existing
DatetimeBasedCursor. Nocomponents.pyexists and none was added.Test Coverage
New
unit_tests/mock-server suite (modelled onsource-harvest, the established manifest-only precedent:unit_tests/pyproject.toml+poetry.lock+ CDKmanifest_only_fixtures), 3 tests passing:test_incremental_requests_cover_each_partition— frozen clock, a range spanning threeP30Dpartitions, and exactpage_size/created_after/created_beforematching for each (no permissive query matching), asserting the boundary day is covered with no gap.test_boundary_date_record_is_emitted— a note created2025-11-10T12:00:00Zis emitted.test_legacy_date_only_state_is_parsed_and_reformatted— incoming{"created_at": "2025-11-10"}(the ≤0.2.11 shape) syncs without error and the newly emitted state is2025-11-11T00:00:00Z.Verified they fail on the pre-fix manifest: tests 1 and 3 fail with
NoMockAddress: GET .../v1/notes?page_size=30&created_after=2025-10-12&created_before=2025-11-10(the truncated bounds), and test 2 fails withassert [] == ['boundary-note']— i.e. the dropped record.Breaking change evaluation
Not breaking. No spec change (no field added/removed/renamed;
start_datestill accepts the same values), no schema, primary-key or cursor-field change, no stream removed, and no reduction in the records a stream returns — the change only adds back records that were being dropped. The state format widens from2026-01-09to2026-01-09T00:00:00Z; old values still parse viacursor_datetime_formats, so no state migration is required. Version bumped0.2.11→0.2.12(patch); progressive rollout is disabled for this connector, so no-rcsuffix.Review guide
airbyte-integrations/connectors/source-granola/manifest.yaml— the fix (4 changed lines innotes.incremental_sync)airbyte-integrations/connectors/source-granola/unit_tests/test_notes.py— asserted slice boundsairbyte-integrations/connectors/source-granola/acceptance-test-config.yml+integration_tests/fixturesmetadata.yaml,docs/integrations/sources/granola.mdUser Impact
Notes created on a 30-day window boundary date are no longer skipped. Users on existing connections start receiving the previously-missed notes from the next sync onward for boundary dates at or after their current cursor position; notes already dropped in past syncs are not backfilled — recovering those needs a full refresh (or a
start_date/state reset). No reconfiguration is required and existing saved state keeps working.51 connections are on this connector (21 enabled), so please review the state-compatibility reasoning above with that blast radius in mind.
Can this PR be safely reverted and rolled back?
Reverting restores the previous (lossy) slicing. State written by 0.2.12 is second-precision ISO, which 0.2.11 also parses via its existing
cursor_datetime_formats, so a rollback does not break state.Link to Devin session: https://app.devin.ai/sessions/b1a2b93211fc4ee9a055cfa0ba7ae9ae