fix(task-delivery): detect closed tasks by statusCategory, not display name (#1541) - #1732
Conversation
…y name (constructorfabric#1541) Close-detection in the Task Delivery gold layer matched the status *display name* against a hardcoded list ('Closed','Resolved','Verified'). Default Jira Cloud "Done" and every non-English or custom workflow therefore reported zero closed tasks, blanking the whole Task Delivery panel. Drive close-detection from a source-neutral status dimension instead: - jira connector: flatten statusCategory.key -> category_key in the jira_statuses stream. - silver: new class_task_statuses (status_id -> unified status_category: new / in_progress / done / undefined) via jira__task_statuses; YouTrack projection derives it from the State-bundle isResolved flag. - gold: new migration recreates task_issue_current_state, task_status_intervals, jira_closed_tasks, close/reopen event views, in_progress seconds, dev seconds and task_delivery_bullet_rows to filter on status_category = 'done' (and 'in_progress'), joining class_task_statuses on value_ids[1]. Object names and column shapes are preserved so downstream views are untouched. - youtrack: full silver dbt models (task_* projections, users snapshot/history, identity_inputs, statuses) — dormant until dbt_select flips to tag:youtrack. - jira: add jira__identity_inputs to complete the profile/identity trio. - e2e: seed bronze_jira.jira_statuses across the Task Delivery tests, add the silver.class_task_statuses placeholder + bronze schema, and a status-category regression test — a done-category status counts regardless of its (possibly non-English) display name, while a status literally named "Closed" but with an indeterminate category does not. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR adds source-neutral task status categories, Jira and YouTrack staging projections, a Silver status dimension, category-based Task Delivery SQL, identity inputs, and Jira end-to-end fixtures covering localized and custom done statuses. ChangesStatus lifecycle contracts
Connector staging projections
Silver status dimension
Gold lifecycle views
Validation fixtures
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Jira as Jira status source
participant Staging as Jira dbt staging
participant Silver as Silver status dimension
participant Gold as Task Delivery views
participant Metrics as Metrics query
Jira->>Staging: emit statusCategory key and id
Staging->>Silver: normalize status_category
Silver->>Gold: join status events by status_id
Gold->>Metrics: expose category-based completion metrics
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 |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/components/connectors/task-tracking/specs/task-metrics-map.md (1)
73-84: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRemove the cross-source
category_keyrequirement.Lines 73-76 say future connectors must export Jira-native keys as
category_key, while lines 78-84 correctly definestatus_categoryas the cross-source contract. YouTrack should emit and consumestatus_category, not Jira'scategory_key; otherwise these sections establish conflicting schemas.🤖 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 `@docs/components/connectors/task-tracking/specs/task-metrics-map.md` around lines 73 - 84, Remove the requirement in the v1 hardcoded mapping section that future connectors export Jira-native values through class_task_statuses.category_key. Update the terminology and connector guidance so status_category is the sole cross-source contract, and explicitly state that YouTrack emits and consumes status_category while category_key remains Jira-specific.
🤖 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 `@docs/components/connectors/task-tracking/silver/jira/specs/DESIGN.md`:
- Line 448: Update the “Connector prerequisite (Bronze)” statement in DESIGN.md
to reflect that jira_statuses now flattens statusCategory.key into category_key;
remove the outdated missing-field wording or explicitly label it as
pre-migration guidance.
- Line 450: Align the documentation with the implemented Silver/Gold contract:
inspect the migration and either add the promised source-neutral
`class_task_status_history` model and materialize the join there, or revise this
section to accurately describe the direct join between
`silver.class_task_statuses` and `silver.class_task_field_history`; ensure the
claims about `jira__task_statuses`, Gold joins, and `status_category` match the
chosen implementation.
In `@docs/components/connectors/task-tracking/silver/specs/DESIGN.md`:
- Line 379: Update the implementation-status note in the canonical design to
reflect that the class_task_statuses Silver model and category-based Gold
migration are now implemented, removing the outdated claim that the model is
unbuilt and statuses remain hardcoded.
- Around line 383-410: The documented status-history boundary conflicts with the
shipped close-detection implementation, which directly joins
silver.class_task_statuses. Update the close-detection model and related Gold
views to read status_category/is_closed from task_tracker_status_history without
dimension joins, or revise the DESIGN.md contract and schema to match the actual
migration; ensure the resulting task_tracker_status_history model is created and
available before downstream consumers.
In `@docs/components/connectors/task-tracking/youtrack/specs/DECOMPOSITION.md`:
- Around line 303-311: Update the Domain Model Entities and Data inventories in
DECOMPOSITION.md to include the new TaskStatus entity and
youtrack__task_statuses data model, ensuring generated ownership and delivery
documentation reflects this Silver projection.
In
`@src/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__identity_inputs.sql`:
- Around line 15-24: Update the deactivation_condition in
identity_inputs_from_history to recognize YouTrack’s boolean representations by
matching lower(new_value) against both 'true' and '1' when field_name is
'banned'.
In
`@src/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__task_field_metadata.sql`:
- Around line 24-38: The append model rereads all bronze custom-field records,
causing duplicate observations on incremental runs. Update the query sourcing
`youtrack_project_custom_fields` to filter records using a persisted
deterministic watermark and tie-breaker based on `_airbyte_extracted_at` plus a
stable unique key, ensuring only new or changed observations are inserted while
retaining the initial full load.
In
`@src/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__task_worklogs.sql`:
- Around line 30-35: The append model currently assigns a new version via
_version on every run and lacks incremental filtering, creating duplicate
historical worklogs. Update youtrack__task_worklogs to derive _version from
w._airbyte_extracted_at and add an incremental predicate using the target
model’s latest processed extraction timestamp, while preserving selection of the
newest row per _airbyte_raw_id.
In
`@src/ingestion/scripts/migrations/20260708000000_task-delivery-status-category.sql`:
- Around line 91-101: The completed-task metric currently collapses status
history to one final close and excludes reopened issues. Update the status_cat
CTE and the tasks_completed logic around the repeated transition-metric section
to emit one completion event for every status transition into done, without
filtering out issues that were later reopened; preserve each qualifying
transition timestamp and avoid maxIf-based aggregation.
- Around line 169-195: Update the final interval-end logic in the event interval
construction to use final_close_at only when the current status is done;
otherwise use now() for reopened or still-active tasks. Locate the expression
handling the last event, including s.final_close_at and (e.evs[i + 1]).1, and
condition it on the current status so post-reopen intervals remain valid and are
not removed by the final interval filter.
---
Outside diff comments:
In `@docs/components/connectors/task-tracking/specs/task-metrics-map.md`:
- Around line 73-84: Remove the requirement in the v1 hardcoded mapping section
that future connectors export Jira-native values through
class_task_statuses.category_key. Update the terminology and connector guidance
so status_category is the sole cross-source contract, and explicitly state that
YouTrack emits and consumes status_category while category_key remains
Jira-specific.
🪄 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
Run ID: 4aff59bb-b7de-45a3-87b5-a788c87e0217
📒 Files selected for processing (42)
docs/components/connectors/task-tracking/silver/jira/specs/DESIGN.mddocs/components/connectors/task-tracking/silver/specs/DESIGN.mddocs/components/connectors/task-tracking/silver/specs/PRD.mddocs/components/connectors/task-tracking/specs/task-metrics-map.mddocs/components/connectors/task-tracking/youtrack/specs/DECOMPOSITION.mddocs/components/connectors/task-tracking/youtrack/specs/DESIGN.mdsrc/ingestion/connectors/task-tracking/jira/connector.yamlsrc/ingestion/connectors/task-tracking/jira/dbt/jira__identity_inputs.sqlsrc/ingestion/connectors/task-tracking/jira/dbt/jira__task_statuses.sqlsrc/ingestion/connectors/task-tracking/jira/dbt/schema.ymlsrc/ingestion/connectors/task-tracking/youtrack/dbt/schema.ymlsrc/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__identity_inputs.sqlsrc/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__task_comments.sqlsrc/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__task_field_metadata.sqlsrc/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__task_projects.sqlsrc/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__task_sprints.sqlsrc/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__task_statuses.sqlsrc/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__task_users.sqlsrc/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__task_worklogs.sqlsrc/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__users_fields_history.sqlsrc/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__users_snapshot.sqlsrc/ingestion/scripts/create-bronze-placeholders.shsrc/ingestion/scripts/migrations/20260708000000_task-delivery-status-category.sqlsrc/ingestion/silver/task-tracking/class_task_statuses.sqlsrc/ingestion/silver/task-tracking/schema.ymlsrc/ingestion/tests/e2e/metrics/schemas/bronze_jira.jira_statuses.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_bugs_to_task_ratio_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_bugs_to_task_ratio_ratiosum_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_due_date_compliance_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_estimation_accuracy_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_flow_efficiency_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_mean_time_to_resolution_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_pickup_time_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_stale_in_progress_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_status_category_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_task_dev_time_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_task_reopen_rate_gate_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_task_reopen_rate_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_tasks_completed_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_worklog_logging_accuracy_jira.test.yamlsrc/ingestion/tests/e2e/metrics/team_bullet_task_delivery_tasks_completed.test.yamlsrc/ingestion/tests/e2e/metrics/templates/jira_task.yaml
| SELECT | ||
| pcf.unique_key AS unique_key, | ||
| pcf.source_id AS insight_source_id, | ||
| CAST('youtrack' AS String) AS data_source, | ||
| pj.short_name AS project_key, | ||
| pcf.field_id AS field_id, | ||
| pcf.field_name AS field_name, | ||
| if(pcf.is_multi_value IS NULL, toUInt8(0), | ||
| toUInt8(pcf.is_multi_value)) AS is_multi, | ||
| pcf.value_type AS field_type, | ||
| toUInt8(lower(toString(pcf.value_type)) NOT IN | ||
| ('text','integer','float','date','date and time','period','string','')) AS has_id, | ||
| toDateTime64(pcf._airbyte_extracted_at, 3) AS observed_at, | ||
| toUnixTimestamp64Milli(pcf._airbyte_extracted_at) AS _version | ||
| FROM {{ source('bronze_youtrack', 'youtrack_project_custom_fields') }} pcf |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Avoid re-appending every field observation on each incremental run.
This append model rereads the entire bronze table on every run. ReplacingMergeTree compaction is asynchronous, so consumers can observe duplicate metadata rows before merges complete. Filter to new/changed observations with a deterministic watermark/tie-breaker before insertion.
🤖 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/task-tracking/youtrack/dbt/youtrack__task_field_metadata.sql`
around lines 24 - 38, The append model rereads all bronze custom-field records,
causing duplicate observations on incremental runs. Update the query sourcing
`youtrack_project_custom_fields` to filter records using a persisted
deterministic watermark and tie-breaker based on `_airbyte_extracted_at` plus a
stable unique key, ensuring only new or changed observations are inserted while
retaining the initial full load.
There was a problem hiding this comment.
Same as the worklogs thread: this mirrors jira__task_field_metadata verbatim, and _version = _airbyte_extracted_at is deterministic, so re-observations collapse under RMT and — critically — union_by_tag dedups to one row per unique_key at read time in the silver model. No duplicate metadata reaches consumers. A watermark filter would be a cross-connector change, out of scope here.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd class_task_field_metadata -t f
echo "---"
fd union_by_tag -t f
echo "---"
rg -n "union_by_tag" -A 20 --type sql | head -100Length of output: 392
🏁 Script executed:
#!/bin/bash
cat -n src/ingestion/silver/task-tracking/class_task_field_metadata.sql
echo "---macro---"
cat -n src/ingestion/dbt/macros/union_by_tag.sqlLength of output: 5324
The macro's dedup logic confirms the user's claim — union_by_tag applies QUALIFY ROW_NUMBER() OVER (PARTITION BY unique_key ORDER BY _version DESC) = 1, which is explicitly documented as a read-time dedup specifically to guard against upstream staging tables re-appending rows without RMT merges having run. That's exactly the scenario here.
@mitasovr Confirmed — the design intent in union_by_tag (src/ingestion/dbt/macros/union_by_tag.sql) explicitly documents this as its purpose: RMT merge timing isn't guaranteed, so the macro dedups by unique_key/_version at read time in silver.class_task_field_metadata regardless of duplicates in the staging layer. That fully covers the concern I raised, so I'll withdraw it. Agreed a watermark filter is out of scope for this connector staging model.
…ds the table (constructorfabric#1541) The Task Delivery e2e fixtures seed bronze_jira.jira_statuses, but the CH seeder INSERTs into an already-existing table — bronze tables are created by create-bronze-placeholders.sh at session start, not from the schemas/*.yaml (those only validate records). The prior commit added the silver.class_task_statuses placeholder but not the bronze one, so every seeded Task Delivery test failed with "table bronze_jira.jira_statuses not found in system.columns" before dbt/migrations even ran. Add the bronze placeholder (mirrors bronze_jira.jira_fields). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
…atuses (constructorfabric#1541) union_by_tag emits its ref() calls only under {% if execute %}, so dbt cannot infer the dependency at parse time and fails to compile class_task_statuses with "unable to infer all dependencies". Every other silver class model carries the same `-- depends_on: {{ ref('jira__<x>') }}` hint; add it here too (jira-only, matching the pattern — the youtrack ref lands with the youtrack silver rollout). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
constructorfabric#1541) The IC Task Delivery bullet (…0011) returns value + median + p25 + p75 + range_min + range_max, and the rig's no-unasserted-stat gate requires every returned stat to be checked. The status-category regression test asserted only `value`. Department = alice alone, so the distribution is the single value {2} and all stats collapse to 2. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
…ic#1541 design (CodeRabbit) Address CodeRabbit review: the specs still described class_task_statuses as "not yet built" and promised a materialized class_task_status_history with Gold performing no join, while the shipped migration joins class_task_statuses directly in Gold. Update to the shipped reality: - flip the "not yet built / Gold hardcodes names" status notes to "shipped (constructorfabric#1541): class_task_statuses built; Gold detects done by status_category". - describe the direct-join implementation (task_issue_current_state / task_status_intervals join class_task_statuses on value_ids[1]); mark the materialized class_task_status_history Silver model as an explicit future refinement (not built in constructorfabric#1541). - jira connector prerequisite: category_key is flattened (added in constructorfabric#1541), not "still missing". - DECOMPOSITION: add TaskStatus / youtrack__task_statuses to the §2.5 entity and data inventories. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
Summary
Fixes #1541. Task Delivery close-detection matched the status display name
against a hardcoded list (
'Closed','Resolved','Verified'), so a default JiraCloud
Done— and every non-English or custom workflow — reported zeroclosed tasks and blanked the whole Task Delivery panel.
Close-detection is now driven by a source-neutral
status_category(
new/in_progress/done/undefined), derived from Jira'sstatusCategory(and, for YouTrack, the State-bundleisResolvedflag) — neverfrom a localized label.
Layers
statusCategory.key→category_keyon thejira_statusesstream.class_task_statuses(status_id → status_category) via aper-source
jira__task_statusesprojection unioned by tag; YouTrackprojection maps
isResolved → done.20260708000000_task-delivery-status-category.sql): recreatestask_issue_current_state,task_status_intervals,jira_closed_tasks, theclose/reopen event views, in-progress/dev seconds and
task_delivery_bullet_rowsto filter onstatus_category(join onvalue_ids[1]). Object names and column shapes are preserved — downstreamviews (
team_member,ic_kpis,exec_summary, …) are untouched.snapshot/history, identity_inputs, statuses). Dormant until
descriptor.dbt_selectflips totag:youtrack— no behaviour change forexisting deployments.
jira__identity_inputsto complete the profile/identity trio(parity with bamboohr).
Tests
bronze_jira.jira_statusesacross the 14 Task Delivery*.test.yamlfixtures (so
class_task_statusesresolves), added thesilver.class_task_statusesplaceholder + bronze schema to the e2e rig.task_delivery_status_category_jira.test.yamlregression test:Done+ a non-English done-category status count, while a status literallynamed
Closedbut with anindeterminatecategory does not → assertstasks_completed = 2. This fails on the pre-Jira close-detection hardcodes English status names → Task Delivery metrics empty for Done/non-English workflows #1541 (name-based) code.Deploy notes
silver.class_task_statuses(dbt) → apply the migration →scripts/post-deploy/refresh-task-views.sh(the refreshable MVs are emptyuntil their next tick).
(YAML/
$ref/schema/placeholder). CI e2e should confirm the suite is green.🤖 Generated with Claude Code
Summary by CodeRabbit
status_categorylifecycle support (new / in_progress / done / undefined) across Jira and YouTrack.status_categoryinstead of localized status names.category_key(key-based) rather thancategory_name(name-based).Files related to the original task (#1541)
Core fix — close-detection by
status_category:src/ingestion/connectors/task-tracking/jira/connector.yaml— flattenstatusCategory.key→category_keysrc/ingestion/connectors/task-tracking/jira/dbt/jira__task_statuses.sql— per-source status dimensionsrc/ingestion/connectors/task-tracking/jira/dbt/schema.yml— registerjira__task_statusessrc/ingestion/silver/task-tracking/class_task_statuses.sql— unified silver status dimensionsrc/ingestion/silver/task-tracking/schema.yml— registerclass_task_statuses(+accepted_valuesonstatus_category)src/ingestion/scripts/migrations/20260708000000_task-delivery-status-category.sql— gold recreate: done/in_progress via categorysrc/ingestion/scripts/create-bronze-placeholders.sh—silver.class_task_statusesplaceholder (migration ref)Tests / e2e for the fix:
src/ingestion/tests/e2e/metrics/task_delivery_status_category_jira.test.yaml— new regression test (name vs category)src/ingestion/tests/e2e/metrics/schemas/bronze_jira.jira_statuses.yaml— bronze schema for seedingsrc/ingestion/tests/e2e/metrics/templates/jira_task.yaml—jira_statusesfixture records (6→done, 3→indeterminate, 1→new)src/ingestion/tests/e2e/metrics/task_delivery_*.test.yaml(13) +team_bullet_task_delivery_tasks_completed.test.yaml— seedbronze_jira.jira_statusesDocs/specs for the fix:
docs/components/connectors/task-tracking/specs/task-metrics-map.mddocs/components/connectors/task-tracking/silver/specs/DESIGN.md,.../silver/specs/PRD.mddocs/components/connectors/task-tracking/silver/jira/specs/DESIGN.mdRelated but broader (bundled in this PR, not required to close #1541):
src/ingestion/connectors/task-tracking/youtrack/dbt/youtrack__*.sql(dormant) +youtrack/dbt/schema.ymlsrc/ingestion/connectors/task-tracking/jira/dbt/jira__identity_inputs.sql— profile/identity trio paritydocs/components/connectors/task-tracking/youtrack/specs/{DESIGN,DECOMPOSITION}.md