Skip to content

Persist complete engineering review findings without GBrain - #2855

Open
Focus695 wants to merge 1 commit into
garrytan:mainfrom
Focus695:fix/persist-eng-review-findings
Open

Focus695 wants to merge 1 commit into
garrytan:mainfrom
Focus695:fix/persist-eng-review-findings

Conversation

@Focus695

@Focus695 Focus695 commented Sep 12, 2026

Copy link
Copy Markdown

Engineering reviews currently persist test plans, dashboard metadata, and a durable architecture decision, but a complete findings document is not required when GBrain is unavailable. Ending the session can still lose issue details, user choices, diagrams, and implementation order.

Require /plan-eng-review to save those findings under the resolved state root's projects/<slug>/eng-reviews/ directory after the interactive review. Records include target and commit metadata, preserve unresolved decisions and suppressed findings, avoid filename collisions, and are read back before reporting success. Resume guidance treats saved findings as historical context and checks current target contents and commit before reuse.

This is a scoped follow-up to #721, rebuilt on current main (71f6048). It carries forward only the local findings document. The old upgrade overlay is omitted: current upgrades use fast-forward/autostash and a separate rendering flow, so the old copy-over approach needs a separate design. This PR does not claim that all customization persistence cases are solved.

Validation:

  • Passed Claude and Codex skill generation and --dry-run freshness checks.
  • Passed git diff --check.
  • Installed all 228 locked dependencies, then passed 789 tests across gen-skill-docs, skill-validation, template-context-parity, and context-budget-ratchet.
  • Full free suite did not pass: it was started before dependencies were installed, reported missing-package errors, SwiftSyntax generator and other failures, and was stopped after those failures. The full suite has not been rerun after installation. These failures have not been compared against an unchanged main checkout; their relationship to this change is unverified.
  • Live LLM behavior/evals have not been validated. Keeping this PR as a draft until full validation is complete.

@trunk-io

trunk-io Bot commented Sep 12, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@Focus695
Focus695 marked this pull request as ready for review September 12, 2026 08:49
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.

1 participant