Repository navigation
feat(file-based): declare Unstructured Document Parser content as semantic-search indexable - #1172
Conversation
…tured content schema Co-Authored-By: AJ Steers <aj@airbyte.io>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1790131496-unstructured-semantic-search-annotation#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1790131496-unstructured-semantic-search-annotationPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
|
/ai-prove-fix |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A moderate null-filling edge case and a semantic-search contract test gap remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Declares unstructured document content fields as semantic-search indexable and preserves annotations during schema processing.
Changes:
- Adds semantic-search metadata to unstructured schemas.
- Prevents null-filling from mutating
x-*annotations. - Updates parser and scenario schema expectations.
| File | Description |
|---|---|
unit_tests/sources/file_based/scenarios/unstructured_scenarios.py |
Updates scenario schema expectations. |
unit_tests/sources/file_based/file_types/test_unstructured_parser.py |
Updates parser schema assertions. |
airbyte_cdk/sources/file_based/stream/default_file_based_stream.py |
Preserves schema extension metadata during null filling. |
airbyte_cdk/sources/file_based/file_types/unstructured_parser.py |
Adds the semantic-search annotation to content. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
|
/prerelease
|
|
Devin, create a PR for source-google-drive to take this CDK update using the prerelease I triggered above. Then kick off /ai-prove-fix using your CI trigger tool in the ops mcp. |
|
👍 On it. Will pin source-google-drive to the prerelease CDK version from the run above in a new airbyte PR, then trigger |
Aldo Gonzalez (aldogonzalez8)
left a comment
Aldo Gonzalez (aldogonzalez8)
left a comment
There was a problem hiding this comment.
APPROVED
Left non-blocking question
Co-Authored-By: AJ Steers <aj@airbyte.io>
|
Just retrigger prerelease here. And don't be blocked on creating that PR - since you can easily bump to new prereleases once the PR is ready. |
|
☑️ Done. Connector PR: airbytehq/airbyte#86924 (pinned to |
| } | ||
| ], | ||
| "windowing": {"context_max_chars": 2048, "context_boundary": "whole_unit"}, | ||
| "embedding": {"model": "text-embedding-3-small"}, |
There was a problem hiding this comment.
I don't think the embedding model should be on the connector side - that should be in the platform. Otherwise we'll need to republish all connectors when swapping models
There was a problem hiding this comment.
yeah the fusion pipeline ignores the model from the connector. we shouldn't include this here
|
❌ Cannot revive Devin session - the session is too old. Please start a new session instead. |
Overview
👉 TL;DR: Files parsed with the Unstructured Document Format (Google Drive, S3, GCS, SFTP, …) now tell downstream semantic indexers that their Markdown content column is indexable text. Until now the discovered schema carried no such hint, so every unstructured stream was silently un-indexable by Sonar/Fusion.
Specifically,
UnstructuredParser.infer_schemaattachesx-airbyte-semantic-search(module-levelSEMANTIC_SEARCH_ANNOTATION) to thecontentproperty, andDefaultFileBasedStream._fill_nullslearns to leavex-*schema extensions untouched.Pairs with:
schema.json)contentannotation (do not merge) airbyte#86924 (source-google-drivepinned to this branch's prerelease; do-not-merge until this lands)Changes
unstructured_parser.py: newSEMANTIC_SEARCH_ANNOTATION(content_type: markdown, one whole-documentchunksample,context_max_chars: 2048,text-embedding-3-small, metadatadocument_key/_ab_source_file_url/_ab_source_file_last_modified) — shape mirrors Sonar'sintegrations/google-drive/connector.yamland validates against Sonar'sSemanticSearchConfig.unstructured_parser.py:infer_schemaemits the annotation oncontent.default_file_based_stream.py:_fill_nullstracks whether it is iterating apropertiesmap (every key is a field name → recurse) or a schema object (skipx-*extension keys). Without this it rewrote the annotation's metadatatype: "string"to["null", "string"], which the consumer rejects.test_unstructured_parser.py/unstructured_scenarios.pyimport the constant; newtest_fill_nulls_properties_map_only_exempts_x_keys_on_schema_objectscovers a top-levelx-statusfield, a nestedproperties.x-innerfield, and the untouched annotation internals.Review Spotlight
Reviewers with limited time, please review first:
unstructured_parser.py#L46-L66: the annotation shapedefault_file_based_stream.py#L344-L371:_fill_nullsproperties-map vs schema-object contexttest_default_file_based_stream.py#L91: regression test for legitimatex-*field namesRisks
content; no record-level change. Verified end-to-end via/ai-prove-fixon the connector PR: DISCOVER diff vssource-google-drive0.5.36is exactlyproperties.content.x-airbyte-semantic-search; SPEC/CHECK/READ at parity.merge_schemasalready preserves non-typekeys, so no other schema-pipeline change was needed.Known CI exceptions
Check: destination-motherduck— MotherDuck token auth failure in CI; fails identically on unrelated PRs (e.g. runs35787513116,35694559096,35639104268).Check: source-google-drive— standard-tests harness instantiatesSourceGoogleDrive()with no args, but the published0.5.36connector'sFileBasedSourcesubclass requirescatalog/config/state. Fails before any schema code runs; orthogonal to this diff.Test plan
poetry run pytest unit_tests/sources/file_based/file_types/test_unstructured_parser.py unit_tests/sources/file_based/stream/test_default_file_based_stream.py unit_tests/sources/file_based/test_scenarios.py -k unstructuredpoetry run ruff check/ruff format --check/mypy— all green in CILink to Devin session: https://app.devin.ai/sessions/ebc5d6457e0b41b4837a46e06e14d66c
Open in Devin Desktop: https://app.devin.ai/desktop/session/ebc5d6457e0b41b4837a46e06e14d66c?variant=devin
Requested by: Aaron ("AJ") Steers (@aaronsteers)