Skip to content

fix: align review contract with BuildShip and harden deploy placement - #54

Merged
sgardoll merged 2 commits into
mainfrom
fix/review-summary-and-deploy-placement
Aug 12, 2026
Merged

fix: align review contract with BuildShip and harden deploy placement#54
sgardoll merged 2 commits into
mainfrom
fix/review-summary-and-deploy-placement

Conversation

@sgardoll

@sgardoll sgardoll commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Fixes two symptoms against the runPipeline backend: an empty review summary, code artifacts landing in the wrong FlutterFlow folder, and makes the review score + deploy guide mandatory on every generation.

Review (must ship every time):

  • buildReviewPrompt: request bundleReview as {status, score, summary, manualActions, findings} so the request matches what the parser reads and what BuildShip's system prompt now requires.
  • reviewPresentation: a missing written summary no longer falls back to the bundle description or a useless placeholder. It synthesizes an actionable deploy guide (verdict, per-artifact pass/warn/fail counts, the manual steps to take in the user's own FlutterFlow project) and explicitly flags when no numeric 0-100 score was returned, so a missing score is never mistaken for a pass.
  • score + summary are enforced on the BuildShip prompt side (see paired PR) — there is no valid review without them.

Placement:

  • normalizeArtifact: a missing artifactType no longer silently becomes a CustomWidget (misrouting classes/functions into lib/custom_code/widgets/); it defaults to a standalone code file (lib/custom_code/ root) and surfaces a warning. Honors artifact.deployPath.
  • bundleDeployPlanner: honors deployPath; merges multiple CustomFunction artifacts into the single shared lib/flutter_flow/custom_functions.dart (verified as FlutterFlow's real layout) instead of erroring on a duplicate-path collision.
  • flutterFlowArtifactValidation: corrected TYPE_FILE_HINTS to the real exported layout.

Pre-existing project state is preserved (not overwritten):

  • Custom functions: the push uses FlutterFlow's functions_map and diffs generated function names against the real exported custom_functions.dart (functions_to_add = new names only, never delete), so functions already in the user's project stay.
  • pubspec.yaml: resolveProjectPubspec reads the real remote pubspec and only merges in the generated packages; it aborts rather than overwrite if the project pubspec can't be read.

Paired with https://github.com/sgardoll/buildship/pull/62. All 159 tests pass.

- buildReviewPrompt: request bundleReview {status,score,summary,findings} to
  match what the review presentation reads, so the summary section shows a
  real score and verdict instead of being empty.
- normalizeArtifact: a missing artifactType no longer silently becomes a
  CustomWidget (which misrouted classes/functions into lib/custom_code/widgets);
  it now defaults to a standalone code file under lib/custom_code/ and surfaces
  a warning. Honor an explicit artifact.deployPath.
- bundleDeployPlanner: honor artifact.deployPath; merge multiple CustomFunction
  artifacts into the single shared lib/flutter_flow/custom_functions.dart
  (verified as FlutterFlow's real layout) instead of erroring/colliding.
- flutterFlowArtifactValidation: correct TYPE_FILE_HINTS to the real exported
  FlutterFlow layout.
@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR aligns the generated review request and presentation with the BuildShip review contract while correcting FlutterFlow deployment paths and shared custom-function planning.

  • Defaults untyped artifacts to standalone code files and preserves explicit deployment paths.
  • Consolidates custom-function artifacts into FlutterFlow’s shared custom_functions.dart target.
  • Synthesizes actionable review summaries when the backend omits written summary text or a numeric score.
  • Updates FlutterFlow path hints and associated contract, planner, validation, and presentation tests.

Confidence Score: 4/5

The PR does not yet appear safe to merge because combining complete custom-function artifacts can still generate an invalid Dart file when a later artifact contains directives.

The shared-file merge copies each complete artifact body in sequence, so imports, library declarations, or part directives from later artifacts remain after earlier function declarations and can make the generated custom_functions.dart fail Dart analysis or compilation.

Files Needing Attention: src/bundleDeployPlanner.js, src/bundleDeployPlanner.test.js

Important Files Changed

Filename Overview
src/artifactBundle.js Changes the missing-type fallback to a standalone code file, preserves deployPath, and emits a placement warning.
src/bundleDeployPlanner.js Honors explicit deployment paths and combines shared custom-function entries before downstream synchronization.
src/flutterFlowArtifactValidation.js Updates deployment hints to match FlutterFlow’s exported project layout.
src/pipelineContracts.js Requests the nested bundleReview fields consumed by the review parser.
src/reviewPresentation.js Generates a verdict, artifact counts, manual steps, and missing-score notice when no written summary is returned.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Pipeline artifact bundle] --> B[Normalize artifact type and deployPath]
    B --> C[Build deployment entries]
    C --> D{CustomFunction at shared path?}
    D -- Multiple --> E[Combine into custom_functions.dart]
    D -- No --> F[Keep inferred or explicit target]
    E --> G[Validate deployment plan]
    F --> G
    G --> H[FlutterFlow synchronization]
    A --> I[Build review request]
    I --> J[Parse bundleReview]
    J --> K{Written summary present?}
    K -- Yes --> L[Present supplied summary and score]
    K -- No --> M[Synthesize verdict, counts, and manual steps]
Loading

Reviews (2): Last reviewed commit: "feat: require review score+summary+manua..." | Re-trigger Greptile

Comment on lines +126 to +128
content: functionEntries
.map(({ entry }) => `// ${entry.artifactId}\n${entry.content}`)
.join("\n\n"),

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.

P1 Merged Dart directives break compilation

When two custom-function artifacts contain complete Dart content and a later artifact includes an import, library, or part directive, this verbatim concatenation places that directive after the first function declaration, causing the generated custom_functions.dart to fail Dart analysis or compilation.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/bundleDeployPlanner.js
Line: 126-128

Comment:
**Merged Dart directives break compilation**

When two custom-function artifacts contain complete Dart content and a later artifact includes an `import`, `library`, or `part` directive, this verbatim concatenation places that directive after the first function declaration, causing the generated `custom_functions.dart` to fail Dart analysis or compilation.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

…ummary

- buildReviewPrompt: request bundleReview.manualActions alongside status/score/summary/findings.
- reviewPresentation: a missing written summary no longer falls back to the bundle
  description or a useless placeholder. It synthesizes an actionable deploy guide
  (overall verdict, per-artifact pass/warn/fail counts, the manual steps the user must
  take in their own FlutterFlow project) and explicitly flags when no numeric 0-100
  score was returned so a missing score is never mistaken for a pass.
@sgardoll

Copy link
Copy Markdown
Owner Author

@greptileai

@sgardoll
sgardoll merged commit 86d2899 into main Aug 12, 2026
6 checks passed
sgardoll added a commit that referenced this pull request Aug 13, 2026
main landed PR #54, which reworked the same review contract this branch
touches. Both sides added manualActions to the review's output requirements,
independently.

Resolution keeps main's shape and this branch's constraint:
- outputRequirements uses main's `bundleReview` key, not this branch's
  `overall`. It mirrors the object the review stage actually emits, so it is
  the better name; the auto-merge had also duplicated manualActions in that
  list, now a single entry.
- The manualActions definition/exclude/preferEmpty block is kept. main asks
  the review FOR manual actions but never bounds them, which is what let the
  review volunteer work FlutterFlow already does.
- Dropped this branch's `assert.doesNotMatch(prompt, /bundleReview/)`. It
  guarded against leaking system-prompt vocabulary while the key was
  `overall`; with main's rename bundleReview is the contract's own key, so the
  assertion is now false by construction.

dist/index.html was regenerated rather than hand-merged.

Suite 196/196.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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