adding missing assets to build #2297

Merged

mfreeman451 merged 2 commits from refs/pull/2297/head into main

2025-10-07 00:09:58 +00:00

mfreeman451 commented

2025-10-06 23:46:03 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1726
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1726
Original created: 2025-10-06T23:46:03Z
Original updated: 2025-10-07T00:10:02Z
Original head: carverauto/serviceradar:chore/missing_web_assets
Original base: main
Original merged: 2025-10-07T00:09:58Z by @mfreeman451

PR Type

Enhancement

Description

Fix web assets packaging with proper directory structure
Add strip_prefix configuration for public assets
Implement copy_to_directory for flattened public files
Update Docker build targets for correct asset paths

Diagram Walkthrough

flowchart LR
  A["web:public_tree"] --> B["copy_to_directory"]
  B --> C["web_public_flat"]
  C --> D["pkg_tar targets"]
  D --> E["Docker images"]
  F["strip_prefix config"] --> G["packaging system"]

File Walkthrough

Relevant files

Configuration changes

packages.bzl

Add strip_prefix for web public assets

packaging/packages.bzl

Add strip_prefix configuration to web public assets
Set prefix to "web/public_bundle" for proper path handling

+1/-0

Enhancement

BUILD.bazel

Restructure web public assets packaging for Docker

docker/images/BUILD.bazel

Import copy_to_directory from aspect_bazel_lib
Create web_public_flat target to flatten public directory structure
Update pkg_tar targets to use flattened public files
Fix package_dir paths for standalone and root configurations

+15/-4

Imported from GitHub pull request. Original GitHub pull request: #1726 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1726 Original created: 2025-10-06T23:46:03Z Original updated: 2025-10-07T00:10:02Z Original head: carverauto/serviceradar:chore/missing_web_assets Original base: main Original merged: 2025-10-07T00:09:58Z by @mfreeman451 --- ### **PR Type** Enhancement ___ ### **Description** - Fix web assets packaging with proper directory structure - Add strip_prefix configuration for public assets - Implement copy_to_directory for flattened public files - Update Docker build targets for correct asset paths ___ ### Diagram Walkthrough ```mermaid flowchart LR A["web:public_tree"] --> B["copy_to_directory"] B --> C["web_public_flat"] C --> D["pkg_tar targets"] D --> E["Docker images"] F["strip_prefix config"] --> G["packaging system"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Configuration changes</strong></td><td><table> <tr> <td> <details> <summary><strong>packages.bzl</strong><dd><code>Add strip_prefix for web public assets</code>                                      </dd></summary> <hr> packaging/packages.bzl <ul><li>Add <code>strip_prefix</code> configuration to web public assets<br> <li> Set prefix to "web/public_bundle" for proper path handling</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1726/files#diff-9bfe2a5141a9e402bb5a5a8fca53b9eea64396ec18108c535392e1054c90b913">+1/-0</a>      </td> </tr> </table></td></tr><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>BUILD.bazel</strong><dd><code>Restructure web public assets packaging for Docker</code>              </dd></summary> <hr> docker/images/BUILD.bazel <ul><li>Import <code>copy_to_directory</code> from aspect_bazel_lib<br> <li> Create <code>web_public_flat</code> target to flatten public directory structure<br> <li> Update pkg_tar targets to use flattened public files<br> <li> Fix package_dir paths for standalone and root configurations</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1726/files#diff-0e4db31c224a8f72ae8e870a849e38a59d74a2c7f7b04347b0b3eb07e20c5a80">+15/-4</a>    </td> </tr> </table></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-10-06 23:46:24 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1726#issuecomment-3374661960
Original created: 2025-10-06T23:46:24Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
⚪	No custom compliance provided Follow the guide to enable custom compliance check.
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1726#issuecomment-3374661960 Original created: 2025-10-06T23:46:24Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </summary></strong> - [ ] Create ticket/issue  </details></td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>No custom compliance provided</strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/'>guide</a> to enable custom compliance check. </details></td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2025-10-06 23:47:25 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1726#issuecomment-3374663490
Original created: 2025-10-06T23:47:25Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Centralize asset path stripping logic The asset path stripping logic is duplicated in two files. It should be centralized in the source target, `//web:public_tree`, to simplify consumers and prevent future inconsistencies. Examples: docker/images/BUILD.bazel [907-915] `copy_to_directory( name = "web_public_flat", srcs = ["//web:public_tree"], out = "public", replace_prefixes = { "web/public_bundle": "", "public_bundle": "", }, )` packaging/packages.bzl [740] `"strip_prefix": "web/public_bundle",` Solution Walkthrough: Before: `# In docker/images/BUILD.bazel copy_to_directory( name = "web_public_flat", srcs = ["//web:public_tree"], replace_prefixes = { "web/public_bundle": "", ... }, ) pkg_tar( srcs = [":web_public_flat"], ... ) # In packaging/packages.bzl WEB_DEB_PACKAGE_FILES = [ ... { "src": "//web:public_tree", "strip_prefix": "web/public_bundle", }, ]` After: `# In web/BUILD.bazel (hypothetical change) filegroup( name = "public_tree", srcs = glob(["public_bundle/**"]), strip_prefix = "public_bundle", # Logic is centralized here ) # In docker/images/BUILD.bazel pkg_tar( srcs = ["//web:public_tree"], # Directly consume the source package_dir = "/app/public", ... ) # In packaging/packages.bzl WEB_DEB_PACKAGE_FILES = [ ... { "src": "//web:public_tree", # No strip_prefix needed }, ]` Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies duplicated path-stripping logic in `docker/images/BUILD.bazel` and `packaging/packages.bzl`, proposing a superior, centralized solution that improves long-term maintainability.	Medium
Possible issue	✅ ~~Fix incorrect asset packaging path~~ Suggestion Impact: The commit updated web_public_root_amd64 to package into /app/public (and similarly adjusted related targets), aligning with the suggestion’s intent. However, it did not add strip_prefix. code diff: `pkg_tar( name = "web_public_root_amd64", - srcs = [":web_public_flat"], - package_dir = "/app", + srcs = ["//web:public_flat"], + package_dir = "/app/public", ) pkg_tar( name = "web_public_standalone_amd64", - srcs = [":web_public_flat"], - package_dir = "/app/.next/standalone", + srcs = ["//web:public_flat"], + package_dir = "/app/.next/standalone/public", )` In the `web_public_root_amd64` target, change the packaging logic to explicitly set `package_dir` to `/app/public` and add `strip_prefix = "public"` for better clarity and robustness. docker/images/BUILD.bazel [917-921] `pkg_tar( name = "web_public_root_amd64", srcs = [":web_public_flat"], - package_dir = "/app", + package_dir = "/app/public", + strip_prefix = "public", )` `[Suggestion processed]` Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that the current code and the proposed change produce the same output structure. The proposed change using `package_dir` and `strip_prefix` is a more explicit and robust pattern for packaging directory contents, improving maintainability.	Low
General	✅ ~~Improve asset packaging path configuration~~ Suggestion Impact: The commit changed package_dir for web_public_standalone_amd64 to "/app/.next/standalone/public", aligning with the suggestion. However, it did not add strip_prefix; instead it switched srcs to use //web:public_flat. code diff: `pkg_tar( name = "web_public_standalone_amd64", - srcs = [":web_public_flat"], - package_dir = "/app/.next/standalone", + srcs = ["//web:public_flat"], + package_dir = "/app/.next/standalone/public", )` In the `web_public_standalone_amd64` target, make the packaging logic more explicit by setting `package_dir` to the full path `/app/.next/standalone/public` and adding `strip_prefix = "public"`. docker/images/BUILD.bazel [923-927] `pkg_tar( name = "web_public_standalone_amd64", srcs = [":web_public_flat"], - package_dir = "/app/.next/standalone", + package_dir = "/app/.next/standalone/public", + strip_prefix = "public", )` `[Suggestion processed]` Suggestion importance[1-10]: 5 __ Why: Similar to the first suggestion, this proposes a more explicit and robust pattern for packaging directory contents by using `package_dir` with the full path and `strip_prefix`. This improves the clarity and maintainability of the build configuration.	Low
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1726#issuecomment-3374663490 Original created: 2025-10-06T23:47:25Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Centralize asset path stripping logic</summary> ___ **The asset path stripping logic is duplicated in two files. It should be <br>centralized in the source target, <code>//web:public_tree</code>, to simplify consumers and <br>prevent future inconsistencies.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1726/files#diff-0e4db31c224a8f72ae8e870a849e38a59d74a2c7f7b04347b0b3eb07e20c5a80R907-R915">docker/images/BUILD.bazel [907-915]</a> </summary> ```starlark copy_to_directory( name = "web_public_flat", srcs = ["//web:public_tree"], out = "public", replace_prefixes = { "web/public_bundle": "", "public_bundle": "", }, ) ``` </details> <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1726/files#diff-9bfe2a5141a9e402bb5a5a8fca53b9eea64396ec18108c535392e1054c90b913R740-R740">packaging/packages.bzl [740]</a> </summary> ```starlark "strip_prefix": "web/public_bundle", ``` </details> ### Solution Walkthrough: #### Before: ```starlark # In docker/images/BUILD.bazel copy_to_directory( name = "web_public_flat", srcs = ["//web:public_tree"], replace_prefixes = { "web/public_bundle": "", ... }, ) pkg_tar( srcs = [":web_public_flat"], ... ) # In packaging/packages.bzl WEB_DEB_PACKAGE_FILES = [ ... { "src": "//web:public_tree", "strip_prefix": "web/public_bundle", }, ] ``` #### After: ```starlark # In web/BUILD.bazel (hypothetical change) filegroup( name = "public_tree", srcs = glob(["public_bundle/**"]), strip_prefix = "public_bundle", # Logic is centralized here ) # In docker/images/BUILD.bazel pkg_tar( srcs = ["//web:public_tree"], # Directly consume the source package_dir = "/app/public", ... ) # In packaging/packages.bzl WEB_DEB_PACKAGE_FILES = [ ... { "src": "//web:public_tree", # No strip_prefix needed }, ] ``` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies duplicated path-stripping logic in `docker/images/BUILD.bazel` and `packaging/packages.bzl`, proposing a superior, centralized solution that improves long-term maintainability. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>✅ <s>Fix incorrect asset packaging path</s></summary> ___ <details><summary><b>Suggestion Impact:</b></summary>The commit updated web_public_root_amd64 to package into /app/public (and similarly adjusted related targets), aligning with the suggestion’s intent. However, it did not add strip_prefix. code diff: ```diff pkg_tar( name = "web_public_root_amd64", - srcs = [":web_public_flat"], - package_dir = "/app", + srcs = ["//web:public_flat"], + package_dir = "/app/public", ) pkg_tar( name = "web_public_standalone_amd64", - srcs = [":web_public_flat"], - package_dir = "/app/.next/standalone", + srcs = ["//web:public_flat"], + package_dir = "/app/.next/standalone/public", ) ``` </details> ___ **In the <code>web_public_root_amd64</code> target, change the packaging logic to explicitly <br>set <code>package_dir</code> to <code>/app/public</code> and add <code>strip_prefix = "public"</code> for better <br>clarity and robustness.** [docker/images/BUILD.bazel [917-921]](https://github.com/carverauto/serviceradar/pull/1726/files#diff-0e4db31c224a8f72ae8e870a849e38a59d74a2c7f7b04347b0b3eb07e20c5a80R917-R921) ```diff pkg_tar( name = "web_public_root_amd64", srcs = [":web_public_flat"], - package_dir = "/app", + package_dir = "/app/public", + strip_prefix = "public", ) ``` `[Suggestion processed]` <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly identifies that the current code and the proposed change produce the same output structure. The proposed change using `package_dir` and `strip_prefix` is a more explicit and robust pattern for packaging directory contents, improving maintainability. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=1>General</td> <td> <details><summary>✅ <s>Improve asset packaging path configuration</s></summary> ___ <details><summary><b>Suggestion Impact:</b></summary>The commit changed package_dir for web_public_standalone_amd64 to "/app/.next/standalone/public", aligning with the suggestion. However, it did not add strip_prefix; instead it switched srcs to use //web:public_flat. code diff: ```diff pkg_tar( name = "web_public_standalone_amd64", - srcs = [":web_public_flat"], - package_dir = "/app/.next/standalone", + srcs = ["//web:public_flat"], + package_dir = "/app/.next/standalone/public", ) ``` </details> ___ **In the <code>web_public_standalone_amd64</code> target, make the packaging logic more <br>explicit by setting <code>package_dir</code> to the full path <code>/app/.next/standalone/public</code> <br>and adding <code>strip_prefix = "public"</code>.** [docker/images/BUILD.bazel [923-927]](https://github.com/carverauto/serviceradar/pull/1726/files#diff-0e4db31c224a8f72ae8e870a849e38a59d74a2c7f7b04347b0b3eb07e20c5a80R923-R927) ```diff pkg_tar( name = "web_public_standalone_amd64", srcs = [":web_public_flat"], - package_dir = "/app/.next/standalone", + package_dir = "/app/.next/standalone/public", + strip_prefix = "public", ) ``` `[Suggestion processed]` <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: Similar to the first suggestion, this proposes a more explicit and robust pattern for packaging directory contents by using `package_dir` with the full path and `strip_prefix`. This improves the clarity and maintainability of the build configuration. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>