Skip to content

feat(strategypro): reverse-engineer flow — Outcomes pilot (submodule bump + tests)#3651

Closed
gloriafolaron wants to merge 2 commits into
masterfrom
feat/logic-model-reverse
Closed

feat(strategypro): reverse-engineer flow — Outcomes pilot (submodule bump + tests)#3651
gloriafolaron wants to merge 2 commits into
masterfrom
feat/logic-model-reverse

Conversation

@gloriafolaron

Copy link
Copy Markdown
Contributor

Summary

Core companion to Leantime/plugins#74 — bumps the StrategyPro submodule pointer to the plugin's feat/logic-model-reverse HEAD and lands unit tests for the reverse flow's read half.

What's in the PR

  • Submodule pointer bump for app/Plugins.
  • tests/Unit/app/Plugins/StrategyPro/Services/WorkAdapterTest.php — 7 tests, 20 assertions covering the four highest-regression-risk behaviors on WorkAdapter:
    • Empty strategy → empty completeness.
    • All goals hidden → "already reflected" completeness.
    • Metric-based classification splits with-target from no-target.
    • Percent goal with endValue=0 is treated as no-target (avoids "0% of 0" nonsense).
    • Hidden goals are removed from candidates but counted honestly.
    • Sparse-completeness message has no unfilled sprintf placeholders.
    • Scope shape passes through cleanly.

The plugin PR carries the actual services, HxControllers, and templates. See there for the full spec + verification checklist.

Merge order

  • Plugin PR merges to plugins:main.
  • This PR's submodule pointer bumps forward to the final plugin merge commit before the core PR merges.
  • No core code changes here — pointer bump + tests only.

Test plan

  • ./vendor/bin/phpunit tests/Unit/app/Plugins/StrategyPro/Services/WorkAdapterTest.php — 7 tests pass.
  • Pint + PHPStan clean on the test file.
  • Manual QA of the reverse flow per the plugin PR checklist.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings July 15, 2026 21:11
@gloriafolaron
gloriafolaron requested a review from a team as a code owner July 15, 2026 21:11
@gloriafolaron
gloriafolaron requested review from marcelfolaron and removed request for a team July 15, 2026 21:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR advances the StrategyPro “reverse-engineer flow” Outcomes pilot by bumping the app/Plugins submodule pointer to the corresponding plugin work and adding unit coverage for the reverse flow’s read/adaptation logic (via WorkAdapter).

Changes:

  • Bump app/Plugins submodule to the StrategyPro plugin HEAD implementing the reverse flow read logic.
  • Add a focused unit test suite for WorkAdapter::adapt() covering goal classification, hidden/provenance filtering outcomes, completeness messaging, and scope passthrough.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
app/Plugins (submodule) Updates StrategyPro plugin code version used by core (reverse-flow Outcomes pilot).
tests/Unit/app/Plugins/StrategyPro/Services/WorkAdapterTest.php Adds 7 unit tests validating WorkAdapter adaptation output shape, classification rules, and completeness messaging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +37 to +40
class WorkAdapterTest extends TestCase
{
public function test_empty_strategy_returns_empty_completeness(): void
{
$this->assertSame(1, $out['outcomes']['hiddenCount']);
}

public function test_sparse_level_when_one_or_two_candidates(): void
Copilot AI review requested due to automatic review settings July 15, 2026 21:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment on lines +37 to +39
class WorkAdapterTest extends TestCase
{
public function test_empty_strategy_returns_empty_completeness(): void
Comment on lines +21 to +23
class ReverseStageTest extends TestCase
{
public function test_impact_can_never_be_seeded(): void
Copilot AI review requested due to automatic review settings July 15, 2026 22:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

$this->assertSame(1, $out['outcomes']['hiddenCount']);
}

public function test_sparse_level_when_one_or_two_candidates(): void
Copilot AI review requested due to automatic review settings July 15, 2026 23:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment on lines +37 to +40
class WorkAdapterTest extends TestCase
{
public function test_empty_strategy_returns_empty_completeness(): void
{
Comment on lines +21 to +24
class ReverseStageTest extends TestCase
{
public function test_impact_can_never_be_seeded(): void
{
Copilot AI review requested due to automatic review settings July 16, 2026 00:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment on lines +37 to +39
class WorkAdapterTest extends TestCase
{
public function test_empty_strategy_returns_empty_completeness(): void
Comment on lines +21 to +23
class ReverseStageTest extends TestCase
{
public function test_impact_can_never_be_seeded(): void
Copilot AI review requested due to automatic review settings July 16, 2026 00:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment on lines +37 to +38
class WorkAdapterTest extends TestCase
{
Comment on lines +21 to +22
class ReverseStageTest extends TestCase
{
Copilot AI review requested due to automatic review settings July 16, 2026 02:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 61 out of 61 changed files in this pull request and generated 5 comments.

Comment thread app/Domain/Install/Repositories/Install.php
Comment on lines 2274 to 2276
'acceptanceCriteria' => $values['acceptanceCriteria'] ?? '',
'outcomeImpact' => $values['outcomeImpact'] ?? '',
'editFrom' => $values['editFrom'] ?? '',
Comment on lines +108 to +110
// Filter to open tickets only — DONE work doesn't need capacity.
$openTickets = array_values(array_filter($tickets, fn ($t) => ! $this->isDone($t)));

Comment on lines +111 to +123
$ts = strtotime($rawDate);
if ($ts === false) {
$errors[] = "line {$lineNo}: unparseable dateRecorded '{$rawDate}'";

continue;
}

$rows[] = [
'itemId' => $itemId,
'value' => (float) $rawValue,
'userId' => ($idxUser !== false && isset($record[$idxUser]) && $record[$idxUser] !== '') ? (int) $record[$idxUser] : null,
'dateRecorded' => date('Y-m-d H:i:s', $ts),
];
Comment on lines +17 to +24
/**
* Shared, period-aware reporting engine feeding the project, plan and strategy report screens.
*
* All methods operate on explicit project-id sets so higher levels (plan = child projects,
* strategy = descendant projects, later company = all projects) compose the same building
* blocks. Ids are filtered to projects the requesting user may view before any query runs,
* and the repository re-applies the SQL access predicate — defense in depth.
*/
Copilot AI review requested due to automatic review settings July 16, 2026 02:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 61 out of 61 changed files in this pull request and generated 6 comments.

Comment on lines +108 to +110
// Filter to open tickets only — DONE work doesn't need capacity.
$openTickets = array_values(array_filter($tickets, fn ($t) => ! $this->isDone($t)));

Comment on lines +350 to +356
private function isDone(array $t): bool
{
// Numeric statuses vary per project; treat -1 and 0 as closed/archived by convention.
$status = (int) ($t['status'] ?? 0);

return $status <= 0;
}
Comment thread app/Domain/Install/Repositories/Install.php
@@ -0,0 +1,334 @@
<?php

namespace Tests\Unit\App\Domain\Reports\Services;
@@ -0,0 +1,175 @@
<?php

namespace Tests\Unit\App\Domain\Reports\Models;
public string $appVersion = '3.9.8';

public string $dbVersion = '3.5.20';
public string $dbVersion = '3.5.22';
@marcelfolaron

Copy link
Copy Markdown
Collaborator

Maintainer triage (advisory — merge authority stays with @marcelfolaron / @broskees) · first pass · status: ready-for-review (blocked by merge-order) · priority: P2 · next action: hold for plugin #74 + verify submodule pointer · owner: @marcelfolaron / @broskees

1. Intent: Core companion to plugins #74 — bumps the app/Plugins submodule pointer to the StrategyPro feat/logic-model-reverse HEAD and lands WorkAdapterTest (7 tests, 20 assertions) covering the reverse flow's read half. No core code changes — pointer bump + tests only.

2. Change requests:

  1. ⛔ CR Documentation #1 — submodule pointer coupling (same class of blocker as feat(core): Resources contract surface — gateway, registry, value objects, shared blade primitives #3645). This PR moves app/Plugins to the current head of the unmerged plugin PR Show all tickets choose milestone out of screen #74. If feat(strategypro): reverse-engineer flow — Outcomes pilot (submodule bump + tests) #3651 merges before Show all tickets choose milestone out of screen #74 lands on plugins:main, master's submodule references a commit that isn't on the plugin default branch — a broken checkout for anyone pulling recursively. Per the standing convention: land the plugin PR first, then do the pointer bump as the explicit last step. This is the single merge-blocker.
  2. ⚠ CR Gitignore #2 — test placement vs. tested code. WorkAdapterTest lives in core but exercises WorkAdapter, which ships in the plugin (Show all tickets choose milestone out of screen #74). Confirm the test can actually resolve the plugin class in core's CI (submodule checked out during the test run) — otherwise these 7 tests silently no-op or error in the core pipeline. If core CI doesn't init submodules, this coverage isn't real where it's running.
  3. ✅ Test content looks right on paper — the 7 behaviors (empty→empty, all-hidden→"already reflected", metric target split, endValue=0→no-target, hidden-but-counted, no unfilled sprintf placeholders, scope pass-through) map cleanly to the §11 rules in the plugin PR. Verify green in the suite.

3. Acceptance checklist (before merge):

  1. Plugin Show all tickets choose milestone out of screen #74 merged to plugins:main first; this PR's pointer bumps to the final Show all tickets choose milestone out of screen #74 merge commit (CR Documentation #1).
  2. Core CI initializes the submodule so WorkAdapterTest runs against the real WorkAdapter (CR Gitignore #2) — confirm 7/7 green, not skipped.
  3. CI green: Pint + PHPStan level 5 + full unit suite on the current head.

4. CI status: ⚠️ Confirm check-runs green in the UI; a head with zero/failing checks must not merge. Merge order: #74#3651 (pointer bump last).

Advisory review only — not an approval, and I am not marking this ready or merging.

@marcelfolaron

Copy link
Copy Markdown
Collaborator

Maintainer triage — CI follow-up (advisory · merge authority stays with @marcelfolaron / @broskees) · status: blocked (CI + merge-order) · priority: P2 · next action: fix failing unit job before merge

Read the check-runs on head 2cce2b3 directly (12 total). Status:

  • unit — FAILED (completed, 2 annotations). This is the exact risk called out as CR Gitignore #2 in the first pass: WorkAdapterTest lives in core but exercises WorkAdapter, which ships in the unmerged plugin PR #74. The core unit job either can't resolve the plugin class (submodule not initialized to a commit that contains WorkAdapter) or the 7 tests genuinely fail. This must be green before merge — a failing unit suite is a hard blocker.
  • CodeQL — success (no new alerts).
  • copilot-pull-request-reviewer — success.

Confirmed merge-order state: plugins #74 is still open / unmerged as of this pass. Per CR #1, this pointer-bump PR must not merge until #74 lands on plugins:main, then re-bump to the final #74 merge commit. So there are now two independent gates:

  1. plugins Show all tickets choose milestone out of screen #74 merged first; pointer re-bumped to its final merge commit.
  2. unit job green on the re-bumped head (submodule initialized so the 7 tests run against the real WorkAdapter, not skipped/errored) — currently failing.

Recommend @gloriafolaron pull the 2 annotations from the unit run to confirm whether it's a submodule-init gap in core CI or a real test failure; the former is a CI-config fix, the latter is a code fix. Either way #3651 stays not-mergeable until it's green.

Advisory review only — not an approval, and I am not marking this ready or merging.

Rebuilds #3651 cleanly on current master. Its plugin-side reverse-engineer
code (WorkAdapter, ReverseStage, guide-mode authoring) is now merged to plugin
main (was plugins #74/#75), and the reports lineage it carried is superseded by
#3643 (already on master) — so all that remains as core is this test coverage
plus the submodule pointer bump to plugin main.

- WorkAdapterTest: 7 tests / 20 assertions (completeness, metric-target
  classification, hidden-goal counting, sparse levels).
- ReverseStageTest: 9 tests / 44 assertions (confidence→selection, canvas order
  inputs→impact, guide prompts).
Both green against plugin main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GYSPNgLdUwEnxgYqTxMc85
Copilot AI review requested due to automatic review settings July 21, 2026 14:50
@gloriafolaron
gloriafolaron force-pushed the feat/logic-model-reverse branch from 2cce2b3 to d8fcec3 Compare July 21, 2026 14:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

tests/Unit/app/Plugins/StrategyPro/Services/WorkAdapterTest.php:39

  • These plugin unit tests will fatal in OSS/CI checkouts where app/Plugins is empty (private submodule). Other plugin tests in this repo guard against missing plugin classes by skipping (see tests/Unit/app/Plugins/PgmPro/Domain/Resources/Services/ResourceStructureServiceTest.php:42-49). Add the same skip here so the core repo test suite remains runnable without private plugins.
class WorkAdapterTest extends TestCase
{
    public function test_empty_strategy_returns_empty_completeness(): void

tests/Unit/app/Plugins/StrategyPro/Services/ReverseStageTest.php:23

  • This test will fatal when the StrategyPro plugin isn't available (core OSS CI often checks out without the private app/Plugins submodule). Add a guard to skip when ReverseStage can't be autoloaded, consistent with the existing plugin test pattern in tests/Unit/app/Plugins/PgmPro/....
class ReverseStageTest extends TestCase
{
    public function test_impact_can_never_be_seeded(): void

Copilot AI review requested due to automatic review settings July 21, 2026 19:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

tests/Unit/app/Plugins/StrategyPro/Services/WorkAdapterTest.php:39

  • These unit tests will fatal in OSS/CI checkouts where app/Plugins is empty (private submodule). Other plugin unit tests (e.g. tests/Unit/app/Plugins/PgmPro/...) guard this by skipping when the plugin class is not present. Add a setUp() that markTestSkipped() when WorkAdapter is unavailable so core CI remains green when submodules aren't checked out.
class WorkAdapterTest extends TestCase
{
    public function test_empty_strategy_returns_empty_completeness(): void

tests/Unit/app/Plugins/StrategyPro/Services/ReverseStageTest.php:23

  • Like other plugin unit tests, this file should skip cleanly when the private app/Plugins submodule isn't present; otherwise use Leantime\Plugins\StrategyPro\Services\ReverseStage; triggers a fatal before PHPUnit can report a skipped test. Add a setUp() guard that markTestSkipped() when ReverseStage can't be autoloaded.
class ReverseStageTest extends TestCase
{
    public function test_impact_can_never_be_seeded(): void

@gloriafolaron

Copy link
Copy Markdown
Contributor Author

Closing per Marcel: this PR's only content was two StrategyPro unit tests (WorkAdapterTest, ReverseStageTest) plus a submodule bump. Plugin tests can't run in core CI — the private app/Plugins submodule isn't checked out, so the SUT classes are absent and the unit job errors with class-not-found.

The tests now live where they can actually run: plugins PR Leantime/plugins#78 (StrategyPro/Tests/Unit/Services/…). The submodule bump will ride along with a normal release bump rather than a standalone PR.

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.

3 participants