fix(source-granola): declare missing record fields and datetime types - #84282
Draft
devin-ai-integration[bot] wants to merge 3 commits into
Draft
fix(source-granola): declare missing record fields and datetime types#84282devin-ai-integration[bot] wants to merge 3 commits into
devin-ai-integration[bot] wants to merge 3 commits into
Conversation
Co-Authored-By: bot_apk <apk@cognition.ai>
Contributor
Author
🤖 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:
|
Contributor
👋 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
|
Contributor
|
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. |
Co-Authored-By: bot_apk <apk@cognition.ai>
Contributor
|
Deploy preview for airbyte-docs ready!
Deployed with vercel-action |
Co-Authored-By: bot_apk <apk@cognition.ai>
Contributor
|
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
2 participants
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.
What
source-granolacertification sub-issue S-2 + S-6: the two inline schemas inmanifest.yamlwere hand-written from a partial view of the API and never re-diffed against live records or the vendor spec, so (a) all sevenformat: date-timeproperties were missingairbyte_type, and (b) six record fields present in every live record were not declared at all. Both schemas setadditionalProperties: true, so those fields flowed through untyped, with no destination column and no schema contract.Resolves https://github.com/airbytehq/airbyte-internal-issues/issues/16930:
This is sequenced to land before the incremental sub-issue (airbytehq/airbyte-internal-issues#16931), which needs
notes.updated_atdeclared as its cursor field.Devin session: https://app.devin.ai/sessions/d25de47b024f4522b779cfed4e4786ab
How
Manifest-only change to the two inline schemas:
airbyte_type: timestamp_with_timezoneadded to everyformat: date-timeproperty (notes.created_at/updated_at;detailed_notes.created_at/updated_at,calendar_event.scheduled_start_time/scheduled_end_time,transcript[].start_time/end_time).spec.start_dateusesformat: dateand is untouched.notes.updated_at;detailed_notes.web_url;detailed_notes.space_membership;folder_membership[].parent_folder_idand.space_id;transcript[].speaker.attribution,.diarization_labeland.name.idremains the onlyrequiredproperty on both streams,additionalProperties: trueis unchanged, and nullable properties keep the file's existingtype: ["null", string]convention.Shapes were taken from live records, not guessed (see below).
space_membershipandfolder_membership[].space_idare not in Granola's published OpenAPI, so they are declared permissively — nullable, nothing required inside.Two deliberate omissions, both open to challenge:
web_urlis declared withoutformat: uri(the vendor declares it). Airbyte's destination type mappers only interpretdate,timeanddate-timestring formats; any otherformatis ignored with a warning, so adding it would produce log noise and no typing benefit.$ref'd shape forfolder/transcript. The parity sub-issue (airbytehq/airbyte-internal-issues#16932) will addfoldersandnote_transcriptsstreams that overlap these shapes. Factoring them out now would mean introducing an abstraction with a single consumer inside a PR that is otherwise purely additive; the second consumer arrives with that sub-issue, which is where the extraction belongs.metadata.autoImportSchemaleft atfalsefor both streams. It only affects Connector Builder drafts —useAutoImportSchema.tsin the webapp returnsfalseunless the displayed version is a draft — so it would not keep this repo-managed manifest in sync with the API, and flipping it would let a Builder round-trip overwrite these curated declarations.Review guide
airbyte-integrations/connectors/source-granola/manifest.yaml— the schema declarationsairbyte-integrations/connectors/source-granola/unit_tests/test_schema_declarations.py— the two regression testsmetadata.yaml+docs/integrations/sources/granola.md— version bump and changelogReproduction and live verification
Reproduced against the real Granola API (credentials from 1Password
direct-connector-credentials/granola; theSECRET_SOURCE-GRANOLA__CREDSsecret named inmetadata.yamldoes not exist indataline-integration-testing, which is worth fixing separately). Full-catalog read on master withairbyte/source-declarative-manifest:7.24.0: 136notes+ 136detailed_notesrecords, then every record key diffed recursively against the declared schemas.Show/Hide live evidence
Undeclared fields on master:
Observed shapes:
Across 41,953
transcript[].speakerobjects:source41,953,attribution40,816 (me/them),diarization_label1,137 (e.g.Speaker A), never both on one object.namewas never observed — it is declared from the vendorSpeakerschema, which documents it as present only when a speaker is identified.Datetime values all carry a timezone, which is what makes
timestamp_with_timezonethe right annotation: notes/transcript timestamps are Z-suffixed UTC (2025-11-10T22:00:41.213Z), and the twocalendar_eventones carry explicit offsets (2025-11-10T14:00:00-08:00). No nulls in any of the seven.After the fix, the same read against the same account reports zero undeclared fields in both streams.
Declarative-First Evaluation
This is a manifest-only (
language:manifest-only,cdk:low-code) connector and the fix is entirely declarative: the change is confined to the twoInlineSchemaLoaderschemas under the manifest's top-levelschemas:key. No custom Python component was written or needed — there is no runtime behaviour change at all, only schema declarations, soRecordFilter,AddFields/RemoveFields, transformations and the rest were not applicable.Test Coverage
New
unit_tests/package (the connector had none), modelled onsource-gocardless/unit_tests:test_schema_declarations_cover_mocked_records— mocks/v1/notesand/v1/notes/{id}with payloads modelled on the real records above, reads both streams, and asserts that the set of record paths absent from the declared schema is empty, walking nested objects and array items recursively.test_date_time_properties_declare_timezone_type— walks both schemas and asserts everyformat: date-timeproperty declaresairbyte_type: timestamp_with_timezone, so future datetime properties are covered too.Both fail on master (
assert {'updated_at'} == set()and the datetime assertion) and pass with this change:poe test-unit-tests→2 passed.poe test-integration-tests→2 passed, 6 skipped, unchanged from master (the 6 skips are acceptance scenarios that need theacceptance-test-config.ymltracked in airbytehq/airbyte-internal-issues#16924; the issue's stated8 passed, 0 skippedbaseline did not reproduce, on master either).Breaking change evaluation
Assessed as non-breaking → MINOR bump,
0.2.11→0.3.0. Noreleases.breakingChangesentry and no migration guide. The reasoning, so a reviewer can challenge it:airbyte_type: timestamp_with_timezoneto a property that already declaresformat: date-timedoes not change the destination column type. Every destination type mapper treats a missingairbyte_typeon adate-timeastimestamp_with_timezonealready —AirbyteProtocolType.kt:71-84(airbyteType == null || ... "timestamp_with_timezone"→TIMESTAMP_WITH_TIMEZONE),AirbyteJsonSchemaType.kt:137-141(null, "timestamp_with_timezone" -> TIMESTAMP_WITH_TIMEZONE), andJsonSchemaToAirbyteType.ktin the Bulk CDK. So these columns are already timestamps today; this change only makes the existing behaviour explicit.format/airbyte_typeis about adding or changing aformathint ({"type":"string"}→{"type":"string","format":"date-time"}), which flips a column from string to timestamp. Noformatis added, removed or changed here. Thesource-pinterestprecedent (🚨🚨🐛 Source Pinterest: Update date-time fields with airbyte_type: timestamp_without_timezone #32595) changed field types; this does not.additionalProperties: true. No field is removed or renamed, no type changes, primary key and cursor are untouched, no spec or state change. Users will need a schema refresh for the new columns to appear, which is the normal additive path.enableProgressiveRolloutisfalse, so no-rc.Nsuffix.User Impact
notes.updated_at,detailed_notes.web_urlanddetailed_notes.space_membership(plus the nested folder/speaker fields) become declared, typed columns after a schema refresh instead of untyped passthrough data. The seven datetime columns are unchanged in behaviour — the annotation documents what destinations already do.Can this PR be safely reverted and rolled back?