fix(block): reset distanceAlongBlock between configurations

[priyanshu7739410]

Summary

Fixes #1427. Resolves a bug in transformBlockToEntry where the blockDistance accumulator was being erroneously carried across entirely distinct calendar configurations. This ensures that every new BlockConfiguration returned by the /api/where/block/{id}.json endpoint accurately tracks its own geometric offsets beginning from 0.0.

Root Cause

The var blockDistance float64 accumulator was previously declared outside the iteration block over serviceIDs, meaning its value was never reset between independent service days.

Fix

Scoped var blockDistance float64 directly into the individual serviceIDs iteration loop.

Tests

Added TestBlockHandlerCrossConfigurationDistanceReset to verify that DistanceAlongBlock is strictly 0.0 at the first stop of independent configurations belonging to the same block entity, without harming the distance accumulations on contiguous trips within that block.

Scope

This PR is narrowly focused to only address the cross-configuration blockDistance leak described in #1427. It intentionally does not address the broader trip ordering mechanisms or the distinct within-trip BlockSequence incrementing behavior flagged in #1018.

Summary by CodeRabbit

• Bug Fixes

  • Fixed distance calculations so each service configuration starts at zero while continuing to accumulate correctly within that configuration.
  • Prevented distance values from carrying over between separate service configurations.

• Tests

  • Added coverage verifying distance resets and accumulation across multiple service configurations.

[coderabbitai]

Warning

Review limit reached

Next included review available in 21 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 298ddc55-5688-4395-aa56-f82027489317

📥 Commits

Reviewing files that changed from the base of the PR and between 1d2d2be and 5530963.

📒 Files selected for processing (1)

• internal/restapi/block_handler_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4811da66-da7d-4dff-901a-bac32d3bf8d3

📥 Commits

Reviewing files that changed from the base of the PR and between f099da2 and 1d2d2be.

📒 Files selected for processing (1)

• internal/restapi/block_handler_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The block handler now resets blockDistance for each service configuration. A regression test verifies that each configuration starts at zero and accumulates distance independently.

Changes

Block distance reset

Layer / File(s)	Summary
Reset distance and validate configuration boundaries internal/restapi/block_handler.go, internal/restapi/block_handler_test.go	blockDistance is initialized inside the service-ID loop. The regression test verifies zero distance at the first stop and accumulation within each configuration.

Priority: ➖ Normal

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

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 1d2d2

Block-stop distances now start at zero for each independent configuration while continuing to accumulate within that configuration. The targeted behavior is covered through the API route, with no remaining concrete merge-blocking risk.

Suggested reviewers: omlahore

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 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 primary change: resetting distanceAlongBlock between block configurations.
Linked Issues check	✅ Passed	The change satisfies issue #1427. It scopes blockDistance to each service configuration and adds a regression test for reset and within-configuration accumulation. It does not address unrelated BlockS…
Out of Scope Changes check	✅ Passed	All changes support issue #1427. The implementation change and regression test are limited to resetting distanceAlongBlock between configurations.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[omlahore]

Fix looks right to me. I ran the new test against the merge base and it fails there on the config 1 assertion, so it is a real regression test and not just a passing one.

One thing I noticed while checking it. The bug is visible through the fixture already, and on 25_1, which the tests above it are using. On the merge base /api/where/block/25_1 returns DistanceAlongBlock of 0 for the first configuration and 433750.77 for the first stop of the second one, and both go to 0 with your change. So I think an assertion through callAPIHandler would cover the same thing while also going through the real GetBlockDetails row shape, which the hand built rows kinda skip since they leave the arrival and departure fields at zero.

[priyanshu7739410]

Thanks for the review! I’ve updated the regression test to use the existing 25_1 fixture and exercise the API through callAPIHandler, as suggested. It now verifies both the reset between configurations and accumulation within each configuration. The update is pushed to the PR.

[omlahore]

Test reads right to me now. I dropped the unused import locally and ran it against the merge base and it fails there with expected: 0, actual: 433750.7712378005 on the config 1 assertion, so it still catches the thing it is meant to catch.

The build is red though. gtfsdb is still imported at the top of block_handler_test.go and nothing uses it now that the synthetic rows are gone, so go vet fails under both tag sets and both Test jobs are failing on it. SonarCloud going green threw me for a second, but it does not compile the Go.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[priyanshu7739410]

Thanks! I removed the unused gtfsdb import and pushed the cleanup in 5530963.

The updated regression test passes, and all CI checks are now green.