feat(source-ashby): declare documented API fields across stream schemas - #84274
feat(source-ashby): declare documented API fields across stream schemas#84274devin-ai-integration[bot] wants to merge 7 commits into
Conversation
Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
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:
|
👋 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
|
Co-Authored-By: bot_apk <apk@cognition.ai>
|
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 |
There was a problem hiding this comment.
remove these tests, unneeded
There was a problem hiding this comment.
👍 On it. Removing the whole unit_tests/ directory — they only assert that the manifest's declared properties match a hardcoded field list, which is a restatement of the diff rather than a behavioral test. Will push shortly and leave the manifest/metadata/changelog changes as-is.
There was a problem hiding this comment.
☑️ Resolved in f58469f. unit_tests/ removed; PR description updated so it no longer claims test coverage.
Co-Authored-By: bot_apk <apk@cognition.ai>
|
↪️ Triggering Reason: Draft schema fix with CI green (only Progressive Rollout Gate checks were cancelled); prove-fix validation is the next pipeline step for: https://github.com/airbytehq/oncall/issues/13283 |
|
✅ Fix Proven —
|
| # | Setup | Outcome |
|---|---|---|
| 1 | GSM secrets, all streams, comparison mode — run | ⚙️ Infrastructure failure, not a verdict. CHECK aborted with Failed to fetch integration test config from GSM for source-ashby → ValueError: Config is required for check command. source-ashby has no GSM integration-test secret, so the GSM path cannot test this connector at all. |
| 2 | Live connection (Connection A — TIER_2, non-EU, unpinned), 10 selected streams, warm read — run | SPEC ✅ · CHECK ✅ · DISCOVER ❌ (13 streams changed, additive_only=false) · READ ❌ (candidates, 85 records, school only) |
| 3 | Same connection, repeat run — run | Same shape, but candidates value diffs 14 records — again school only. An identical setup producing 85 then 14 is the first signal that school is run-to-run noise. |
| 4 | Discriminating run: identical connector code on both sides (control overridden to the 0.4.0 pre-release) — run |
DISCOVER ✅ Discovered catalog unchanged across 17 streams · READ ❌ candidates 94 records still differ, school only |
Finding 1 — the catalog change is exactly the intended additive declaration. In the attempt-2 DISCOVER diff, all 13 changed streams report only dictionary_item_added — zero dictionary_item_removed, zero type_changes, zero values_changed. The check still renders ❌ because additive_only is a whole-check verdict and this PR adds properties / items to fields that already existed (candidate → candidate.properties, hiringTeam → hiringTeam.items), which the harness scores as re-declaration rather than growth. Since every stream keeps additionalProperties: true and no additionalProperties: false is introduced, declaring subfields cannot drop data.
Finding 2 — the candidates READ failure is not this PR. Attempt 4 ran the same build against itself and still reported 94 differing candidates records, exclusively on root['data']['school']. Across three live runs the count was 85 → 14 → 94, and no other field ever appeared. A schema-only PR has no code path that could rewrite a field value, so the difference comes from the API returning school non-deterministically between the sequential control and target reads.
Everything else clean, on every live run:
SPEC✅ — no spec change; no saved config becomes invalid.CHECK✅ — both versions connect.- Record counts identical target vs control on all 8 read streams (largest:
applications21,905 andcandidates20,747) — no decrease anywhere, andrecords_only_in_control= 0. pk_presence✅ (0 missing, 0 extra),pk_uniqueness✅ on both control and target.- Final state ✅
Final state unchanged across 10 streams. - The fix was exercised, not merely un-regressed: the new declarations appear on 13 streams in
DISCOVER, and the read covered 10 of them against live data.
Customer-identifying details for the connection used are recorded privately in the originating internal issue and deliberately omitted here.
Airbyte Support Bot (airbyte-support-bot)
added
the
hyd-prove
|
Co-Authored-By: bot_apk <apk@cognition.ai>
3 participants
What
Requested by Patrick Nilan (@pnilan) via
/ai-fixon airbytehq/airbyte-internal-issues#16922 (audit finding S-1: 49 documented Ashby API fields undeclared across 10 stream schemas).source-ashbydeclares its stream schemas inline inmanifest.yaml. Comparing them against Ashby's published OpenAPI definitions showed a large set of documented top-level fields that were never declared, plus many objects/arrays declared as baretype: object/type: arraywith noproperties/items. Since every stream setsadditionalProperties: truethose fields still pass through at runtime, but they are absent from the catalog — untyped, not selectable in the UI, and not guaranteed in destinations.Field lists were re-derived independently of the audit, from the machine-readable OpenAPI blocks Ashby publishes on each reference page (e.g. https://developers.ashbyhq.com/reference/userlist.md), rather than taken from the issue text. That re-derivation matched the audit and also surfaced four fields the audit missed on
interview_schedules(createdAt,updatedAt,scheduledBy,interviewEvents) and five extra onjobs(createdAt,updatedAt,openedAt,closedAt,brandId, which the audit flagged for verification).How
Declared the documented-but-missing top-level fields on 12 streams:
applicationsopeningscandidateslocationcustom_fieldsdescription,isPrivate,isRequired,selectableValuesdepartmentscreatedAt,externalName,extraData,updatedAtinterview_schedulescreatedAt,updatedAt,scheduledBy,interviewEventsinterview_stagesinterviewStageGroupIdjobsauthor,brandId,closedAt,compensation,createdAt,location,openedAt,openings,updatedAtjob_postingsapplicationDeadline,applyLink,compensationTierSummary,shouldDisplayCompensationOnJobBoard,status,updatedAt,workplaceTypelocationsexternalName,extraData,parentLocationId,type,workplaceTypeoffersformDefinition,offerStatus,versionsuserscustomFields,globalRole,isEnabled,managerId,updatedAtapplication_criteria_evaluationsid,criterion,status,skipReason,outcomeNumber,evaluatedAtEverything added is nullable (
type: ["null", <type>]) to match the existing convention, withformat: date-time/format: datewhere documented.Previously-bare nested objects/arrays were expanded one level: their documented child properties are declared with scalar types, and any grandchild object/array stays bare. This is a deliberate bound to keep the change reviewable — e.g.
candidates.emailAddresses.itemsnow declaresvalue/type/isPrimary, butoffers.latestVersion.salarystays a bare object.Polymorphic fields are declared as the full documented union rather than one branch:
customFields[].valueis["null", "boolean", "number", "string", "array", "object"]andcustomFields[].valueLabelis["null", "string", "array"](Ashby documents both asoneOf). Declaring one branch would have been worse than leaving them undeclared.Deliberately not changed
interviews— untouched. Its declared schema and the documentedinterview.listobject share onlyid; the declared shape (applicationId,interviewScheduleId,startTime,meetingLink, …) looks like a scheduled-interview object, not an interview definition. Either the schema was derived from a different resource or the docs and live response have diverged. Adding the 11 documented fields here without a live response to check against would likely produce 11 permanently-null columns. Needs live verification first.interview_stages.isArchived,application_criteria_evaluations.assessmentType/criterionName/jobId) — they presumably came from real responses.primary_keychanged.application_criteria_evaluationsstill hasprimary_key: []even thoughidis now declared; setting a PK is a separate change with different blast radius.Review guide
airbyte-integrations/connectors/source-ashby/manifest.yaml— the schema additions. Note some existing properties moved: new fields were inserted alphabetically, which reordered a handful of neighbours.metadata.yaml+docs/integrations/sources/ashby.md— version bump and changelog.Test Coverage
No tests. A unit test module pinning the declared schemas to the documented field lists was added initially and removed at Patrick Nilan (@pnilan)'s request — it only restated the diff rather than covering behavior.
No live Ashby credentials exist for this connector (nothing in GSM for
source-ashby, nothing in 1Password), so the fields could not be confirmed against a live response either. This is a schema-vs-published-OpenAPI reconciliation only.Declarative-First Evaluation
No custom Python component was added or needed — this is a pure
InlineSchemaLoadermanifest change.Breaking Change Evaluation
Not breaking. No field was removed or retyped, no primary key, cursor, spec, or state format changed, and no stream stops returning records. Adding declarations to a schema whose
additionalPropertiesis alreadytrueonly surfaces fields that were already flowing through. Bumped0.3.8→0.4.0(minor: catalog additions). Progressive rollout is disabled for this connector, so no-rcsuffix.User Impact
Newly-declared fields become typed, selectable in the UI, and guaranteed in destinations. Users need to refresh the source schema to pick them up.
metadata.autoImportSchemaisfalseon all streams, so this won't happen automatically.Follow-ups (not in this PR)
interviewsagainst a liveinterview.listresponse and reconcile the declared shape.primary_key: [id]forapplication_criteria_evaluations.autoImportSchemaon streams where it's appropriate, so drift like this self-corrects.Can this PR be safely reverted and rolled back?
Related to https://github.com/airbytehq/airbyte-internal-issues/issues/16922:
Devin session