Skip to content

Bump the github-directory descriptor so the "None" fix can reach Airbyte - #2421

Merged
mozhaev-dev merged 1 commit into
mainfrom
2393-descriptor-version-bump
Aug 11, 2026
Merged

Bump the github-directory descriptor so the "None" fix can reach Airbyte#2421
mozhaev-dev merged 1 commit into
mainfrom
2393-descriptor-version-bump

Conversation

@mozhaev-dev

@mozhaev-dev mozhaev-dev commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #2415. Same issue: #2393.

Problem

#2415 changed connectors/git/github-directory/connector.yaml and did not touch descriptor.yaml. That leaves the fix inert.

Reconcile republishes a nocode manifest only when the descriptor version differs from the version stored on the Airbyte definition (ADR-0015). Equal versions are a no-op, so a manifest-only change stays in the repository and reaches no cluster — while the deploy reports success with the previous manifest still active. That combination is what makes the omission easy to miss: nothing fails, and the connector keeps serving the pre-fix manifest indefinitely.

The connector's own deploy prerequisites already state this:

descriptor.yaml version bumped whenever connector.yaml changed. […] A manifest-only PR without a version bump silently never reaches Airbyte — the deploy "succeeds" with the old manifest still active. CI auto-bumps descriptors only for image ref changes, NOT for manifest edits.

I missed it in #2415.

Change

One line: version: "1.0.0""1.0.1", plus a comment recording why the bump exists, so the next manifest edit has the precedent in front of it.

Strict semver per ADR-0015; a manifest behaviour fix with no schema or stream change is a patch.

Affected areas

connectors/git/github-directory/descriptor.yaml only. No manifest change, no schema change, no stream change — #2415 already carries those.

How to test

Nothing to run locally; the descriptor is read at reconcile time, not by any test suite. What it enables is verifiable after deployment, in the bronze table the connector writes:

SELECT
    countIf(email = 'None') AS as_text_none,   -- expected 0 with the fixed manifest
    countIf(email = '')     AS as_empty        -- expected > 0
FROM bronze_github_directory.org_members;

as_empty > 0 is expected because GitHub returns no e-mail for a member whose address is unverified or outside the token's scopes — the common case. as_text_none > 0 after a sync means the definition serving it is still the pre-fix one.

Note for whoever redeploys

Order matters on an environment that already ingested this connector. The manifest fix stops new "None" values; it does not remove observations already recorded, and it does not release bindings already derived from them — the persons journal is append-only, and those accounts read as resolved rather than returning to the operator review queue. Recreating the database without redeploying the connector first reproduces the original defect from the first sync, because a fresh database has no incremental watermark to accidentally suppress the stale value.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated the GitHub directory connector to its latest descriptor version.

The manifest fix for #2393 landed without a descriptor-version bump, which
leaves it inert. Reconcile republishes a nocode manifest only when the
descriptor `version` differs from the version stored on the Airbyte
definition (ADR-0015); equal versions are a no-op, so a manifest-only change
stays in the repository and reaches no cluster — and the deploy reports
success with the previous manifest still active, which is what makes the
omission easy to miss.

CI auto-bumps descriptors for image-ref changes only, never for manifest
edits, so this has to be explicit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The GitHub directory connector descriptor version changes from 1.0.0 to 1.0.1.

Changes

GitHub directory descriptor

Layer / File(s) Summary
Increment descriptor version
src/ingestion/connectors/git/github-directory/descriptor.yaml
The descriptor version changes from 1.0.0 to 1.0.1 to republish the manifest.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

Suggested reviewers: ktursunov

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the descriptor version bump and its purpose of delivering the "None" fix to Airbyte.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2393-descriptor-version-bump

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mozhaev-dev
mozhaev-dev marked this pull request as ready for review August 11, 2026 06:37
@mozhaev-dev
mozhaev-dev requested a review from a team as a code owner August 11, 2026 06:37

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/ingestion/connectors/git/github-directory/descriptor.yaml`:
- Around line 15-18: Update the version comment in the descriptor manifest to
identify `#2415` as the primary fix reference, while retaining `#2393` only as a
secondary reference to the original bug report.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0e4bd68c-a03d-48c0-a1a0-3e1e8d8375ce

📥 Commits

Reviewing files that changed from the base of the PR and between f45db64 and f305a18.

📒 Files selected for processing (1)
  • src/ingestion/connectors/git/github-directory/descriptor.yaml

Comment on lines +15 to +18
# 1.0.1 — the manifest stopped storing absent profile fields as the text
# "None" (#2393). The bump is what makes that reach Airbyte at all: reconcile
# republishes a nocode manifest only on descriptor-version drift, so the
# previous manifest-only change was live in the repo and inert on every

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

rg -n -C 4 '`#2393`|`#2415`|descriptor-version|reconcile' src/ingestion/connectors/git/github-directory/descriptor.yaml src/ingestion 2>/dev/null | head -200
printf '\nRemotes:\n'
git remote -v

Repository: constructorfabric/insight

Length of output: 19813


🌐 Web query:

site:github.com/constructorfabric/insight/issues/2393 OR site:github.com/constructorfabric/insight/issues/2415

💡 Result:

  • Issue #2393: After sign-in, the portal can show another employee’s profile/data because identity resolution maps missing GitHub emails to the literal "None", grouping users incorrectly. It was opened August 10, 2026 and remains open. [1]
  • PR #2415: Fixes this by converting missing GitHub profile fields (email, name, company, role) to empty strings instead of "None". It was merged into main on August 11, 2026, but existing incorrect identity records require separate data repair. [2]

Update the descriptor comment to reference #2415 as the fix.

#2393 is the original bug report. Keep it only as a secondary reference.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ingestion/connectors/git/github-directory/descriptor.yaml` around lines
15 - 18, Update the version comment in the descriptor manifest to identify `#2415`
as the primary fix reference, while retaining `#2393` only as a secondary
reference to the original bug report.

@mozhaev-dev
mozhaev-dev added this pull request to the merge queue Aug 11, 2026
Merged via the queue into main with commit b97bc06 Aug 11, 2026
60 of 61 checks passed
@mozhaev-dev
mozhaev-dev deleted the 2393-descriptor-version-bump branch August 11, 2026 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants