Repository navigation
Conversation
![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.
Reviewed commit: f13465be01
ℹ️ 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 "@codex address that feedback".
| ) | ||
|
|
||
| DuckDBStore(config.store.path).reset() | ||
| _store_call(lambda: DuckDBStore(config.store.path).reset()) |
There was a problem hiding this comment.
When agentprof store reset runs concurrently with another process holding the database, this wrapper cannot provide the promised lock error because DuckDBStore.reset() unlinks the database before attempting connect(). On POSIX, unlinking a locked file succeeds, so reset creates a new database while the other process continues using the now-unlinked old database, silently losing any subsequent work; on platforms that reject the unlink, the uncaught PermissionError can still produce a traceback. Make reset detect or coordinate the lock before removing either database file.
Useful? React with 👍 / 👎.
| except StoreConnectionError as exc: | ||
| _exit_store_locked(exc) |
There was a problem hiding this comment.
When another process locks the store, this handler is unreachable because DuckDBStore.migrations() opens the database directly with duckdb.connect(..., read_only=True) rather than DuckDBStore.connect(). The resulting duckdb.IOException falls into the generic workspace-error branch, so agentprof doctor does not emit the newly added store-locked message or retry guidance. Have migrations() use the translated connection path or translate its exception directly.
Useful? React with 👍 / 👎.
| return _truncate_svg_label(names[0], max_chars=26) | ||
| return _truncate_svg_label(f"{names[0]} +{count - 1} more", max_chars=26) |
There was a problem hiding this comment.
For agent names containing wide glyphs, such as a 26-character uppercase name, this limit still permits text wider than the roughly 166 pixels available inside the card. Because proportional fonts do not map characters to a fixed width, the change does not reliably prevent the clipping it is intended to fix. Use an SVG clipping/text-length mechanism or a conservative width-aware truncation instead of accepting every label up to 26 characters.
Useful? React with 👍 / 👎.
Summary
Why
The full feature/user-story QA pass found two user-facing defects: raw DuckDB lock tracebacks during concurrent CLI access, and clipped text in the report visual artifact. Both are logistical/UX issues that make otherwise-correct behavior feel broken.
Validation
uv run pytest->139 passedagentprof demo --dir outputs/.../demo-smokeagentprof store stats, confirmed exit code 2 with no tracebackNote: local QA tracker artifacts remain under
outputs/in the workspace and were intentionally left out of the source PR because they include transient demo databases and rendered previews.