Skip to content

feat(planning): guarded tool to restore shift-1 ids damaged by the planning read - #910

Merged
renemadsen merged 2 commits into
masterfrom
feat/shift-id-repair
Oct 4, 2026
Merged

renemadsen merged 2 commits into
masterfrom
feat/shift-id-repair

Conversation

@renemadsen

Copy link
Copy Markdown
Member

Summary

The plugin's planning read used to run Start1Id /= 5 + 1 (integer division by 6) and save the row. That was fixed in plugin #1732. Before the fix it damaged shift-1 ids on about 3,300 one-minute rows. On a few five-minute shifts that cross midnight it also overwrote the stop stamp. This PR adds a guarded tool that puts the original values back from a manifest.

  • ShiftIdRepair.ApplyAsync runs inside one transaction through the execution strategy and locks the manifest rows with FOR UPDATE. For each row it checks that:

    • the row still exists for the same worker and date;
    • its Version and both ids still match the manifest;
    • the row is not reconciled, and its date is not locked by DayLock.

    If all checks pass, it sets the ids, plus the stamps when the manifest gives them, and saves through PnBase.Update, which writes a version row. Hours and flex are never touched. Rows that are Changed, Missing or Locked are reported and do not block the others. A lost commit acknowledgement is detected the same way as in the flex repair tool.

  • tools/ShiftIdRepair is a console app that is not in the .sln; CI builds it.

    • Dry run is the default. --apply requires --confirm-database.
    • It works in chunks of 200 lines and refuses an --out folder that is not empty.
    • It writes and fsyncs the before-image before any report row. A reversed manifest undoes a run.
    • With --apply it stops at the first chunk that throws.
  • ShiftIdRepairCsv reads and writes the manifest and the before-image using invariant culture. The Date column accepts a bare date.

The manifests are built offline from PlanRegistrationVersions: each id gets the value it had just before the divide-by-6, checked against the row's stamps. They hold row ids and numbers only and stay out of the repository.

Tests

ShiftIdRepairTests (DbTestFixture) covers:

  • repair, including the version row;
  • a dry run that writes nothing;
  • Changed, Missing and Locked;
  • hours left untouched;
  • a rejected line that does not block the others;
  • undo with a reversed manifest.

ShiftIdRepairCsvTests covers the header, column count, duplicate ids, the da-DK culture, a bare Date, and the round trips.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VDsVLaH7SqGi1bFk8nAtP1

…anning read

A planning-read workaround (removed in the plugin) divided Start1Id/Stop1Id
by six and, on five-minute rows crossing midnight, overwrote the stop
stamp. ShiftIdRepair applies a manifest of the original values: per row it
checks the row still has the version and ids seen when the manifest was
built and is not on a reconciled day, then sets the ids (and stamps when
given) through PnBase.Update so each change gets a version row. Dry run by
default; --apply requires --confirm-database; the before-image is written
to disk first and a reversed manifest undoes the repair.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDsVLaH7SqGi1bFk8nAtP1
Copilot AI balanced review requested due to automatic review settings October 4, 2026 13:29

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

The day-lock check can race with concurrent reconciliation of a later row for the same worker.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Adds a guarded maintenance tool to restore corrupted shift IDs and timestamps from offline manifests.

Changes:

  • Adds transactional repair logic, CSV handling, and reversible before-images.
  • Adds a dry-run-first console application with safety checks and reporting.
  • Adds repair/CSV tests and CI builds for the standalone tool.
File Description
tools/​ShiftIdRepair/​ShiftIdRepair.csproj Defines the console tool.
tools/​ShiftIdRepair/​README.md Documents operation, recovery, and safeguards.
tools/​ShiftIdRepair/​Program.cs Implements CLI orchestration and reporting.
Microting.TimePlanningBase/​Infrastructure/​Helpers/​ShiftIdRepairTypes.cs Defines repair records and outcomes.
Microting.TimePlanningBase/​Infrastructure/​Helpers/​ShiftIdRepairCsv.cs Parses and writes manifests and before-images.
Microting.TimePlanningBase/​Infrastructure/​Helpers/​ShiftIdRepair.cs Implements transactional guarded repairs.
Microting.TimePlanningBase.Tests/​ShiftIdRepairTests.cs Tests database repair behavior.
Microting.TimePlanningBase.Tests/​ShiftIdRepairCsvTests.cs Tests CSV validation and round trips.
.github/​workflows/​dotnet-core.yml Builds the tool in release CI.
.github/​workflows/​dotnet-core-pr.yml Builds the tool in PR CI.
.github/​workflows/​dotnet-core-master.yml Builds the tool on master CI.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…ng the day lock

Locking only the manifest rows let a reconciliation of a later day of the
same worker commit between the reconciled-boundary read and our commit.
Reconciliation updates that worker's PlanRegistrations rows, so locking all
of the chunk's workers' rows first makes it wait for our commit and keeps
the boundary valid until then.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDsVLaH7SqGi1bFk8nAtP1
Copilot AI balanced review requested due to automatic review settings October 4, 2026 13:43

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

The utility performs live transactional data repair with broad locking, retry, and recovery semantics requiring final human validation.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Reject duplicate --apply flags consistently

tools/​ShiftIdRepair/​Program.cs:274

Repeated value flags are rejected, but --apply bypasses that duplicate check, so --apply --apply ... is accepted despite the documented rule that repeated flags are invocation errors. Check apply before setting it and return the same duplicate-flag error used for value flags.

@renemadsen
renemadsen merged commit d0d5883 into master Oct 4, 2026
3 checks passed
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.

2 participants