fix: Improve SEO generator diagnostics #133
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @gocanto, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the diagnostic capabilities of the SEO generator. It introduces a mechanism to explicitly report the CGO availability status, which is vital for understanding environment-specific behaviors. Furthermore, it integrates comprehensive debug logging across the entire SEO generation pipeline and refines error handling by providing contextual information with each propagated error, thereby streamlining the debugging process and improving overall system observability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully improves the diagnostics for the SEO generator by adding more detailed logging and wrapping errors to provide better context. The use of build tags to report CGO availability is a good addition. The changes are well-aligned with the stated goals. I have added a couple of suggestions to enhance maintainability and performance by refactoring some repetitive code and caching data to avoid redundant fetches.
gocanto
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker-compose.yml (1)
93-99: Enable CGO at build time via build‐arg
The API Dockerfile installs CGO deps but doesn’t accept CGO_ENABLED—add:
- In docker/dockerfile-api:
ARG CGO_ENABLED ENV CGO_ENABLED=${CGO_ENABLED}- In docker-compose.yml under the
apiservice’sbuild.args:build: args: - CGO_ENABLED=1
♻️ Duplicate comments (1)
metal/cli/seo/generator.go (1)
15-16: Breaks non‑CGO builds; split CGO detection via build tags and remove runtime/cgo importBlank‑importing
runtime/cgoand hardcodingcgoEnabled = trueforces CGO and fails whenCGO_ENABLED=0(compiler excludes runtime/cgo). This contradicts “report CGO availability” and removes the pure‑Go path. Use build‑tagged files to set cgoEnabled instead.Apply within this file:
- _ "runtime/cgo" - ... -const cgoEnabled = trueAdd new files:
cgo_enabled.go
//go:build cgo package seo import _ "runtime/cgo" // ensures presence when built with CGO const cgoEnabled = truecgo_disabled.go
//go:build !cgo package seo const cgoEnabled = falseThe existing
fmt.Printf("CGO enabled: %t", cgoEnabled)will now accurately reflect build mode without breaking pure‑Go builds.Also applies to: 28-28
🧹 Nitpick comments (3)
Makefile (1)
32-32: Don’t force CGO globally; make it overridable or scope per target.EXPORT_ALL_VARIABLES plus
CGO_ENABLED := 1forces CGO for all commands, which can break cross-compiles or environments lacking a C toolchain. Prefer a default that users can override, or set per build targets.Apply:
-CGO_ENABLED := 1 +CGO_ENABLED ?= 1Optionally scope CGO to build targets only:
build-local build-ci build-release: export CGO_ENABLED=1metal/cli/seo/client.go (2)
57-65: Nice: per-entity caching via sync.Once reduces repeated IOThe lazy, once-per-entity fetch is clean and thread-safe. Good alignment with generator usage.
Note: sync.Once also caches the first error permanently (no retries). If transient errors are possible, consider:
- a ResetCache() to clear c.data between runs/tests, or
- using singleflight.Group to dedupe concurrent calls but allow subsequent retries.
Example ResetCache:
// Add to Client func (c *Client) ResetCache() { c.data = ClientData{} }Use cautiously only when no concurrent reads are in-flight.
Also applies to: 68-75, 83-91
20-32: Avoid exposing cache internals; make ClientData unexportedClientData leaks internal caching details into the public API. Make it unexported (clientData) to preserve flexibility.
-type ClientData struct { +type clientData struct { // ... } type Client struct { WebsiteRoutes *router.WebsiteRoutes Fixture router.Fixture - data ClientData + data clientData }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Makefile(1 hunks)docker-compose.yml(2 hunks)metal/cli/seo/client.go(4 hunks)metal/cli/seo/generator.go(9 hunks)pkg/images/webp_encode_stub.go(0 hunks)
💤 Files with no reviewable changes (1)
- pkg/images/webp_encode_stub.go
🧰 Additional context used
🧬 Code graph analysis (2)
metal/cli/seo/generator.go (4)
pkg/cli/message.go (2)
Magentaln(33-35)Cyanln(49-51)metal/cli/seo/categories.go (1)
NewCategories(16-21)metal/cli/seo/client.go (1)
Client(14-18)metal/cli/seo/sections.go (1)
NewSections(14-16)
metal/cli/seo/client.go (8)
handler/payload/profile.go (1)
ProfileResponse(3-6)handler/payload/projects.go (1)
ProjectsResponse(3-6)handler/payload/recommendations.go (1)
RecommendationsResponse(3-6)metal/router/static.go (1)
StaticRouteResource(10-12)handler/profile.go (1)
MakeProfileHandler(16-20)metal/router/fixture.go (1)
Fixture(15-20)handler/projects.go (1)
MakeProjectsHandler(16-20)handler/recommendations.go (1)
MakeRecommendationsHandler(16-20)
🔇 Additional comments (1)
metal/cli/seo/generator.go (1)
96-111: Good refactor: step pipeline simplifies flow and error contextThe loop removes repetition and standardizes contextual error wrapping. Nice.
gocanto
deleted the
gus/fix-metal/cli/seo/generator.go-on-ubuntu-2025-10-13
branch
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68ecb31887c483339fd428f9661a82ca
Summary by CodeRabbit
New Features
Improvements
Chores