fix(ingestion): heal Decimal jira id columns on warm clusters (#1743) - #1940
Merged
mitasovr merged 7 commits intoJul 27, 2026
Merged
Conversation
📝 WalkthroughWalkthroughThe PR adds post-migration healing for Jira ID columns and updates bootstrap documentation, dbt version detection, and connector DDL reminder prerequisites. ChangesIngestion migration and bootstrap updates
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
|
mitasovr
approved these changes
Jul 27, 2026
cyberantonz
enabled auto-merge (squash)
July 27, 2026 09:49
cyberantonz
disabled auto-merge
July 27, 2026 09:59
ktursunov
approved these changes
Jul 27, 2026
mitasovr
enabled auto-merge (squash)
July 27, 2026 11:28
ktursunov
approved these changes
Jul 27, 2026
…uctorfabric#1743) constructorfabric#1892 retyped jira__task_worklogs.worklog_id and jira__task_comments.comment_id projections to toString(), but incremental-append staging tables on existing stands keep the old Decimal(38,9) column; the positional union with the youtrack String twins then fails with NO_COMMON_TYPE and silver class_task_* stays empty. MODIFY the warm staging tables and their silver targets to Nullable(String) in the clickhouse-migrate heals (guarded, idempotent). Verified on CH 25.7.5.34: reproduced code 386, heal converges the union. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
Regenerating the connectors-ddl snapshot surfaced three gaps: - run-dbt.sh venv check used 'pip show | grep -q' under pipefail: grep -q exits at first match, pip dies on EPIPE, and a valid venv gets rm -rf'd (fatal when the rebuild interpreter lacks ensurepip). String-match the captured output instead. - The CI reminder comment and README only said how to run, not the prerequisites: throwaway ClickHouse, .env with a host-and-container reachable CLICKHOUSE_HOST, python3.12/3.11, and real HubSpot/Salesforce credentials (their CDK discover calls the live APIs) or the snapshot fallback for their bronze. Snapshot regenerated on CH 25.7.5.34: byte-identical, no snapshot commit needed for the constructorfabric#1743 heals. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
cyberantonz
force-pushed
the
fix/1743-jira-id-decimal-heal
branch
from
July 27, 2026 12:20
0b9522c to
6279b88
Compare
mitasovr
approved these changes
Jul 27, 2026
ktursunov
approved these changes
Jul 27, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/scripts/bootstrap-db/README.md`:
- Line 11: Update the local ClickHouse example in the README to use the
machine’s LAN IP for host-based dbt, matching the prerequisite guidance. If
retaining host.docker.internal, explicitly scope it to container-only usage and
ensure the example no longer claims it works for dbt.
- Line 12: Correct the fallback snapshot path in
src/ingestion/scripts/bootstrap-db/README.md at lines 12-12 and the generated
reminder in .github/workflows/connectors-ddl-reminder.yml at lines 58-60 to use
../connectors-ddl/{hubspot,salesforce}.sql from the bootstrap-db working
directory, or explicitly set the working directory before using the
repository-root-relative path.
In `@src/ingestion/scripts/bootstrap-db/run-dbt.sh`:
- Around line 19-22: Update the virtualenv reuse guard around DBT_BIN and
INSTALLED to query both dbt-clickhouse and dbt-core, parse each package’s
reported version exactly, and require exact equality with DBT_CLICKHOUSE_VERSION
and DBT_CORE_VERSION. Recreate the environment when either package is missing,
either version differs, or DBT_BIN is unavailable; do not rely on substring
matching.
🪄 Autofix (Beta)
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: d5e1fdac-a127-405c-a3f8-6f9c1622173f
📥 Commits
Reviewing files that changed from the base of the PR and between a0af56bdbeafceb0c26a6e03f32fb7546d4dd130 and b89284c.
📒 Files selected for processing (4)
.github/workflows/connectors-ddl-reminder.ymlsrc/ingestion/scripts/apply-ch-migrations.shsrc/ingestion/scripts/bootstrap-db/README.mdsrc/ingestion/scripts/bootstrap-db/run-dbt.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ingestion/scripts/apply-ch-migrations.sh
| - ClickHouse reachable under `CLICKHOUSE_HOST` both from this machine (dbt) and from inside docker containers (destination connector). For a ClickHouse running on this machine use `host.docker.internal`. | ||
| - `docker`, `jq`, `yq` (mikefarah v4) | ||
| - `python3.12` or `python3.11` on `PATH` — `run-dbt.sh` builds a local `.venv` with the pinned dbt from it (parity with the toolbox image; dbt-core 1.10 does not run on newer pythons). A `python -m venv`-capable interpreter is required (uv-managed pythons lack `ensurepip`; with those, pre-build the venv via `uv venv --seed .venv && .venv/bin/pip install dbt-core==<pin> dbt-clickhouse==<pin>`). | ||
| - ClickHouse reachable under `CLICKHOUSE_HOST` both from this machine (dbt) and from inside docker containers (destination connector). For a ClickHouse running on this machine use the machine's LAN IP (`ipconfig getifaddr en0`) — `host.docker.internal` resolves inside containers but not on the macOS host itself. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the ClickHouse host instructions consistent.
Line 11 now correctly says host-based dbt should use the machine LAN IP, but the local example at Line 25 still sets CLICKHOUSE_HOST=host.docker.internal and claims it works for dbt. On macOS, following that example makes host-side dbt unable to resolve ClickHouse; update the example to match this prerequisite or explicitly scope host.docker.internal to containers only.
🤖 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/scripts/bootstrap-db/README.md` at line 11, Update the local
ClickHouse example in the README to use the machine’s LAN IP for host-based dbt,
matching the prerequisite guidance. If retaining host.docker.internal,
explicitly scope it to container-only usage and ensure the example no longer
claims it works for dbt.
| - `docker`, `jq`, `yq` (mikefarah v4) | ||
| - `python3.12` or `python3.11` on `PATH` — `run-dbt.sh` builds a local `.venv` with the pinned dbt from it (parity with the toolbox image; dbt-core 1.10 does not run on newer pythons). A `python -m venv`-capable interpreter is required (uv-managed pythons lack `ensurepip`; with those, pre-build the venv via `uv venv --seed .venv && .venv/bin/pip install dbt-core==<pin> dbt-clickhouse==<pin>`). | ||
| - ClickHouse reachable under `CLICKHOUSE_HOST` both from this machine (dbt) and from inside docker containers (destination connector). For a ClickHouse running on this machine use the machine's LAN IP (`ipconfig getifaddr en0`) — `host.docker.internal` resolves inside containers but not on the macOS host itself. | ||
| - Real HubSpot and Salesforce credentials in `.env` — their CDK `discover` calls the live APIs, so fake values fail. Without credentials, seed their bronze from the committed snapshot instead: apply `scripts/connectors-ddl/{hubspot,salesforce}.sql`, run `./run-dbt.sh --select hubspot__bronze_promoted salesforce__bronze_promoted`, and continue from the dbt step. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the fallback snapshot path consistently.
Both instructions use a repository-root-relative path while the documented bootstrap flow runs from src/ingestion/scripts/bootstrap-db; use ../connectors-ddl/{hubspot,salesforce}.sql from that directory, or make the working directory explicit.
src/ingestion/scripts/bootstrap-db/README.md#L12-L12: correct the fallback path in the prerequisites..github/workflows/connectors-ddl-reminder.yml#L58-L60: apply the same corrected path in the generated reminder.
📍 Affects 2 files
src/ingestion/scripts/bootstrap-db/README.md#L12-L12(this comment).github/workflows/connectors-ddl-reminder.yml#L58-L60
🤖 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/scripts/bootstrap-db/README.md` at line 12, Correct the
fallback snapshot path in src/ingestion/scripts/bootstrap-db/README.md at lines
12-12 and the generated reminder in
.github/workflows/connectors-ddl-reminder.yml at lines 58-60 to use
../connectors-ddl/{hubspot,salesforce}.sql from the bootstrap-db working
directory, or explicitly set the working directory before using the
repository-root-relative path.
Comment on lines
+19
to
+22
| # No `pip show | grep -q`: grep -q exits at first match, pip dies on EPIPE and | ||
| # pipefail fails the check for a perfectly good venv (which then gets rm -rf'd). | ||
| INSTALLED="$("${VENV_DIR}/bin/pip" show dbt-clickhouse 2>/dev/null || true)" | ||
| if [[ ! -x "${DBT_BIN}" ]] || [[ "${INSTALLED}" != *"Version: ${DBT_CLICKHOUSE_VERSION}"* ]]; then |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'run-dbt\.sh' . || true
echo "== file excerpt =="
if [ -f src/ingestion/scripts/bootstrap-db/run-dbt.sh ]; then
nl -ba src/ingestion/scripts/bootstrap-db/run-dbt.sh | sed -n '1,80p'
fi
echo "== dbt version variables/usages =="
rg -n "DBT_(CLICKHOUSE|CORE)_VERSION|dbt-clickhouse|dbt-core|pip.*(show|install)" src/ingestion || true
echo "== deterministic behavior probe for bash substring matches =="
python3 - <<'PY'
import shlex, subprocess, sys
cases = [
("1.10.0", "1.10.0"),
("1.10.0rc1", "1.10.0"),
("1.10.0.post1", "1.10.0"),
("2.10.0", "1.10.0"),
("1.10.0", "1.10.0dev"),
]
for expected, actual in cases:
installed = f'Name: dbt-clickhouse\nVersion: {actual}'
bash = f'INSTALLED=$\'{installed.replace(chr(10), "\\n").replace("Name", "\\\\nName").replace("Version", "\\\\nVersion")}\'\n[[ $INSTALLED == *"Version: ${expected}"* ]]'
res = subprocess.run(["bash","-c", bash], stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
print(f"expected={expected!r} actual={actual!r} accepted={res.returncode==0}")
PYRepository: constructorfabric/insight
Length of output: 305
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
if [ -f src/ingestion/scripts/bootstrap-db/run-dbt.sh ]; then
awk '{printf "%6d\t%s\n", NR, $0}' src/ingestion/scripts/bootstrap-db/run-dbt.sh | sed -n '1,90p'
fi
echo "== dbt version variables/usages =="
if command -v rg >/dev/null 2>&1; then
rg -n "DBT_(CLICKHOUSE|CORE)_VERSION|dbt-clickhouse|dbt-core|pip.*(show|install)" src/ingestion || true
else
grep -RInE "DBT_(CLICKHOUSE|CORE)_VERSION|dbt-clickhouse|dbt-core|pip.*(show|install)" src/ingestion || true
fi
echo "== deterministic behavior probe =="
python3 - <<'PY'
import subprocess
cases = [
("1.10.0", "1.10.0"),
("1.0.0", "1.10.0"),
("1.10.0rc1", "1.10.0"),
("1.10.0.post1", "1.10.0"),
("dbt-core1.10.0rc1", "1.10.0"),
]
for expected, actual in cases:
installed = "Name: dbt-clickhouse\nVersion: " + actual
bash = 'INSTALLED=$' + repr(installed) + '\n[[ "${INSTALLED}" != *"Version: ' + expected + '*" ]] && echo reject || echo accept'
res = subprocess.run(["bash","-c", bash], stdin=subprocess.DEVNULL, text=True, stdout=subprocess.PIPE)
print(f"expected={expected!r} actual={actual!r} returncode={res.returncode} output={res.stdout.strip()}")
PYRepository: constructorfabric/insight
Length of output: 7176
Validate both pinned dbt versions with exact comparisons.
The current reuse guard only checks dbt-clickhouse, and Version: ${DBT_CLICKHOUSE_VERSION} is substring-matched, so incompatible values like 1.9.6.post1 could pass when the pin is 1.9.6. Also validate DBT_CORE_VERSION from pip show dbt-core with an exact version parse before reusing the venv.
🤖 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/scripts/bootstrap-db/run-dbt.sh` around lines 19 - 22, Update
the virtualenv reuse guard around DBT_BIN and INSTALLED to query both
dbt-clickhouse and dbt-core, parse each package’s reported version exactly, and
require exact equality with DBT_CLICKHOUSE_VERSION and DBT_CORE_VERSION.
Recreate the environment when either package is missing, either version differs,
or DBT_BIN is unavailable; do not rely on substring matching.
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
3 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.
Summary
Fixes the remaining failure in #1743: on warm stands
silver.class_task_worklogsaborts withNO_COMMON_TYPE: Decimal(38,9), String(see the latest issue comment), leaving Task Delivery / Code Quality blank.#1892 retyped the jira staging projections (
toString(worklog_id),toString(comment_id)), which fixes fresh installs — but those models are incremental-append, so existing stands keep the oldDecimal(38,9)column and the positionalunion_by_tagwith the youtrackStringtwins has no common supertype.Fix
Guarded, idempotent heal in the clickhouse-migrate hook (same pattern as the collab-chat/CRM heals):
MODIFY COLUMN IF EXISTS … Nullable(String)onstaging.jira__task_worklogs.worklog_id,staging.jira__task_comments.comment_idsilver.class_task_worklogs.worklog_id,silver.class_task_comments.comment_id(jira-first stands created these as Decimal)Verification
On ClickHouse 25.7.5.34: reproduced the exact code-386 error with mismatched staging tables; after the heal the union returns rows from both sources (
50026renders with no fractional tail); re-running the heal is a no-op.Closes #1743
🤖 Generated with Claude Code
Summary by CodeRabbit