Albums: bug fixes and improvements - #1453
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
💤 Files with no reviewable changes (5)
🚧 Files skipped from review as they are similar to previous changes (10)
WalkthroughAlbum covers now derive from the first linked image and remain hidden for locked albums. Cover mutation is removed across the stack. Album pages use local skeletons, non-blocking refresh, cache invalidation, and updated deletion behavior. Radix UI dependencies are updated. ChangesAlbum cover and loading behavior
Radix UI dependency updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AlbumPage
participant AlbumQuery
participant AlbumSkeleton
participant AlbumGrid
AlbumPage->>AlbumQuery: request albums
AlbumQuery-->>AlbumPage: report loading state
AlbumPage->>AlbumSkeleton: render placeholders
AlbumQuery-->>AlbumPage: return album data
AlbumPage->>AlbumGrid: render album cards
Possibly related issues
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
frontend/package.jsonParsing error: Missing semicolon. (2:8) 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
.github/workflows/lint.yml (1)
33-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe exclusion assertions can pass without any formatter installed. The hook rewrites files only when the matching formatter exists, and CI runs the hook test before frontend dependencies are installed. Both sites must change so the test proves exclusion rather than absence.
.github/workflows/lint.yml#L33-L35: move theAgent hook testsstep below theInstall frontend dependenciesstep sofrontend/node_modules/prettierexists when the test runs.scripts/agent-format-hook.test.mjs#L141-L153: add a negative control that formats a non-excluded temporary.jsonfile inside the repository, and skip the exclusion assertion when that control file is unchanged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/lint.yml around lines 33 - 35, The Agent hook tests step in .github/workflows/lint.yml#L33-L35 must move below Install frontend dependencies so the formatter is available. In scripts/agent-format-hook.test.mjs#L141-L153, add a negative-control temporary .json file inside the repository, format it, and skip the exclusion assertion when that control file remains unchanged; update both affected sites accordingly.scripts/agent-format-hook.mjs (1)
48-64: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider a few more Ruff invocation forms in the guard.
isRuffFormatCommandanchors on^ruff\s+format, so these forms pass through unblocked:
- a path-qualified binary:
backend/venv/bin/ruff format .or./.venv/bin/ruff formatnpx ruff formatandpipenv run ruff format- environment assignments after a runner:
uv run RUFF_CACHE_DIR=/tmp ruff format ., becauseENV_PREFIXis stripped only once before the peel loopThe last case is a one-line fix: move the
ENV_PREFIXstrip inside the peel loop and addnpx/pipenvtoRUNNER_PREFIX.♻️ Proposed widening of the guard
-const RUNNER_PREFIX = /^(?:uv|poetry|pipx|pdm|hatch|rye)\s+run\s+/; +const RUNNER_PREFIX = /^(?:uv|poetry|pipx|pdm|hatch|rye|pipenv)\s+run\s+|^npx\s+(?:-y\s+)?/; const PYTHON_M_PREFIX = /^python[0-9.]*\s+-m\s+/; const ENV_PREFIX = /^(?:\w+=\S*\s+)+/; +// Any leading path segments before the binary name. +const PATH_PREFIX = /^\S*[\\/]/; /** True when a single command segment invokes the Ruff formatter. */ function isRuffFormatCommand(segment) { - let s = segment.trim().replace(ENV_PREFIX, ''); + let s = segment.trim(); // Peel runner wrappers: `uv run ruff format`, `python -m ruff format`. for (;;) { - const next = s.replace(RUNNER_PREFIX, '').replace(PYTHON_M_PREFIX, ''); + const next = s + .replace(ENV_PREFIX, '') + .replace(RUNNER_PREFIX, '') + .replace(PYTHON_M_PREFIX, ''); if (next === s) break; s = next; } - return /^ruff\s+format\b/.test(s); + return /^ruff\s+format\b/.test(s.replace(PATH_PREFIX, '')); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/agent-format-hook.mjs` around lines 48 - 64, Update isRuffFormatCommand and its prefix handling to recognize path-qualified Ruff binaries, including relative and virtual-environment paths, and add npx and pipenv to RUNNER_PREFIX. Move ENV_PREFIX stripping into the runner-peeling loop so assignments after wrappers such as uv run are removed before checking the command, while preserving detection of Ruff’s format subcommand.scripts/agent-format-hook.test.mjs (1)
144-157: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe probe writes into a tracked source tree.
mkdtempSynccreates__hook_probe__-*insidefrontend/src/components/ui. Thefinallyblock removes it on a normal exit. If the process is interrupted, the directory stays behind and pollutesgit status. The exclusion assertion only needsisExcludedto see the path prefix, so a fixed non-existent path is enough for the subprocess exit-code check, and the byte-for-byte check can use a temp directory whose relative path still starts withfrontend/src/components/ui/. Alternatively, add__hook_probe__-*to.gitignore.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/agent-format-hook.test.mjs` around lines 144 - 157, Move the exclusion probe setup in the test block around runHook and isExcluded out of the tracked frontend source tree. Use a fixed or independently created temporary path that still has the frontend/src/components/ui/ prefix for exclusion detection, while preserving the unchanged-file assertion and cleanup behavior; alternatively, ignore the __hook_probe__-* pattern without changing the test’s assertions.agent-kit/skills/add-backend-endpoint/SKILL.md (1)
70-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRequire a malformed-row test for list endpoints.
The skill mandates row-by-row list construction at Line 58-59, but this checklist only requires success, empty, and one failure path. Add a test with one valid record and one malformed record.
backend/tests/test_videos.pyalready demonstrates this pattern.As per path instructions, test code must be automated, comprehensive, and cover critical functionality.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent-kit/skills/add-backend-endpoint/SKILL.md` around lines 70 - 74, Update the tests checklist for backend list endpoints to explicitly require a malformed-row test containing one valid record and one malformed record, while retaining the success, empty, and failure-path coverage. Reference the existing pattern in backend/tests/test_videos.py and ensure the test validates row-by-row handling.Source: Path instructions
agent-kit/templates/route.py.md (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the required return annotation.
get_all_<resource>is a backend route function without a return type. Add-> GetAll<Resource>Responseafter adapting the template placeholders.Proposed fix
-def get_all_<resource>(): +def get_all_<resource>() -> GetAll<Resource>Response:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent-kit/templates/route.py.md` at line 39, Add the required return annotation to the get_all_<resource> route template, adapting the resource placeholder so the function returns GetAll<Resource>Response while preserving the existing route signature and behavior.Source: Coding guidelines
docs/ai-contributing.md (1)
66-73: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin or validate third-party skill installations.
npx skills add shadcn/uidoes not record the CLI version or the installed skill revision. Different contributors can execute different third-party code and commit different files. Document pinned revisions or require review of generated files before committing them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ai-contributing.md` around lines 66 - 73, Update the third-party skill installation guidance around npx skills add shadcn/ui to require recording and pinning the CLI version and installed skill revision, or reviewing the generated files before committing them. Preserve the instruction to leave the skill in the installer-managed directory and document the adopted skill and rationale in third-party-skills.md.agent-kit/templates/slice.ts.md (1)
61-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the exported selectors explicitly.
The selectors define an exported TypeScript API boundary, but their return types are inferred. Declare
Item[]andItem | nullexplicitly.Proposed fix
+import type { Item } from '`@/types/Media`'; import { RootState } from '`@/app/store`'; -export const selectItems = (state: RootState) => state.feature.items; +export const selectItems = (state: RootState): Item[] => state.feature.items; -export const selectCurrentItem = (state: RootState) => +export const selectCurrentItem = (state: RootState): Item | null => state.feature.currentViewIndex >= 0 ? state.feature.items[state.feature.currentViewIndex] : null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent-kit/templates/slice.ts.md` around lines 61 - 66, Update the exported selectors selectItems and selectCurrentItem with explicit return types of Item[] and Item | null, respectively, while preserving their existing state access and conditional behavior.Source: Coding guidelines
backend/app/routes/albums.py (1)
36-60: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider batching the per-album cover lookup.
db_get_album_cover_pathis called once per unlocked album inside this loop. Each call opens a new SQLite connection. This adds a second per-album query on top of the existingdb_get_album_imagescall, so the number of DB round trips forGET /albums/scales linearly with the album count.A single query that groups
album_imagesbyalbum_idand selects the minimumrowidper group can retrieve all covers in one round trip, while the loop still builds the response row by row.♻️ Proposed batched cover lookup
+def db_get_all_album_cover_paths() -> dict[str, str]: + """Cover path per album, in one round trip.""" + conn = sqlite3.connect(DATABASE_PATH) + cursor = conn.cursor() + try: + cursor.execute( + """ + SELECT ai.album_id, i.path + FROM album_images ai + JOIN images i ON i.id = ai.image_id + WHERE ai.rowid = ( + SELECT MIN(rowid) FROM album_images WHERE album_id = ai.album_id + ) + """ + ) + return dict(cursor.fetchall()) + finally: + conn.close()def get_albums(): """Get all albums. Always returns both locked and unlocked albums.""" albums = db_get_all_albums() album_list = [] + covers = db_get_all_album_cover_paths() for album in albums: # Get image count for each album image_ids = db_get_album_images(album[0]) image_count = len(image_ids) is_locked = bool(album[3]) album_list.append( Album( album_id=album[0], album_name=album[1], description=album[2] or "", is_locked=is_locked, cover_image_path=( - None if is_locked else db_get_album_cover_path(album[0]) + None if is_locked else covers.get(album[0]) ), image_count=image_count, ) ) return GetAlbumsResponse(success=True, albums=album_list)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/routes/albums.py` around lines 36 - 60, Update get_albums to batch cover retrieval before the response-building loop, using one query that groups album_images by album_id and selects each group’s minimum rowid, then map those results by album ID. Replace per-album db_get_album_cover_path calls with lookups in the batched mapping while preserving None for locked albums.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent-kit/providers/codex.md`:
- Around line 23-25: Update the Codex hook limitation statement in the pre-PR
guidance to say that this repository does not configure Codex hooks, rather than
claiming Codex lacks hook support. Preserve the instruction to run the listed
checks manually, while noting that equivalent hook wiring can be added to
automate them.
In `@agent-kit/README.md`:
- Around line 69-77: Update the third-party skill workflow documentation around
“Adopting a third-party skill” to require a reviewed immutable revision, such as
a commit or tag, for both installation and updates. Instruct users to record the
selected revision alongside the adoption rationale in
references/third-party-skills.md, while preserving the installer-managed skill
locations and update guidance.
In `@agent-kit/references/ci-gates.md`:
- Line 14: Pin every external package used by documented npx commands, using
reviewed exact versions or repository-managed dependencies. Update
agent-kit/README.md lines 72 and 76, agent-kit/providers/codex.md lines 72-72,
agent-kit/references/third-party-skills.md lines 8-10 and 41-43,
docs/ai-contributing.md lines 67 and 71, agent-kit/references/ci-gates.md line
14, and agent-kit/skills/pre-pr-check/SKILL.md line 33; apply the pinning
consistently to markdownlint-cli2, skills update, and the candidate skills add
shadcn/ui commands.
In `@agent-kit/skills/add-backend-endpoint/SKILL.md`:
- Around line 81-85: Update the validation commands in the add-backend-endpoint
playbook to include both backend and sync-microservice PyInstaller build gates,
matching the commands defined in agent-kit/references/ci-gates.md; alternatively
delegate validation to the corrected pre-pr-check skill that already runs them.
In `@agent-kit/skills/pre-pr-check/SKILL.md`:
- Around line 10-12: Update the pre-PR gate instructions in SKILL.md to match
the CI gate set defined in ci-gates.md: always include the agent hook tests, add
both PyInstaller build checks, and remove cargo test. Revise the “same checks as
CI” wording if needed, and preserve CI’s required execution order while
reporting every check without stopping after failures.
In `@backend/AGENTS.md`:
- Around line 69-72: Make the area-specific check commands explicit about their
working directories: in backend/AGENTS.md lines 69-72, use the repository-root
config path or wrap the command with (cd backend && ...); in frontend/AGENTS.md
lines 82-86, wrap npm checks with (cd frontend && ...); and in
frontend/src-tauri/AGENTS.md lines 24-31, wrap Cargo checks with (cd
frontend/src-tauri && ...), preserving the repository-root AGENTS.md command
contract.
In `@scripts/agent-format-hook.test.mjs`:
- Line 5: Update the comment above the test hook to reference
.github/workflows/lint.yml and its Linting job instead of the stale
.github/workflows/pr-check-tests.yml reference.
---
Nitpick comments:
In @.github/workflows/lint.yml:
- Around line 33-35: The Agent hook tests step in
.github/workflows/lint.yml#L33-L35 must move below Install frontend dependencies
so the formatter is available. In scripts/agent-format-hook.test.mjs#L141-L153,
add a negative-control temporary .json file inside the repository, format it,
and skip the exclusion assertion when that control file remains unchanged;
update both affected sites accordingly.
In `@agent-kit/skills/add-backend-endpoint/SKILL.md`:
- Around line 70-74: Update the tests checklist for backend list endpoints to
explicitly require a malformed-row test containing one valid record and one
malformed record, while retaining the success, empty, and failure-path coverage.
Reference the existing pattern in backend/tests/test_videos.py and ensure the
test validates row-by-row handling.
In `@agent-kit/templates/route.py.md`:
- Line 39: Add the required return annotation to the get_all_<resource> route
template, adapting the resource placeholder so the function returns
GetAll<Resource>Response while preserving the existing route signature and
behavior.
In `@agent-kit/templates/slice.ts.md`:
- Around line 61-66: Update the exported selectors selectItems and
selectCurrentItem with explicit return types of Item[] and Item | null,
respectively, while preserving their existing state access and conditional
behavior.
In `@backend/app/routes/albums.py`:
- Around line 36-60: Update get_albums to batch cover retrieval before the
response-building loop, using one query that groups album_images by album_id and
selects each group’s minimum rowid, then map those results by album ID. Replace
per-album db_get_album_cover_path calls with lookups in the batched mapping
while preserving None for locked albums.
In `@docs/ai-contributing.md`:
- Around line 66-73: Update the third-party skill installation guidance around
npx skills add shadcn/ui to require recording and pinning the CLI version and
installed skill revision, or reviewing the generated files before committing
them. Preserve the instruction to leave the skill in the installer-managed
directory and document the adopted skill and rationale in third-party-skills.md.
In `@scripts/agent-format-hook.mjs`:
- Around line 48-64: Update isRuffFormatCommand and its prefix handling to
recognize path-qualified Ruff binaries, including relative and
virtual-environment paths, and add npx and pipenv to RUNNER_PREFIX. Move
ENV_PREFIX stripping into the runner-peeling loop so assignments after wrappers
such as uv run are removed before checking the command, while preserving
detection of Ruff’s format subcommand.
In `@scripts/agent-format-hook.test.mjs`:
- Around line 144-157: Move the exclusion probe setup in the test block around
runHook and isExcluded out of the tracked frontend source tree. Use a fixed or
independently created temporary path that still has the
frontend/src/components/ui/ prefix for exclusion detection, while preserving the
unchanged-file assertion and cleanup behavior; alternatively, ignore the
__hook_probe__-* pattern without changing the test’s assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9a9e02e1-406d-4e67-821e-aec738cd9b21
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (61)
.agents/skills/add-backend-endpoint/SKILL.md.agents/skills/add-frontend-feature/SKILL.md.agents/skills/onboard/SKILL.md.agents/skills/pre-pr-check/SKILL.md.claude/agents/pictopy-reviewer.md.claude/settings.json.claude/skills/add-backend-endpoint/SKILL.md.claude/skills/add-frontend-feature/SKILL.md.claude/skills/onboard/SKILL.md.claude/skills/pre-pr-check/SKILL.md.cursor/rules/pictopy.mdc.github/copilot-instructions.md.github/workflows/lint.yml.gitignoreAGENTS.mdCLAUDE.mdCONTRIBUTING.mdagent-kit/README.mdagent-kit/providers/claude-code.mdagent-kit/providers/codex.mdagent-kit/providers/copilot.mdagent-kit/providers/cursor.mdagent-kit/references/backend-endpoint-walkthrough.mdagent-kit/references/ci-gates.mdagent-kit/references/frontend-feature-walkthrough.mdagent-kit/references/third-party-skills.mdagent-kit/skills/add-backend-endpoint/SKILL.mdagent-kit/skills/add-frontend-feature/SKILL.mdagent-kit/skills/onboard/SKILL.mdagent-kit/skills/pre-pr-check/SKILL.mdagent-kit/templates/component.test.tsx.mdagent-kit/templates/database.py.mdagent-kit/templates/route.py.mdagent-kit/templates/slice.ts.mdbackend/AGENTS.mdbackend/CLAUDE.mdbackend/app/database/albums.pybackend/app/routes/albums.pybackend/app/schemas/album.pybackend/tests/test_albums.pybackend/tests/test_albums_db.pydocs/ai-contributing.mddocs/backend/backend_python/openapi.jsonfrontend/AGENTS.mdfrontend/CLAUDE.mdfrontend/package.jsonfrontend/src-tauri/AGENTS.mdfrontend/src-tauri/CLAUDE.mdfrontend/src/api/api-functions/albums.tsfrontend/src/api/apiEndpoints.tsfrontend/src/components/Albums/AlbumCard.tsxfrontend/src/pages/Album/Album.tsxfrontend/src/pages/Album/AlbumDetail.tsxfrontend/src/pages/__tests__/Album.test.tsxfrontend/src/pages/__tests__/AlbumDetail.test.tsxfrontend/src/types/Album.tsmkdocs.ymlscripts/agent-format-hook.mjsscripts/agent-format-hook.test.mjssync-microservice/AGENTS.mdsync-microservice/CLAUDE.md
💤 Files with no reviewable changes (5)
- backend/app/schemas/album.py
- docs/backend/backend_python/openapi.json
- frontend/src/api/apiEndpoints.ts
- frontend/src/api/api-functions/albums.ts
- frontend/src/types/Album.ts
| - Codex has no equivalent of Claude Code's hooks, so the automatic formatting does not | ||
| apply. Run the checks in `agent-kit/skills/pre-pr-check/SKILL.md` manually before | ||
| pushing — especially `pre-commit`, since nothing will have reformatted Python for you. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
OpenAI Codex hooks documentation codex hooks
💡 Result:
In the context of OpenAI Codex, "hooks" refer to an extensibility framework that allows users to inject custom scripts and logic into the agentic loop [1][2]. This system is distinct from general API webhooks [3]. Key features and documentation details for Codex hooks include: Functionality: Hooks enable customization of the agent's behavior at various points in its execution lifecycle, such as session starts, tool usage (pre/post), and permission requests [1][4]. Configuration: Codex discovers hooks through configuration files, specifically hooks.json or inline [hooks] tables within config.toml files [1][5]. Security and Trust: Because hooks allow for script execution, Codex implements a trust model. Non-managed command hooks require the user to review and explicitly trust the definition, which is then recorded against the hook's hash [1][2]. Users can manage these using the /hooks command in the Codex CLI [1]. Management: Enterprise administrators can enforce specific hook configurations using requirements.toml files, which can override or restrict local user, project, or session hook settings [1][6]. Common Hook Events: The framework includes various trigger points, including: - Session lifecycle: SessionStart, SessionEnd [1][2]. - Agent actions: PreToolUse, PostToolUse, SubagentStart, SubagentStop [1][4]. - User interaction: UserPromptSubmit [1][4]. - System tasks: PreCompact, PostCompact, PermissionRequest [1][4]. For further technical details, developers can refer to the official Codex developer documentation at https://developers.openai.com/codex/hooks [1]. Please note that these hooks are specific to the Codex agent environment and should not be confused with standard OpenAI API webhooks used for event notifications [3].
Citations:
- 1: https://developers.openai.com/codex/hooks
- 2: https://learn.chatgpt.com/docs/hooks
- 3: https://developers.openai.com/api/docs/guides/webhooks
- 4: https://github.com/openai/codex/blob/f1affbac/codex-rs/core/src/hook_runtime.rs
- 5: https://learn.chatgpt.com/docs/hooks.md
- 6: https://github.com/openai/codex/blob/53b50197/docs/config.md
🌐 Web query:
OpenAI Codex hooks generally available
💡 Result:
As of August 3, 2026, OpenAI Codex remains a generally available service and is actively supported [1][2]. It is accessible through several platforms, including the ChatGPT desktop application, the web interface, the Codex CLI, and various IDE extensions [1][3]. OpenAI has explicitly confirmed that its standalone Codex application is "here to stay," despite the integration of its capabilities into broader platforms like ChatGPT Work [2]. While the service remains available, OpenAI has recently completed a series of model deprecations [4][5]. On July 23, 2026, OpenAI permanently shut down several legacy Codex-lineage models within the API [4][6][5]. Developers and users who were relying on these older model versions were required to migrate to newer, supported alternatives, such as the GPT-5.6 series, to maintain functionality [7][4][5]. Users can continue to use Codex by signing in with their ChatGPT account (e.g., Plus, Pro, Business, or Enterprise plans) or by utilizing an API key with the updated, supported model versions [3][8].
Citations:
- 1: https://developers.openai.com/codex
- 2: https://www.theverge.com/ai-artificial-intelligence/964293/openais-standalone-codex-app-is-here-to-stay
- 3: https://github.com/openai/codex
- 4: https://developers.openai.com/api/docs/deprecations
- 5: https://codex.danielvaughan.com/2026/06/02/codex-model-sunset-june-july-2026-deprecation-timeline-migration-paths-config-recipes/
- 6: https://community.openai.com/t/deprecation-notice-upcoming-model-shutdowns-in-2026/1379553/2
- 7: https://chatgpt.com/codex/pricing/
- 8: Codex model deprecations openai/codex#17038
🌐 Web query:
site:developers.openai.com/codex/hooks hooks.json config.toml Codex hooks documentation
💡 Result:
In the Codex ecosystem, hooks allow for the execution of custom logic (such as commands) in response to specific events, such as before or after tool use. Codex discovers these hooks by looking for configuration files located next to active configuration layers [1]. Configuration Methods Codex supports defining hooks using either a standalone hooks.json file or by including an inline [hooks] table directly within a config.toml file [1]. Common file locations include: - ~/.codex/hooks.json or ~/.codex/config.toml (User-level) - /.codex/hooks.json or /.codex/config.toml (Project-level) If a single configuration layer contains both a hooks.json file and inline hooks in config.toml, Codex will merge them and issue a warning at startup; it is recommended to use only one representation per layer [1]. Configuration and Precedence - Feature Toggle: Hooks are enabled by default. To disable them, you can set [features].hooks = false in your config.toml file [1]. - Precedence: Codex processes multiple hook locations. If multiple hooks are found, they are all executed. - Enforcement: Administrators can enforce hook usage via requirements.toml by pinning [features].hooks = true [1]. Example Syntax The following shows an example of a PreToolUse hook defined in TOML: [[hooks.PreToolUse]] matcher = "^Bash$" [[hooks.PreToolUse.hooks]] type = "command" command = '/usr/bin/python3 "$(git rev-parse --show-toplevel)/.codex/hooks/pre_tool_use_policy.py"' timeout = 30 statusMessage = "Checking Bash command" For JSON, the structure mirrors this hierarchy, using "hooks" as the key within a hooks.json file [1].
Citations:
Reword the Codex hook limitation as a repository configuration gap.
Codex supports hooks via hooks.json and inline [hooks] tables in config.toml, so this line should not say Codex has no hook equivalent. Say this repository does not configure Codex hooks, and users must run the listed pre-PR checks manually unless they add equivalent hook wiring.
Proposed documentation change
-Codex has no equivalent of Claude Code's hooks, so the automatic formatting does not
-apply. Run the checks in `agent-kit/skills/pre-pr-check/SKILL.md` manually before
-pushing — especially `pre-commit`, since nothing will have reformatted Python for you.
+This repository does not configure Codex hooks, so automatic formatting does not apply
+here. Run the checks in `agent-kit/skills/pre-pr-check/SKILL.md` manually unless
+equivalent Codex hooks are configured.[low_effort.and_high_reward]
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Codex has no equivalent of Claude Code's hooks, so the automatic formatting does not | |
| apply. Run the checks in `agent-kit/skills/pre-pr-check/SKILL.md` manually before | |
| pushing — especially `pre-commit`, since nothing will have reformatted Python for you. | |
| This repository does not configure Codex hooks, so automatic formatting does not apply | |
| here. Run the checks in `agent-kit/skills/pre-pr-check/SKILL.md` manually unless | |
| equivalent Codex hooks are configured. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent-kit/providers/codex.md` around lines 23 - 25, Update the Codex hook
limitation statement in the pre-PR guidance to say that this repository does not
configure Codex hooks, rather than claiming Codex lacks hook support. Preserve
the instruction to run the listed checks manually, while noting that equivalent
hook wiring can be added to automate them.
| ## Adopting a third-party skill | ||
|
|
||
| ```bash | ||
| npx skills add <owner>/<repo> | ||
| ``` | ||
|
|
||
| This installs into `.claude/skills/` or `.agents/skills/` and stays updatable with | ||
| `npx skills update`. Leave it where the installer put it — moving it here breaks updates — | ||
| and record what you adopted and why in `references/third-party-skills.md`. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin third-party skill revisions before installation or update.
The documented npx skills add <owner>/<repo> and npx skills update workflow does not require a reviewed immutable revision. A changed external skill can alter agent instructions and tool requests after repository review. Require a reviewed commit, tag, or equivalent immutable reference. Record that revision in agent-kit/references/third-party-skills.md.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent-kit/README.md` around lines 69 - 77, Update the third-party skill
workflow documentation around “Adopting a third-party skill” to require a
reviewed immutable revision, such as a commit or tag, for both installation and
updates. Instruct users to record the selected revision alongside the adoption
rationale in references/third-party-skills.md, while preserving the
installer-managed skill locations and update guidance.
|
|
||
| | Check | Local command | | ||
| | --- | --- | | ||
| | Markdown | `npx markdownlint-cli2 --config .github/.markdownlint-cli2.jsonc` | |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
if rg -n --glob '*.md' --glob '*.yml' --glob '*.yaml' \
'npx[[:space:]]+(markdownlint-cli2|skills)([[:space:]]|$)' .
then
echo "Unpinned npx command found." >&2
exit 1
fiRepository: AOSSIE-Org/PictoPy
Length of output: 1172
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Relevant npx occurrences:\n'
rg -n --glob '*.md' --glob '*.yml' --glob '*.yaml' \
'npx[[:space:]]+([^[:space:]]+)([[:space:]]|$)' .
printf '\nRelevant files:\n'
for f in \
agent-kit/README.md \
agent-kit/providers/codex.md \
agent-kit/references/ci-gates.md \
agent-kit/references/third-party-skills.md \
agent-kit/skills/pre-pr-check/SKILL.md \
docs/ai-contributing.md
do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,140p' "$f"
else
echo "MISSING: $f"
fi
done
printf '\nDependency/config markers:\n'
rg -n --hidden --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' \
'markdownlint-cli2|skills|npx|package-manager|dependencies|devDependencies|toolchain|pyinstaller' \
.github .vercel .github || trueRepository: AOSSIE-Org/PictoPy
Length of output: 23396
Pin every external package invoked through npx.
These instructions execute unpinned remote packages; a new publication may change install behavior or execute arbitrary code. Use reviewed exact versions or repository-managed dependencies for every documented npx command.
Remaining unpinned occurrences:
agent-kit/README.md:72and same command acrossagent-kit/providers/codex.md,agent-kit/references/third-party-skills.mdagent-kit/README.md:76,agent-kit/references/third-party-skills.md:14,docs/ai-contributing.md:71fornpx skills updateagent-kit/references/ci-gates.md:14andagent-kit/skills/pre-pr-check/SKILL.md:33fornpx markdownlint-cli2- Candidate
npx skills add shadcn/uiinagent-kit/references/third-party-skills.md:41anddocs/ai-contributing.md:67
🧰 Tools
🪛 LanguageTool
[uncategorized] ~14-~14: The official name of this software platform is spelled with a capital “H”.
Context: ...al command | | --- | --- | | Markdown | npx markdownlint-cli2 --config .github/.markdownlint-cli2.jsonc | | Frontend ...
(GITHUB)
📍 Affects 3 files
agent-kit/references/ci-gates.md#L14-L14(this comment)agent-kit/references/third-party-skills.md#L8-L10agent-kit/references/third-party-skills.md#L41-L43agent-kit/skills/pre-pr-check/SKILL.md#L33-L33
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent-kit/references/ci-gates.md` at line 14, Pin every external package used
by documented npx commands, using reviewed exact versions or repository-managed
dependencies. Update agent-kit/README.md lines 72 and 76,
agent-kit/providers/codex.md lines 72-72,
agent-kit/references/third-party-skills.md lines 8-10 and 41-43,
docs/ai-contributing.md lines 67 and 71, agent-kit/references/ci-gates.md line
14, and agent-kit/skills/pre-pr-check/SKILL.md line 33; apply the pinning
consistently to markdownlint-cli2, skills update, and the candidate skills add
shadcn/ui commands.
Source: Linters/SAST tools
| // Regression tests for the agent format hook. | ||
| // | ||
| // Run: node scripts/agent-format-hook.test.mjs | ||
| // Also runs in CI, in the Linting job of .github/workflows/pr-check-tests.yml. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the stale workflow reference.
The CI step was added to .github/workflows/lint.yml, in the Linting job. This comment names .github/workflows/pr-check-tests.yml.
📝 Proposed fix
-// Also runs in CI, in the Linting job of .github/workflows/pr-check-tests.yml.
+// Also runs in CI, in the Linting job of .github/workflows/lint.yml.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Also runs in CI, in the Linting job of .github/workflows/pr-check-tests.yml. | |
| // Also runs in CI, in the Linting job of .github/workflows/lint.yml. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/agent-format-hook.test.mjs` at line 5, Update the comment above the
test hook to reference .github/workflows/lint.yml and its Linting job instead of
the stale .github/workflows/pr-check-tests.yml reference.
Deleting an album goes dropdown menu -> confirm dialog, so a Radix menu and a Radix dialog are mounted at the same moment. Radix keeps its focus trap in a module-level stack and pauses the previously active scope, but react-dialog had drifted ahead of every other Radix package and pinned its own private copies of the shared internals. Two copies meant two stacks, so neither scope paused the other and they yanked focus back and forth until the call stack blew, pegging the main thread until reload. Aligning the Radix packages to one generation collapses all 16 duplicated internals to a single copy each, including react-dismissable-layer, which owns the body pointer-events state and could strand the app the same way.
Deleting removed the album from Redux before the request was sent and never put it back, so a failed delete made the album vanish from the UI while it still existed on disk — it only reappeared on a manual refresh. The confirmation dialog also closed only on success, leaving it stacked under the error dialog when the delete failed. Drop the optimistic write and let autoInvalidateTags refetch the list, matching how folder deletion already works, and close the confirmation when the user confirms rather than when the request resolves.
The album list blocked the whole window behind the global loader while it fetched, and the grid ran two columns wider than the memories grid at every breakpoint, so the two pages never lined up. Swap the loader dispatch for skeleton tiles and adopt the memories grid. Rendering skeletons first also stops "No Albums Created Yet" flashing before the first response lands, which the loader overlay used to hide.
Refresh greyed out the whole window behind the global loader for what is a background refetch. Report the work on the button itself, the way the memories Regenerate button does, and disable it while a fetch is in flight so the list stays readable and a second refetch cannot be queued. The global loader now has no caller left on this page.
The detail page dispatched showLoader while its images loaded and only hid it again on the success branch. Leaving the page mid-request, or an all-images fetch that never resolved, left the overlay up with nothing on the page to clear it — the same stuck-overlay freeze, reached a different way. It also blocked the window for what the list page now renders as skeletons. Drop the loader for skeleton tiles matching ImageCard, and gate the empty state behind them so it cannot claim the album is empty mid-load. Removing images and setting a cover both change what the list page shows, so invalidate ['albums'] rather than leaving it to a remount refetch.
The albums table has no created_at or updated_at column and the Album response model carries no timestamp fields, so both pages fell through to new Date().toISOString() on every fetch. Album.created_at was required by the type but only ever held the moment it was last read, which would have quietly produced nonsense the first time anything sorted by it. Nothing read either field, so drop them from the type and both mappers.
Choosing a cover was a right-click menu on every photo in an album, which
is a lot of ceremony for something that can just pick itself. The cover is
now the album's first image, resolved in one join at read time, so a new
album has artwork the moment it has a photo instead of only after someone
goes looking for the menu.
Drops the whole manual path: the PUT /albums/{id}/cover route, its schema,
the db writer, the API wrapper, and the per-image menu whose only item it
was. cover_image_path stays on the response, derived rather than stored.
A locked album listed its cover like any other, so the grid showed a photo from inside an album whose whole point is that you need a password to see inside it. The lock badge sat on top of the very content it was meant to be gating. Both reads now send no cover path for a locked album, and skip the lookup rather than fetching a path they will not return. The card already falls back to the placeholder when the path is absent.
Fit one more column from the sm breakpoint up and tighten the caption under the cover, so more of the library is visible at a glance without the tiles feeling cramped. The skeleton follows the same measurements to keep the grid from resizing as it loads.
6b45479 to
382978d
Compare
1 participant
Fixes most of #1452.
What changed:
installed at once and kept taking focus from each other.
longer gets stuck behind the error popup.
Refresh reports itself on the button instead of blocking the page.
and its endpoint are gone.
Still open on #1452: opening one album still downloads the whole library, removing photos
has no confirmation, and locked albums still show their photo count.
Note: this branch sits on top of #1408, so the diff includes those commits until that one
merges.
Summary by CodeRabbit
New Features
Removed
Bug Fixes
Tests