feat: Map plugin_data by persona_name - #12481
Conversation
Resolves the TODO in `backend/scripts/web.py` to map all plugin_data by persona_name so that it can be locally mapped via json. Adds a new function `get_plugin_data_by_persona_name()` that iterates over all items in the `plugins_data` collection, mapping them using the `name` field as the key, and outputting to `plugin_data_by_persona_name.json`. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 5c9631498b
ℹ️ 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".
|
|
||
| def get_plugin_data_by_persona_name() -> None: | ||
| plugin_data_by_name: Dict[str, Dict[str, Any]] = {} | ||
| plugins_ref = db.collection("plugins_data").stream() |
There was a problem hiding this comment.
This new read uses the legacy global db proxy, so the helper bypasses the repository's call-time client seam and cannot accept an explicitly selected or injected Firestore client. Import get_firestore_client() and resolve it inside this function instead.
AGENTS.md reference: backend/AGENTS.md:L169-L171
Useful? React with 👍 / 👎.
| data: Dict[str, Any] = cast(Dict[str, Any], raw) if isinstance(raw, dict) else {} | ||
| name = data.get("name") | ||
| if name: | ||
| plugin_data_by_name[name] = data |
There was a problem hiding this comment.
When two persona documents have the same display name, this assignment silently replaces the first document with the later one, so the generated file does not contain all plugin data. This is a supported case because persona creation uniquifies username but accepts duplicate name values; group documents under each name or use a unique identifier rather than storing a single value.
Useful? React with 👍 / 👎.
| with open("plugin_data_by_persona_name.json", "w") as f: | ||
| json.dump(plugin_data_by_name, f, default=str) |
There was a problem hiding this comment.
When this script is run from either the repository root or backend/, the output path is not covered by .gitignore, and the complete plugin documents can include owner email addresses, persona/chat/memory prompts, Twitter data, and integration configuration. This leaves a sensitive untracked artifact that a routine broad git add can commit; write it beneath an ignored data directory or add a precise ignore entry for this generated file.
Useful? React with 👍 / 👎.
| import json | ||
|
|
||
|
|
||
| def get_plugin_data_by_persona_name() -> None: |
There was a problem hiding this comment.
This commit adds a new Firestore-to-JSON feature without adding any test for its core mapping behavior or its main error cases, such as absent or duplicate names. Add a hermetic test using an injected fake client and a temporary output directory so regressions in the generated mapping are caught by the backend suite.
AGENTS.md reference: AGENTS.md:L28-L33
Useful? React with 👍 / 👎.
Git-on-my-level
left a comment
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for picking up this TODO — the new get_plugin_data_by_persona_name() in backend/scripts/web.py mirrors the existing get_user_messages_with_bot_name() style nicely (same defensive cast(...) if isinstance(raw, dict) handling, same json.dump(..., default=str)), and the needed typing imports were already in place. Requesting changes for three things that affect what this export actually produces before it gets run against real data:
-
Duplicate
namesilently drops documents.plugin_data_by_name[name] = datais last-write-wins, and nothing in the schema makesnameunique —usernameis the uniquified persona handle (seeget_persona_by_username_dbinbackend/database/apps.py, which filters onusername+capabilities array_contains 'persona'), whilenameis a free-form display name. The TODO asks to map all plugin_data, so an overwrite here quietly skews any analysis built on this file. Please preserve all docs for a shared name (e.g. key byusername, or keep a list per name / disambiguate with the doc id). -
Scope vs. function name.
plugins_datais the apps collection (apps_collection = 'plugins_data'inbackend/database/apps.py), so this streams every app, not just personas, and keys them by display name. If the goal is persona analysis, acapabilities array_contains 'persona'filter would match the..._by_persona_namename; otherwise consider renaming to reflect that it maps all plugin docs by display name. -
Output path sits in tracked territory.
plugin_data_by_persona_name.jsonis written to the CWD and isn't covered by.gitignore— the ignored script outputs live underbackend/scripts/rag/*.json,/backend/scripts/data/, andbackend/scripts/stt/diarization.json. These documents can carry owner emails, persona/chat prompts, and integration config, so an accidentalgit add -Aafter a run would commit user data./backend/scripts/data/already exists and is ignored — writing there (or gitignoring the filename) fixes it. The pre-existinguser_messages_with_bot_name.jsonwrite has the same exposure and could get the same one-line fix while you're in here.
Minor, non-blocking: newer scripts (e.g. backend/scripts/chat_agent_cost_report.py, backend/scripts/backfill_mcp_key_full_access.py) resolve Firestore via get_firestore_client() at call time rather than the legacy global db; fine to defer since this file already uses db throughout.
No test demanded for this to land — it's a manual analytics script — but a tiny hermetic test for the duplicate-name handling would be welcome if you want one. Once the collision handling and output location are sorted this should be quick to re-review.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
username is the uniquified handle so duplicate display names no longer drop documents. JSON output goes to the gitignored scripts/data dir.
|
Owner approval acknowledged — leaving this as a comment rather than another change request, since the remaining items are non-blocking for a manual analytics script. New information from comparing the two PRs implementing this TODO: this one is the functionally correct one. Two non-blocking caveats from the earlier review worth keeping in mind when the script is actually run:
Either way, a maintainer call on which duplicate lands is the only thing left before merge. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
CR follow-up: key persona |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_8a9b6015-0be8-459f-b886-09657ab4905e) |
Resolved at 1d61495: export now keys on the uniquified username via setdefault (duplicate display names no longer drop docs), filters to persona-capability docs, and writes both JSON outputs under the gitignored backend/scripts/data/ directory.
|
Thanks for the follow-up — all three items from the earlier change request are resolved at 1d61495, verified against
The earlier change request is dismissed as resolved. The one call still open before merge is the routine maintainer choice between this PR and sibling #12489 (same TODO, same hunk) — this one remains the functionally correct implementation. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
Git-on-my-level
added
the
positive-signal
3 participants
Implemented the logic to map plugin_data by persona_name into a local json file, as requested by the TODO item in
backend/scripts/web.py.Failure-Class: none
PR created automatically by Jules for task 417711359684236132 started by @undivisible
Note
Low Risk
Offline analytics script changes only; no production API or auth paths, with exports confined to a gitignored data folder.
Overview
Adds
get_plugin_data_by_persona_name(), which streams Firestoreplugins_data, keeps docs with apersonacapability and ausername, and writes a username-keyed map tobackend/scripts/data/plugin_data_by_persona_name.json.get_user_messages_with_bot_name()now writes its export to the samebackend/scripts/data/directory instead of the script working directory. The__main__block runs the new export after the messages dump, replacing the prior TODO.Reviewed by Cursor Bugbot for commit 1d61495. Configure here.