feat(source-ashby): add application_history stream - #84392
feat(source-ashby): add application_history stream#84392devin-ai-integration[bot] wants to merge 4 commits into
Conversation
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.
Pull request overview
Adds a new application_history stream to the source-ashby declarative connector to expose per-application interview stage entry/exit timestamps (from POST /application.listHistory), along with endpoint-scoped rate limiting and accompanying docs/version updates.
Changes:
- Introduces
application_historyas a full-refresh substream ofapplications, including cursor pagination and a declaredprimary_key: [id]. - Adds an endpoint-scoped
api_budgetpolicy to cap calls to/application.listHistory. - Updates connector version and documentation to surface the new stream and its operational/cost implications.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/integrations/sources/ashby.md | Documents the new stream, permissions, usage/join guidance, cost warning, and changelog entry. |
| airbyte-integrations/connectors/source-ashby/metadata.yaml | Bumps the connector image tag to 0.4.0. |
| airbyte-integrations/connectors/source-ashby/manifest.yaml | Adds api_budget, fixes an AddedFieldDefinition type, and defines the new application_history stream and parent substream. |
| airbyte-integrations/connectors/source-ashby/AGENTS.md | Updates contributor guidance to include the new stream’s characteristics and operational notes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| page_size_option: | ||
| type: RequestOption | ||
| inject_into: body_json | ||
| field_name: limit |
There was a problem hiding this comment.
Good catch on the inconsistency, but it points the other way: limit is correct and the existing per_page usages are the latent bug. Ashby's ApplicationListRequest is additionalProperties: false with syncToken, cursor, limit, createdAfter, createdBefore, status, jobId, and expand — there is no per_page parameter anywhere in the API. So the streams injecting per_page are most likely having their page size silently ignored today and falling back to Ashby's default page size, which is exactly why this connector's measured throughput is as low as it is.
I'm not fixing that here on purpose. Correcting per_page to limit across the existing streams changes the request shape and page count for every stream in the connector, which deserves its own PR with its own version bump and its own regression run rather than riding along inside a new-stream PR — especially with #84274 open against this same manifest. The new stream uses the documented parameter so it's right from the start, and I've flagged the cleanup separately.
There was a problem hiding this comment.
🚫 Not fixing here — filed as #84394 instead. Disposition marker plus the tracking issue for the per_page cleanup, which I couldn't add to the reply above (the API rejects edits to inline review comments from this account).
| id: | ||
| type: string | ||
| format: uuid |
There was a problem hiding this comment.
Deliberate, and I'd like to keep it non-nullable. id is this stream's declared primary key, and Ashby's ApplicationHistory response schema marks it required, so a null there would mean an unkeyable record rather than a value we should quietly accept. The nullable-union style you're citing on applications.properties.id is a real inconsistency, but it's the pre-existing side of it: that stream declares primary_key: [id] while typing id as ["null", "string"], which is the combination that actually risks trouble. Tightening applications is out of scope here — this PR deliberately touches no existing stream, since #84274 is open against this same manifest.
Worth noting the typing has no effect on whether a null gets through: additionalProperties: true plus the default transformer means a null id would still be emitted, just flagged against the schema. So this is about declaring intent accurately, and "the primary key is always present" is the accurate intent.
There was a problem hiding this comment.
🚫 Not fixing — disposition marker for the reply above, which I can't edit in place (the API rejects edits to inline review comments from this account).
|
↪️ Triggering Reason: Draft PR, CI checks passing, and no |
|
🧪 Prove-Fix Validation —
|
| Check | Result |
|---|---|
SPEC |
✅ identical |
CHECK |
✅ passes on both versions |
DISCOVER |
✅ 17 → 18 streams; application_history present on target only; all 17 existing stream schemas unchanged |
READ — existing streams |
✅ 43,113 records on both versions, per-stream delta 0 across all 8 streams that returned data; no missing/duplicate PKs |
READ — application_history |
⚪ not exercised (stream absent from the configured catalog available for the read) |
candidates was flagged with value diffs — this is upstream API nondeterminism, not a regression
Two independent comparison runs flagged candidates field-value differences (75, then 83). Analysis of the record artifacts:
- Diffs are confined to two fields:
fileHandles(Ashby-side file IDs / signed handles / converted filenames, which rotate) andschool. - The differing record sets are not reproducible: only 153 of roughly 312 differing records are common to both runs, and each run flags a different number of fields.
- On records where only
schooldiffers,updatedAtis byte-identical on both sides — the record was not modified upstream, the API simply returned a different value from the candidate's education entries. - Record counts, PK sets, and PK integrity are identical every time.
- The PR diff is
+177 / -0lines inmanifest.yaml: thecandidatesstream definition is untouched, so no code path exists by which the new stream could change it.
Conclusion: the Ashby API returns nondeterministic values for these fields between successive reads. This is pre-existing API behavior surfaced by strict field-equality comparison, not a behavior change introduced by this PR.
Why application_history could not be read
- The first attempt used integration-test credentials and failed before the connector ran (
Failed to fetch integration test config from GSM for source-ashby→Config is required for check command). There is nosource-ashbyintegration-test secret provisioned, so there is no first-party Ashby account to read from. - The subsequent attempts used a real connection's config and configured catalog. That catalog was created before this PR, so it does not contain
application_history— and the regression harness filters an existing configured catalog rather than re-discovering streams, so the new stream cannot be added to the read that way.
To close this gap, one of:
- Provision Ashby integration-test credentials (a
SECRET_SOURCE-ASHBY__CREDSentry) so the GSM path can read every discovered stream, including new ones. This is the durable fix and also unblocks futuresource-ashbyprove-fix runs. - Pin a consenting connection to the prerelease, refresh its schema and enable
application_historyfor one sync. This touches a customer connection, so it needs the Slack HITL approval gate before any pin.
Pre-flight checks
- Viability: ✅ Uses only built-in declarative components (
SimpleRetriever,DefaultPaginator+CursorPagination,SubstreamPartitionRouter,AddFields,InlineSchemaLoader) — no custom Python.enteredStageAt/leftStageAt/stageIdare exactly the stage-transition timestamps the requester wanted, and the stream carriesapplication_id+application_status+application_created_atso it can be joined tointerview_schedulesdownstream. - Design intent: ✅ Full-refresh only is intentional —
/application.listHistoryexposes no date filter, and history rows are mutable.⚠️ Cost note: the stream fans out one request per application in scope (start_date-filtered parent), so runtime scales with application count; the new rate policy bounds it to 100 req/min. - Safety: ✅ No malicious or obfuscated code, no new hosts (
api.ashbyhq.comonly), no credential handling beyond the existingBasicHttpAuthenticator, no exfiltration paths. - Breaking change: ✅ Non-breaking / additive. No stream removed or renamed, no field types changed, no primary-key, cursor, state or spec changes. Minor bump
0.3.8→0.4.0is appropriate for a new stream pre-1.0. - Reversibility: ✅ Rolling back to
0.3.8is safe — no state format or spec change; the only effect is thatapplication_historystops being discoverable. ⚠️ Worth a reviewer's eye:0.3.8-rc.5reverted an earlierapi_budget(plus concurrency) after rollout monitoring found source-read regressions and 429 warnings. This PR reintroduces anapi_budget, but scoped by anHttpRequestRegexMatcherto/application\.listHistoryonly, and no concurrency change comes with it. The regression runs confirm existing streams are unaffected by its presence (identical counts and PKs on every run).
Test cases run
- Integration-test credentials, all streams, comparison mode — run — ⚪ infrastructure failure, no connector behavior observed (no GSM config).
- Real connection config + catalog, all configured streams, comparison mode — run — ✅ SPEC/CHECK/DISCOVER pass, existing-stream read identical,
candidatesfield diffs flagged. - Repeat of case 2 narrowed to
applications+candidates— run — ✅ confirms thecandidatesdiffs are nonreproducible upstream nondeterminism.
Recommended next steps
- Provision
source-ashbyintegration-test credentials, then re-run this validation to exerciseapplication_historyend to end (criteria 2–4). Alternatively, request HITL approval to pin one consenting connection. - The change is safe to canary independently of that gap —
/ai-canary-prerelease— since it is additive, reversible, and proven not to alter existing stream output. - Once merged and rolled out, the customer's "Entered At" report field is served by joining
application_history.enteredStageAttointerview_scheduleson application + stage id.
Detailed, non-sanitized evidence is in the linked private issue.
🤖 Automated validation — Devin session
|
Context for the in-flight What is already validated, from this PR's own session:
What failed, and why it is not a defect in this change:
So the remaining gap is specific: the pagination |
|
|
Follow-up for the prove-fix run: there are no GSM integration-test credentials for Evidence from that run itself, https://github.com/airbytehq/airbyte-ops-mcp/actions/runs/31797791741: Consistent with two other checks: So the two viable paths for a live read are: read through a Cloud connection's config (which is what the earlier runs did — An all-streams comparison read over a connection would exercise the 10 configured streams and silently skip the new one — which is the " |
|
Session provenance for my two comments above (they were posted without it, and the API rejects edits to comments from this account): https://app.devin.ai/sessions/7ac246d54a0641de9a9bd4b1c1cca2fb |
|
🙋 Escalated per Hands-Free AI Triage Project — |
Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
|
What
Adds a new
application_historystream tosource-ashby, exposing interview stage entry/exit timestamps (enteredStageAt/leftStageAt) so customers can measure funnel velocity per stage. These timestamps exist nowhere else in the Ashby API — only onPOST /application.listHistory— so no combination of existing streams can produce them.Requested via airbytehq/oncall#13283. The reporting customer needs it for all historic applications, including archived and hired ones (funnel drop-off analysis), so the parent is deliberately unfiltered by
status.How
application_historyis a full-refresh substream ofapplications, following theapplication_criteria_evaluationspattern already in this manifest: aSubstreamPartitionRouterover an inlineapplications_for_historyparent,parent_key: id,partition_field: application_id, andAddFieldsre-attachingapplication_idto each child record.Three things differ from that prior art, deliberately:
application.listHistoryreturns paged results, so it gets aDefaultPaginatorwithCursorPaginationinjectingcursorinto the request body, rather thanNoPagination.primary_key: [id], so a full-refresh-overwrite or dedup destination keys history events correctly instead of appending duplicates on every sync.statusandcreatedAtride along viaextra_fieldson theParentStreamConfig, surfaced asapplication_statusandapplication_created_at. Funnel analysis needs both, and carrying them here saves the user a join againstapplications— which matters because this stream is intended to run on its own connection, separately fromapplications.Incremental sync is not offered, and that is a limitation of the endpoint rather than a shortcut.
ApplicationListHistoryRequestisadditionalProperties: falsewith onlyapplicationId,cursor, andlimit— there is no date filter and nosyncToken. Ashby'ssyncTokenmechanism, where it does exist, is an opaque value returned in the response envelope that must be persisted and replayed in the next sync's request body, and that is not expressible in a declarative manifest on CDK 7.17.4:incremental_syncaccepts onlyDatetimeBasedCursorandIncrementingCountCursor,CustomIncrementalSyncwas removed from the declarative schema in CDK v7.0.0, andget_request_body_jsonacceptsstream_stateand discards it. This connector stays manifest-only, so full refresh is the correct shape.Errors are differentiated per application rather than uniformly. Ashby returns what would be
4XXas HTTP 200 withsuccess: false, so without an error handler a permission failure or an API-side rejection is indistinguishable from an account with no interview history: the stream completes, emits nothing, and exits 0. The child requester therefore gets aDefaultErrorHandlerwhose orderedHttpResponseFilters split soft failures by documented error code:errorInfo.code == 'application_not_found'→IGNORE. This is the one per-application condition Ashby documents (deleted or inaccessible application). With one partition per application, a single bad application must not abort a fan-out measured in hours, so that application's history is skipped and logged.success: falseenvelope →FAIL. That coversmissing_endpoint_permission, any unrecognised code, and the OpenAPIErrorResponseshape (errors: [{message}]) which carries no code field at all. Unknown means loud, never silent.429→RATE_LIMITEDand500/502/503/504→RETRY, declared explicitly because filters are evaluated in order and Ashby wraps those responses in the samesuccess: falseenvelope, which would otherwise be caught by theFAILfilter and skip the retry it deserves. Codes outside those lists still fall through to the CDK's default mapping.Instead of the source-wide
ConcurrencyLevelthat was reverted in #84214 after source-read regressions and a 429 retry warning, this adds an endpoint-scopedHTTPAPIBudget: aMovingWindowCallRatePolicyof 100 requests perPT1M, matched by anHttpRequestRegexMatcheron/application\.listHistory. This is a ceiling (~1.67 req/sec) sitting just above the ~1.31 req/sec this connector sustains single-threaded, so it caps bursts without throttling normal reads. Requests that match no policy are allowed through unchanged (APIBudget.acquire_calllogs and proceeds whenget_matching_policyreturnsNone), so the other 17 streams are unaffected.Review guide
airbyte-integrations/connectors/source-ashby/manifest.yaml—api_budget(lines 3–12) and theapplication_historystream, whoseerror_handleron the child requester is the part worth reading closely: filter order determines whether a 429 retries or fails. Also a one-line fix at line 1372 adding the requiredtype: AddedFieldDefinitionto the pre-existingapplication_criteria_evaluationstransformation, which failed strict Draft-7 validation of the manifest.airbyte-integrations/connectors/source-ashby/metadata.yaml— minor bump,0.3.8→0.4.0.docs/integrations/sources/ashby.md— stream list, permissions table, the documented warehouse joins, the cost warning, and the changelog.Verified against the exact CDK the connector runs on (
source-declarative-manifest:7.17.4), without live API access — there are no Ashby credentials in GSM or 1Password, socheckandreadcould not be exercised locally:discoverreturns 18 streams; the 17 existing streams are unchanged, andapplication_historyresolves with primary key[['id']]and the typed fields above.stop_conditionandcursor_valuewere evaluated through the CDK's ownInterpolatedBoolean/InterpolatedStringrather than only checked at construction time.{{ not response.moreDataAvailable }}returnsFalsewhen more data is available andTrueboth when the flag isfalseand when it is absent entirely;{{ response.nextCursor }}yields the cursor when present andNoneon the last page. Theresponse.get(...)form behaved identically in the 7.17.4 Jinja sandbox, so the plain attribute form shipped.application.listHistoryresponses through the CDK: anapplication_not_foundenvelope on one partition of several skips only that partition and the read exits 0 with the other partitions' records intact;missing_endpoint_permissionand the code-lesserrors[]shape each emit an error trace, mark the streamINCOMPLETE, and exit non-zero;429and500each retry five times with1, 2, 4, 8, 16s backoff; and a healthy multi-page response fires no filter and is byte-for-byte unaffected.HttpResponseFilter.error_messageis interpolated with onlyconfig,responseandheadersin 7.17.4 —stream_sliceis not in scope and raisesJinja macro has undeclared variables— so the applicationId cannot be interpolated into these messages. On theFAILpath it arrives anyway, because the CDK'sinternal_messageincludes the request body (Request (body): '{"applicationId": ..., "limit": 100}'). On theIGNOREpath the CDK logs the custom message only, so that message carries Ashby'serrorInfo.requestId, which their support docs ask customers to quote.extra_fieldssemantics were confirmed in CDK source:_extract_extra_fieldsjoins each path with.and, importantly, sets the key toNoneonKeyError, so{{ stream_slice.extra_fields['status'] }}cannot raise on an application that lacks the field.User Impact
Additive. A new stream appears in the catalog; no existing stream, schema, or state format changes. Users must enable
application_historyexplicitly and re-run discovery to see it.Cost — please read before approving. This stream issues at least one request per application, on every sync, with no incremental support. On a real account measured at ~1.31 req/sec sustained single-threaded with ~108,100 applications, the application fan-out alone is on the order of 8 to 24 hours, and pagination adds more requests on top. It is intended to run on its own connection on a slow schedule, not alongside the other streams on an hourly one. This tradeoff is inherent to the endpoint, not to this implementation: there is no way to ask Ashby for "history that changed since X".
A deleted or inaccessible application is now skipped with an
INFOlog rather than silently contributing zero rows, and any other Ashby error fails the sync instead of completing as if the account had no interview history.The parent inherits
createdAfterfromconfig['start_date'], sostart_datemust be set early enough to cover the history the user wants — a latestart_datesilently yields partial history rather than an error.Version collision: #84274 is open and also targets
0.4.0. Whichever of the two merges second needs to rebase its version bump and changelog entry.Can this PR be safely reverted and rolled back?
Link to Devin session: https://app.devin.ai/sessions/7ac246d54a0641de9a9bd4b1c1cca2fb