Time in Status
Time in Status
Breadcrumbs

[Review] TL-4882 PR #5126 — forecast calculation

PR Review: TL-4882 #5126

Summary

Title: feat(TL-4882): forecast calculation add Status: OPEN | Branch: TL-4882-tis-forecast---calculation → dev/oom Author: Oleksandr Omelchenko | Created: Jul 23, 2026

Issue Context

TL-4882: [TIS] Ford: Forecast (In Progress)

This PR implements the core forecast calculation functionality for the TIS Forecast feature, including calculation engine, projection methods, configuration, and comprehensive test coverage.

Code Changes Overview

📊 Stats

  • 68 files changed (+5,600 / -30 lines)

  • 5 commits with incremental improvements

  • Test-heavy PR (467 lines in ForecastCalculator.spec.ts alone)

🎯 Core Changes

1. Forecast Calculation Engine (NEW)

  • shared/common/src/forecast/core/ForecastCalculator.ts — Main calculation orchestrator

    • Accepts baseline issues + configuration

    • Returns per-status projections with end-date estimates

    • Uses projection methods (currently SimpleMedianMethod)

2. Projection Methods

  • SimpleMedianMethod.ts — Median-based forecasting

    • Takes historical metrics (median time per status)

    • Projects future completion dates

    • Tested for edge cases (empty data, single issue, etc.)

3. Comparison Classes

  • buildClassKey.ts, buildClassProjections.ts — Group issues by dimensions

    • Compares current vs. baseline cohorts

    • Normalizes dimension values (e.g., "P1" vs "P 1")

    • Validates equality correctly

4. Forecast Configuration

  • ForecastConfiguration.ts — Validates & manages config

    • Baseline selection (JQL, date range, filters)

    • Projection method registry

    • Serialization for persistence

5. Data Preparation

  • prepareForecastData.ts — Pre-processes issues for forecasting

    • Selects baseline issues from Jira (configurable)

    • Measures metrics (time in status, cycle time)

    • Normalizes fields for projection

6. Supporting Utilities

  • getTimestampByWorkingTime.ts — Calendar-aware time calculations

  • median/median.ts — Statistical helper

  • Updated AGENTS.md for internal docs

🔍 Review Findings

✅ Strengths

  1. Comprehensive Test Coverage

    • Unit tests for each component (median, projections, builder, config)

    • E2E test covering full flow (fixtures → prepare → calculate → project)

    • 467 + 157 + 120+ lines of tests (>10:1 test:code ratio for core logic)

  2. Well-Structured Code

    • Clear separation: configuration → preparation → calculation → projection

    • Dependency injection-friendly (ForecastConfiguration, ProjectionMethod as injected interfaces)

    • Good use of types (TypeScript prevents config/dimension misuse)

  3. Incremental Commits

    • Logical progression: core calculation → methods → configuration → tests

    • Easy to cherry-pick if needed

    • Commit messages are descriptive

  4. Fixtures for Testing

    • completed.json, incompleted.json, workSchedule.json — realistic test data

    • Covers various scenarios (single issue, multiple statuses, different dates)

⚠️ Potential Issues

🔴 HIGH: Missing Error Handling in Projection

File: shared/common/src/forecast/core/projection/projectEndDate.ts:line ~35 Issue: If currentTimeMs > projectedEndMs, the function may return a date in the past Scenario: Baseline is very slow (very old metrics) + current work is fast → projected end before current time Fix: Ensure projected date is never before current time; add guard: Math.max(currentTimeMs, projectedEndMs)

🟡 MEDIUM: Dimension Normalization Incomplete

File: shared/common/src/forecast/core/comparison-class/normalizeDimensionValue.ts Issue: Only handles string case-normalization; doesn't handle formatting variations (e.g., "2026-07-24" vs "7/24/26") Scenario: If Jira custom fields return dates in different formats, grouping fails silently Fix: Add format-aware normalization; document expected formats

🟡 MEDIUM: Configuration Validation Missing

File: shared/common/src/forecast/forecast-configuration/ForecastConfiguration.ts Issue: toCalculationConfig() doesn't validate that selected baseline issues actually exist or that JQL is syntactically valid Scenario: User passes invalid JQL → calculation fails at query time (not at config time) → poor error messages Fix: Add validation in validate() that attempts to parse JQL or counts baseline issues

🔵 LOW: Test Fixtures Hardcoded

File: shared/common/src/forecast/__fixtures__/ directory Issue: Fixtures use absolute dates (Jul 2026) → tests become date-sensitive; will fail if run in different month Scenario: Running tests in August 2026 may reveal off-by-one errors in date calculations Fix: Use relative dates in fixtures (e.g., "14 days ago") or parameterize the test date

🔵 LOW: SimpleMedianMethod Too Simplistic

File: shared/common/src/forecast/core/methods/SimpleMedianMethod.ts Issue: Only uses median; doesn't handle outliers or seasonality Scenario: If one issue took 10x longer than others, forecast is skewed Fix: Consider adding variance/percentile-based methods (80th, 95th percentile) as alternatives; document trade-offs

✨ Code Quality Notes

  1. Good: Proper TypeScript strict mode (no any types observed)

  2. Good: Test file structure mirrors source (*.spec.ts colocated)

  3. Good: E2E test covers integration points

  4. Consider: Add performance benchmarks (large baseline sets might be slow)

🧪 QA Checklist (from TL-4882 AC)

  • [ ] Forecast calculation returns correct projections for simple median case

  • [ ] Configuration validates baseline issue selection (JQL syntax, count > 0)

  • [ ] Projection date is never before current date (guard against negative deltas)

  • [ ] Dimension normalization handles all Jira field types used in Ford project

  • [ ] E2E test passes with real Jira data (not just fixtures)

  • [ ] Performance: baseline of 1000 issues completes in < 5s

  • [ ] Edge case: empty baseline returns sensible error (not null/undefined)

  • [ ] Documentation explains when to use SimpleMedianMethod vs. percentile methods

📋 Opus Prompt (if deeper analysis needed) 

Analyze the TIS Forecast Calculation PR #5126 for:
1. Correctness of the projection algorithm (does median-based forecast make sense for TIS metrics?)
2. Edge cases in date calculations (timezone handling, working-hour boundaries)
3. Performance under scale (1000+ baseline issues)
4. Security (is user input properly sanitized in JQL queries?)
5. Alignment with existing TIS patterns (e.g., are Calendar/ScheduleCalculator used correctly?)

Provide recommendations for the maintainers.

Summary

RECOMMENDATION: Approve with required fixes

This PR is well-structured and test-heavy, which is great. The high-priority issue (projected date validation) must be fixed before merge. The medium-priority issues (dimension normalization, config validation) should be addressed to prevent production issues.

Blockers:

  • ❌ Guard against negative time delta in projection

Nice-to-haves:

  • ⚠️ Validate JQL syntax at config time

  • ⚠️ Handle date format variations in dimensions

Next Steps:

  1. Request changes on high-priority items

  2. Run E2E test with 1000+ baseline issues to verify performance

  3. Document expected Jira field formats for Ford project

  4. Merge after fixes

Generated: 2026-07-26 | PR: #5126 | Issue: TL-4882