Add Python SDK#2705
Open
nezaj wants to merge 26 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a complete Python Admin SDK (async + sync), a TS genpy CLI to emit Python types, GitHub CI and PyPI publish workflows, Python scaffolding/examples, a sandbox tester, developer tooling (unasync, Makefile), docs, and extensive tests. ChangesPython Admin SDK end-to-end delivery
Estimated code review effort Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
|
View Vercel preview at instant-www-js-python-sdk-v1-jsv.vercel.app. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
.github/workflows/python-publish.yml (1)
10-31: ⚡ Quick winAdd explicit minimal permissions to the build job.
The build job uses default permissions, which are broader than necessary. Explicitly declaring minimal permissions improves security posture by limiting what a compromised step could access.
🛡️ Suggested fix
jobs: build: name: Build wheel + sdist runs-on: ubuntu-latest timeout-minutes: 10 + permissions: + contents: read steps: - name: Check out code uses: actions/checkout@v4🤖 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/python-publish.yml around lines 10 - 31, The build job currently relies on default repo-wide permissions; add an explicit minimal permissions block under the build job (the "build" job) to restrict access — e.g., set permissions: contents: read — so the workflow only has read access to repository contents while running the Build step (working-directory: client/packages/python run: uv build) and the Upload distributions step (uses: actions/upload-artifact@v4). Ensure no extra permissions (like write) are granted unless required.client/packages/python/src/instantdb/_sync/webhooks/receiver.py (1)
113-126: ⚡ Quick winDocument handler signature requirements for sync vs async flavors.
The docstring at lines 114-115 states "Handlers under
AsyncInstantmust beasync def; underInstantthey're plaindef. Mixing flavors is unsupported." However, there is no runtime enforcement. If a user accidentally passes anasync defhandler to the syncWebhooks.process_payload, line 126'shandler(arg)call will silently create an unawaited coroutine rather than raising a clear error.Consider adding a runtime check or expanding the docstring with a concrete example to reduce the risk of this mistake.
Optional: Add runtime check
+import inspect + def process_payload( self, handlers: dict[str, Any], payload: dict[str, Any], ) -> None: """Dispatch each record in the payload to its matching handler. ... """ records_map = self._schema.get("records", {}) if self._schema else {} for record in payload.get("data") or []: namespace = record.get("namespace") action = record.get("action") handler = _resolve_handler(handlers, namespace, action) if handler is None: continue + if inspect.iscoroutinefunction(handler): + raise InstantError( + "Sync Webhooks.process_payload received an async handler. " + "Use AsyncWebhooks for async def handlers." + ) record_model = records_map.get((namespace, action)) arg = record_model.model_validate(record) if record_model else record handler(arg)🤖 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 `@client/packages/python/src/instantdb/_sync/webhooks/receiver.py` around lines 113 - 126, The handler invocation can silently create unawaited coroutines when a user passes an async function to the sync webhook processor; update the sync code path in Webhooks.process_payload to detect wrong-flavored handlers by checking if the resolved handler is a coroutine function (e.g., using asyncio.iscoroutinefunction or inspect.iscoroutinefunction) right before the call to handler(arg) and raise a clear TypeError stating that AsyncInstant handlers (async def) are not allowed in Instant (sync) processing and must be converted to plain def; keep the check adjacent to where _resolve_handler(...) returns handler so the error surfaces immediately and references AsyncInstant vs Instant in the message.
🤖 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 `@client/packages/cli/src/commands/genpy.ts`:
- Around line 186-189: buildEntityModelsPy and buildTxStubPyi currently emit raw
schema keys (f.name, attrName, link.label) as Python identifiers which can
produce invalid .py/.pyi; create a helper makeSafePyIdent(key) to produce a
valid Python identifier (e.g., replace invalid chars, prefix reserved keywords
with underscore) and use that safe name when emitting class attributes in
buildEntityModelsPy, then preserve the original key by emitting a class-level
mapping like __field_aliases__ = {"safe_name": "raw-key"} so runtimes/tools can
map back; for buildTxStubPyi, switch to the functional TypedDict form when any
key is not a valid identifier (TypedDict("Name", {"raw-key": Type, ...}))
instead of class-syntax fields, falling back to class syntax only when all keys
are valid identifiers; update usages of f.name/attrName/link.label to call
makeSafePyIdent and adjust emitted declarations accordingly.
- Around line 439-446: Sanitize entity names before emitting attributes in
_TxBuilder: when iterating identifierEntities in genpy.ts, transform each
entName into a valid Python identifier (e.g., replace non-alphanumeric chars
with underscores, collapse consecutive underscores, and if it starts with a
digit prefix with an underscore) and use that sanitized identifier as the
attribute key while keeping className(entName) for the type reference; also
ensure sanitized names are unique (append suffix if collision) so generated
lines like ` <sanitized>: _NamespaceBuilder[_${className(entName)}Chunk]`
always produce valid Python syntax.
In `@client/packages/python/pyproject.toml`:
- Around line 38-52: The dynamic Hatch version path ([tool.hatch.version].path)
points to ../version/src/version.ts which is outside the sdist discovery scope,
so sdist-built packages cannot find version.ts; fix by either moving version.ts
into the Python package tree (e.g., under src/instantdb) and updating
[tool.hatch.version].path to the new relative path, or add a
tool.hatch.build.targets.sdist.force-include entry that explicitly includes
../version/src/version.ts (and mirror that change for the wheel config if
necessary) so both wheel and sdist can resolve the version file at build time.
In `@client/packages/python/README.md`:
- Around line 16-22: The header block opens with <p align="center"> but ends
with a stray opening <p> instead of a closing </p>, leaving invalid HTML;
replace the final "<p>" with the correct closing tag "</p>" so the block
properly closes (look for the <p align="center"> line and the trailing lone <p>
in the README.md header).
In `@client/packages/python/scripts/run_unasync.py`:
- Around line 56-59: The REMOVE_BLOCK regex requires a trailing newline after
the "# UNASYNC_REMOVE_END" marker so blocks at EOF without a final newline are
not matched; update REMOVE_BLOCK to allow either a newline OR end-of-file after
the end marker (i.e., make the trailing newline optional and accept EOF) and
apply the same change to the other removal regex later in the file that handles
UNASYNC blocks so marker blocks at EOF are stripped correctly.
In `@client/packages/python/src/instantdb/_async/storage.py`:
- Line 27: The async method AsyncStorage.upload_file calls the synchronous
helper _read_bytes which uses Path.read_bytes() and file.read(), blocking the
event loop; change the implementation so file reading is non-blocking — either
make _read_bytes an async function (rename to _read_bytes_async) that uses an
async file library like aiofiles for Path and file-like objects, or keep a sync
helper but call it via loop.run_in_executor from upload_file to offload I/O to a
thread; ensure both Path and file-like input are supported and update
AsyncStorage.upload_file to await the new async helper or await the executor
future accordingly and adjust any callers.
In `@client/packages/python/src/instantdb/_async/streams/writer.py`:
- Around line 106-112: The assert in async def write(self, chunk: str) should be
replaced with explicit validation: check if self._connection is None or
self._stream_id_future is None and raise an InstantError with a clear message
(e.g., "Stream not opened; use async context manager or call open() before
write()") instead of letting an AssertionError propagate; update the write
method to perform these checks before awaiting self._stream_id_future to provide
a deterministic, descriptive error when write() is called too early.
In `@client/packages/python/src/instantdb/_sync/webhooks/manager.py`:
- Around line 109-115: The disable method currently uses a truthiness check on
reason which drops empty strings; update the construction of body in disable
(method disable in manager.py) to check explicitly for None (e.g., use "if
reason is not None") so that reason="" is preserved and sent to the API when
provided; ensure the rest of the method (_http.post call and return via
_webhook_info) remains unchanged.
In `@client/packages/python/src/instantdb/_webhooks_crypto.py`:
- Around line 90-94: The freshness check currently only rejects old signatures;
compute the age as now - sig.t (using now = int(received_at if received_at is
not None else time.time())) and reject when abs(age) > max_age_seconds so
very-future timestamps also fail; update the InstantError to indicate whether
the signature is too old or too far in the future (include age and tolerance)
and apply this logic in the same function/context where sig.t, now,
max_age_seconds and InstantError are used in _webhooks_crypto.py.
In `@client/sandbox/admin-sdk-python/src/main_sync.py`:
- Line 168: The default sandbox API URI uses the wrong port (9888); update the
api_uri default value in main_sync.py (variable api_uri) to use
"http://localhost:8888" and search for the other occurrence mentioned (the
similar default at the other location around the second occurrence) and change
that default as well so both places match the documented/local sandbox port
8888.
---
Nitpick comments:
In @.github/workflows/python-publish.yml:
- Around line 10-31: The build job currently relies on default repo-wide
permissions; add an explicit minimal permissions block under the build job (the
"build" job) to restrict access — e.g., set permissions: contents: read — so the
workflow only has read access to repository contents while running the Build
step (working-directory: client/packages/python run: uv build) and the Upload
distributions step (uses: actions/upload-artifact@v4). Ensure no extra
permissions (like write) are granted unless required.
In `@client/packages/python/src/instantdb/_sync/webhooks/receiver.py`:
- Around line 113-126: The handler invocation can silently create unawaited
coroutines when a user passes an async function to the sync webhook processor;
update the sync code path in Webhooks.process_payload to detect wrong-flavored
handlers by checking if the resolved handler is a coroutine function (e.g.,
using asyncio.iscoroutinefunction or inspect.iscoroutinefunction) right before
the call to handler(arg) and raise a clear TypeError stating that AsyncInstant
handlers (async def) are not allowed in Instant (sync) processing and must be
converted to plain def; keep the check adjacent to where _resolve_handler(...)
returns handler so the error surfaces immediately and references AsyncInstant vs
Instant in the message.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9341d838-b8b4-4063-b170-fd34d253d229
⛔ Files ignored due to path filters (1)
client/packages/cli/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (73)
.github/workflows/python-publish.yml.github/workflows/python.ymlclient/packages/cli/src/commands/genpy.tsclient/packages/cli/src/index.tsclient/packages/create-instant-app/__tests__/e2e/create-instant-app.e2e.test.tsclient/packages/create-instant-app/scripts/copyExamples.tsclient/packages/create-instant-app/src/cli.tsclient/packages/create-instant-app/src/env.tsclient/packages/create-instant-app/src/index.tsclient/packages/python/.gitignoreclient/packages/python/Makefileclient/packages/python/README.mdclient/packages/python/pyproject.tomlclient/packages/python/scripts/run_unasync.pyclient/packages/python/src/instantdb/__init__.pyclient/packages/python/src/instantdb/_async/__init__.pyclient/packages/python/src/instantdb/_async/auth.pyclient/packages/python/src/instantdb/_async/client.pyclient/packages/python/src/instantdb/_async/http.pyclient/packages/python/src/instantdb/_async/rooms.pyclient/packages/python/src/instantdb/_async/storage.pyclient/packages/python/src/instantdb/_async/streams/__init__.pyclient/packages/python/src/instantdb/_async/streams/_connection.pyclient/packages/python/src/instantdb/_async/streams/reader.pyclient/packages/python/src/instantdb/_async/streams/writer.pyclient/packages/python/src/instantdb/_async/subscribe.pyclient/packages/python/src/instantdb/_async/webhooks/__init__.pyclient/packages/python/src/instantdb/_async/webhooks/manager.pyclient/packages/python/src/instantdb/_async/webhooks/receiver.pyclient/packages/python/src/instantdb/_errors.pyclient/packages/python/src/instantdb/_http_errors.pyclient/packages/python/src/instantdb/_sync/__init__.pyclient/packages/python/src/instantdb/_sync/auth.pyclient/packages/python/src/instantdb/_sync/client.pyclient/packages/python/src/instantdb/_sync/http.pyclient/packages/python/src/instantdb/_sync/rooms.pyclient/packages/python/src/instantdb/_sync/storage.pyclient/packages/python/src/instantdb/_sync/webhooks/__init__.pyclient/packages/python/src/instantdb/_sync/webhooks/manager.pyclient/packages/python/src/instantdb/_sync/webhooks/receiver.pyclient/packages/python/src/instantdb/_transact.pyclient/packages/python/src/instantdb/_version.pyclient/packages/python/src/instantdb/_webhooks_crypto.pyclient/packages/python/src/instantdb/py.typedclient/packages/python/tests/__init__.pyclient/packages/python/tests/conftest.pyclient/packages/python/tests/test_client.pyclient/packages/python/tests/test_storage.pyclient/packages/python/tests/test_streams_state.pyclient/packages/python/tests/test_subscription_state.pyclient/packages/python/tests/test_sync_client.pyclient/packages/python/tests/test_transact.pyclient/packages/python/tests/test_validation.pyclient/packages/python/tests/test_webhooks_crypto.pyclient/packages/python/tests/test_webhooks_dispatch.pyclient/sandbox/admin-sdk-python/.env.exampleclient/sandbox/admin-sdk-python/.gitignoreclient/sandbox/admin-sdk-python/README.mdclient/sandbox/admin-sdk-python/pyproject.tomlclient/sandbox/admin-sdk-python/src/__init__.pyclient/sandbox/admin-sdk-python/src/main.pyclient/sandbox/admin-sdk-python/src/main_sync.pyclient/www/app/docs/backend/page.mdclient/www/app/docs/start-python/page.mdclient/www/data/docsNavigation.jsexamples/python-script/.env.exampleexamples/python-script/.gitignoreexamples/python-script/.python-versionexamples/python-script/README.mdexamples/python-script/instant.perms.tsexamples/python-script/instant.schema.tsexamples/python-script/main.pyexamples/python-script/pyproject.toml
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
client/packages/python/README.md (1)
16-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the unclosed paragraph block in the centered links section.
The block opened on Line 16 uses
<p align="center">but Line 21 has<p>instead of</p>, leaving invalid HTML.Suggested fix
-<p> +</p>🤖 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 `@client/packages/python/README.md` around lines 16 - 21, The centered links paragraph is left with an opening tag "<p align=\"center\">" but closed with "<p>" instead of the correct "</p>"; update the closing tag to "</p>" so the block becomes a properly formed paragraph (locate the block containing "<p align=\"center\">" and replace the erroneous "<p>" with "</p>").
🤖 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.
Duplicate comments:
In `@client/packages/python/README.md`:
- Around line 16-21: The centered links paragraph is left with an opening tag
"<p align=\"center\">" but closed with "<p>" instead of the correct "</p>";
update the closing tag to "</p>" so the block becomes a properly formed
paragraph (locate the block containing "<p align=\"center\">" and replace the
erroneous "<p>" with "</p>").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a0bff568-f3e0-413e-9def-ee35f1c31924
📒 Files selected for processing (3)
client/packages/cli/src/commands/genpy.tsclient/packages/python/README.mdclient/www/app/docs/start-python/page.md
✅ Files skipped from review due to trivial changes (1)
- client/www/app/docs/start-python/page.md
🚧 Files skipped from review as they are similar to previous changes (1)
- client/packages/cli/src/commands/genpy.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@client/packages/cli/src/commands/genpy.ts`:
- Around line 174-180: The buildEntityModelsPy flow currently uses
safePyIdent(attrName) and pushes into fieldsByEntity[entName] without ensuring
per-entity uniqueness, causing duplicate Python attributes when two different
raw keys sanitize to the same name; update the population logic in
buildEntityModelsPy (where safePyIdent, fieldsByEntity, and the pushed field
objects with name/rawKey/hasDefault are handled) to detect collisions per entity
(e.g., maintain a map from sanitized name → count) and disambiguate by appending
a deterministic suffix (like _1, _2) to the field.name while preserving the
original rawKey for aliasing, and ensure any downstream code that emits the
class uses the disambiguated field.name and rawKey for alias=... so no fields
are overwritten.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 73f3544d-f9b5-47bd-baad-ec98fde5647f
📒 Files selected for processing (15)
.github/workflows/python-publish.ymlclient/packages/cli/src/commands/genpy.tsclient/packages/python/README.mdclient/packages/python/pyproject.tomlclient/packages/python/scripts/run_unasync.pyclient/packages/python/src/instantdb/_async/storage.pyclient/packages/python/src/instantdb/_async/streams/writer.pyclient/packages/python/src/instantdb/_async/webhooks/manager.pyclient/packages/python/src/instantdb/_io.pyclient/packages/python/src/instantdb/_sync/storage.pyclient/packages/python/src/instantdb/_sync/webhooks/manager.pyclient/packages/python/src/instantdb/_webhooks_crypto.pyclient/packages/python/tests/test_storage.pyclient/packages/python/tests/test_webhooks_crypto.pyclient/sandbox/admin-sdk-python/src/main_sync.py
✅ Files skipped from review due to trivial changes (1)
- client/packages/python/README.md
🚧 Files skipped from review as they are similar to previous changes (7)
- client/packages/python/tests/test_storage.py
- client/packages/python/pyproject.toml
- client/packages/python/tests/test_webhooks_crypto.py
- client/packages/python/scripts/run_unasync.py
- client/packages/python/src/instantdb/_async/webhooks/manager.py
- client/packages/python/src/instantdb/_sync/webhooks/manager.py
- client/sandbox/admin-sdk-python/src/main_sync.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@client/packages/python/src/instantdb/_async/_upload_io.py`:
- Around line 58-63: Open the Path before measuring to avoid TOCTOU: call
file.open("rb") into fp first, compute the size from the opened handle (seek to
end and tell, then seek back to start) instead of using file.stat(), then call
_check_size_match(file_size, size, str(file)); ensure that if _check_size_match
(or any pre-return step) raises you close fp before propagating the error, and
finally return _UploadBody(_aiter_file(fp), size, fp.close). Reference: Path,
_aiter_file, _UploadBody, _check_size_match.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: dcab2256-e590-444b-b90b-f9dd836e2be6
📒 Files selected for processing (11)
client/packages/python/scripts/run_unasync.pyclient/packages/python/src/instantdb/_async/_upload_io.pyclient/packages/python/src/instantdb/_async/http.pyclient/packages/python/src/instantdb/_async/storage.pyclient/packages/python/src/instantdb/_io.pyclient/packages/python/src/instantdb/_sync/_upload_io.pyclient/packages/python/src/instantdb/_sync/http.pyclient/packages/python/src/instantdb/_sync/storage.pyclient/packages/python/tests/test_storage.pyclient/sandbox/admin-sdk-python/src/main.pyclient/www/app/docs/start-python/page.md
✅ Files skipped from review due to trivial changes (2)
- client/packages/python/src/instantdb/_sync/http.py
- client/www/app/docs/start-python/page.md
🚧 Files skipped from review as they are similar to previous changes (3)
- client/packages/python/src/instantdb/_async/http.py
- client/packages/python/scripts/run_unasync.py
- client/sandbox/admin-sdk-python/src/main.py
Foundation: hatchling package, version sync from version.ts, runtime deps. `_AsyncHTTP` over httpx with per-request header construction; `shared_client=` lets `as_user()` scoped clients reuse the parent's connection pool. `AsyncInstant`: query, transact, debug_query/debug_transact, as_user, async context manager. `app_id` / `admin_token` fall back to `INSTANT_APP_ID` / `INSTANT_ADMIN_TOKEN` env vars. Surface: `AsyncAuth` (magic codes, tokens, user CRUD), `AsyncStorage` (upload_file accepts bytes / Path / file-like), `AsyncRooms` (get_presence), `_TxBuilder` (with underscore-attr guard so copy/pickle/Jupyter introspection probes get AttributeError). CI: `python.yml` (ruff + mypy strict + pytest), `python-publish.yml` (OIDC publish to PyPI on version.ts bump). Sandbox at `client/sandbox/admin-sdk-python/` for integration testers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`AsyncInstant.subscribe_query(q, *, rule_params=...)` returns an
`AsyncSubscription` (async iterator + context manager). Background
task streams `/admin/subscribe-query` SSE into a bounded
`asyncio.Queue` (cap 100, drops oldest — matches JS).
Payload shape:
{"type": "ok", "data", "page_info", "session_info"}
{"type": "error", "error": InstantAPIError, "ready_state", ...}
Server kebab-and-question-mark keys (`machine-id`, `has-next-page?`)
translated to snake_case at the SDK boundary. Context exit cancels
the background task and closes the SSE stream (incl. exception path).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/python-publish.yml (1)
9-12: ⚡ Quick winAdd a least-privilege
permissionsblock.The
buildjob has nopermissions:declaration, so it runs the checkout/build with the defaultGITHUB_TOKENscopes (often write). For a workflow that publishes to PyPI, scope the token down explicitly. Thepublishjob already restricts toid-token: write; adding a read-only default at the top keepsbuildminimal too.🔒️ Proposed change
on: push: branches: [main] paths: - 'client/packages/version/src/version.ts' +permissions: + contents: read + jobs: build: name: Build wheel runs-on: ubuntu-latest🤖 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/python-publish.yml around lines 9 - 12, Add a top-level least-privilege permissions block to the workflow to limit GITHUB_TOKEN scopes for the entire run (so the build job isn't granted broad write scopes). Modify the workflow root (the jobs: / build: section shown with name "Build wheel" and runs-on "ubuntu-latest") to include a permissions: declaration such as read-only for repository contents (e.g., contents: read) and any other minimal scopes needed, leaving the existing publish job's permissions (id-token: write) intact.
🤖 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.
Nitpick comments:
In @.github/workflows/python-publish.yml:
- Around line 9-12: Add a top-level least-privilege permissions block to the
workflow to limit GITHUB_TOKEN scopes for the entire run (so the build job isn't
granted broad write scopes). Modify the workflow root (the jobs: / build:
section shown with name "Build wheel" and runs-on "ubuntu-latest") to include a
permissions: declaration such as read-only for repository contents (e.g.,
contents: read) and any other minimal scopes needed, leaving the existing
publish job's permissions (id-token: write) intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c4fab513-2e62-4e58-8c8f-e42d876985e6
⛔ Files ignored due to path filters (1)
client/packages/cli/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (81)
.github/workflows/python-publish.yml.github/workflows/python.ymlclient/packages/cli/package.jsonclient/packages/cli/src/commands/genpy.tsclient/packages/cli/src/context/projectInfo.tsclient/packages/cli/src/index.tsclient/packages/cli/src/util/loadConfig.tsclient/packages/cli/src/util/projectDir.tsclient/packages/create-instant-app/__tests__/e2e/create-instant-app.e2e.test.tsclient/packages/create-instant-app/scripts/copyExamples.tsclient/packages/create-instant-app/src/cli.tsclient/packages/create-instant-app/src/env.tsclient/packages/create-instant-app/src/index.tsclient/packages/python/.gitignoreclient/packages/python/Makefileclient/packages/python/README.mdclient/packages/python/pyproject.tomlclient/packages/python/scripts/run_unasync.pyclient/packages/python/src/instantdb/__init__.pyclient/packages/python/src/instantdb/_async/__init__.pyclient/packages/python/src/instantdb/_async/_upload_io.pyclient/packages/python/src/instantdb/_async/auth.pyclient/packages/python/src/instantdb/_async/client.pyclient/packages/python/src/instantdb/_async/http.pyclient/packages/python/src/instantdb/_async/rooms.pyclient/packages/python/src/instantdb/_async/storage.pyclient/packages/python/src/instantdb/_async/streams/__init__.pyclient/packages/python/src/instantdb/_async/streams/_connection.pyclient/packages/python/src/instantdb/_async/streams/reader.pyclient/packages/python/src/instantdb/_async/streams/writer.pyclient/packages/python/src/instantdb/_async/subscribe.pyclient/packages/python/src/instantdb/_async/webhooks/__init__.pyclient/packages/python/src/instantdb/_async/webhooks/manager.pyclient/packages/python/src/instantdb/_async/webhooks/receiver.pyclient/packages/python/src/instantdb/_errors.pyclient/packages/python/src/instantdb/_http_errors.pyclient/packages/python/src/instantdb/_io.pyclient/packages/python/src/instantdb/_sync/__init__.pyclient/packages/python/src/instantdb/_sync/_upload_io.pyclient/packages/python/src/instantdb/_sync/auth.pyclient/packages/python/src/instantdb/_sync/client.pyclient/packages/python/src/instantdb/_sync/http.pyclient/packages/python/src/instantdb/_sync/rooms.pyclient/packages/python/src/instantdb/_sync/storage.pyclient/packages/python/src/instantdb/_sync/webhooks/__init__.pyclient/packages/python/src/instantdb/_sync/webhooks/manager.pyclient/packages/python/src/instantdb/_sync/webhooks/receiver.pyclient/packages/python/src/instantdb/_transact.pyclient/packages/python/src/instantdb/_version.pyclient/packages/python/src/instantdb/_webhooks_crypto.pyclient/packages/python/src/instantdb/py.typedclient/packages/python/tests/__init__.pyclient/packages/python/tests/conftest.pyclient/packages/python/tests/test_client.pyclient/packages/python/tests/test_storage.pyclient/packages/python/tests/test_streams_state.pyclient/packages/python/tests/test_subscription_state.pyclient/packages/python/tests/test_sync_client.pyclient/packages/python/tests/test_transact.pyclient/packages/python/tests/test_validation.pyclient/packages/python/tests/test_webhooks_crypto.pyclient/packages/python/tests/test_webhooks_dispatch.pyclient/sandbox/admin-sdk-python/.env.exampleclient/sandbox/admin-sdk-python/.gitignoreclient/sandbox/admin-sdk-python/README.mdclient/sandbox/admin-sdk-python/pyproject.tomlclient/sandbox/admin-sdk-python/src/__init__.pyclient/sandbox/admin-sdk-python/src/main.pyclient/sandbox/admin-sdk-python/src/main_sync.pyclient/www/app/docs/backend/page.mdclient/www/app/docs/start-python/page.mdclient/www/data/docsNavigation.jsexamples/python-script/.env.exampleexamples/python-script/.gitignoreexamples/python-script/.python-versionexamples/python-script/README.mdexamples/python-script/instant.perms.tsexamples/python-script/instant.schema.tsexamples/python-script/instant_types.pyexamples/python-script/main.pyexamples/python-script/pyproject.toml
✅ Files skipped from review due to trivial changes (12)
- client/packages/python/.gitignore
- client/sandbox/admin-sdk-python/.gitignore
- client/sandbox/admin-sdk-python/pyproject.toml
- examples/python-script/.python-version
- client/packages/create-instant-app/scripts/copyExamples.ts
- client/sandbox/admin-sdk-python/README.md
- client/packages/python/src/instantdb/_sync/rooms.py
- examples/python-script/.gitignore
- client/packages/python/src/instantdb/_sync/webhooks/init.py
- client/packages/python/src/instantdb/_sync/init.py
- client/www/app/docs/start-python/page.md
- examples/python-script/README.md
🚧 Files skipped from review as they are similar to previous changes (36)
- client/www/app/docs/backend/page.md
- examples/python-script/pyproject.toml
- examples/python-script/instant.perms.ts
- client/packages/python/src/instantdb/_async/webhooks/init.py
- client/www/data/docsNavigation.js
- client/packages/python/tests/conftest.py
- examples/python-script/instant.schema.ts
- client/packages/python/src/instantdb/init.py
- client/packages/python/src/instantdb/_http_errors.py
- examples/python-script/main.py
- client/packages/python/src/instantdb/_async/rooms.py
- client/packages/python/src/instantdb/_errors.py
- client/packages/python/src/instantdb/_sync/webhooks/manager.py
- client/packages/python/src/instantdb/_async/streams/init.py
- client/packages/python/src/instantdb/_version.py
- client/packages/python/src/instantdb/_async/storage.py
- client/packages/create-instant-app/src/cli.ts
- client/packages/cli/src/index.ts
- client/packages/python/src/instantdb/_transact.py
- client/packages/python/tests/test_transact.py
- client/packages/python/src/instantdb/_async/webhooks/manager.py
- client/packages/create-instant-app/tests/e2e/create-instant-app.e2e.test.ts
- client/packages/python/src/instantdb/_io.py
- client/packages/python/tests/test_sync_client.py
- client/packages/create-instant-app/src/index.ts
- client/packages/python/tests/test_storage.py
- client/packages/python/src/instantdb/_async/subscribe.py
- client/packages/python/tests/test_subscription_state.py
- client/packages/python/src/instantdb/_async/_upload_io.py
- client/packages/python/src/instantdb/_async/auth.py
- client/packages/python/src/instantdb/_sync/client.py
- client/packages/python/src/instantdb/_sync/auth.py
- client/packages/python/pyproject.toml
- client/packages/python/src/instantdb/_async/streams/reader.py
- client/packages/python/scripts/run_unasync.py
- client/packages/python/src/instantdb/_async/streams/writer.py
Comment on lines
+163
to
+194
| function findIdentifierCollisions(schema: SchemaLike): string | undefined { | ||
| for (const entName of Object.keys(schema.entities)) { | ||
| const seen = new Map<string, string>([['id', 'implicit `id`']]); | ||
| const check = (rawKey: string, kind: string): string | undefined => { | ||
| const safe = safePyIdent(rawKey); | ||
| const existing = seen.get(safe); | ||
| if (existing !== undefined) { | ||
| return ( | ||
| `Entity "${entName}": ${existing} and ${kind} "${rawKey}" both ` + | ||
| `map to the Python identifier "${safe}". Rename one in your schema.` | ||
| ); | ||
| } | ||
| seen.set(safe, `${kind} "${rawKey}"`); | ||
| return undefined; | ||
| }; | ||
| for (const attrName of Object.keys(schema.entities[entName].attrs)) { | ||
| const err = check(attrName, 'attribute'); | ||
| if (err) return err; | ||
| } | ||
| for (const link of Object.values(schema.links)) { | ||
| if (link.forward.on === entName) { | ||
| const err = check(link.forward.label, 'link'); | ||
| if (err) return err; | ||
| } | ||
| if (link.reverse.on === entName) { | ||
| const err = check(link.reverse.label, 'link'); | ||
| if (err) return err; | ||
| } | ||
| } | ||
| } | ||
| return undefined; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for entity-name validation/uniqueness rules in the schema definition layer.
rg -nP --type=ts -C3 '(InstantSchemaDef|entities)\b' client/packages/core/src 2>/dev/null | rg -nP -C3 '(reserved|\$|duplicate|already|unique|validate)' || true
rg -nP --type=ts -C3 'className\s*\(' client/packages/cli/src/commands/genpy.tsRepository: instantdb/instant
Length of output: 15728
Add cross-entity className(...) collision detection in genpy
findIdentifierCollisions only checks safePyIdent clashes within a single entity’s attrs/links vs id; it doesn’t reject distinct entity names that normalize to the same Python class name (e.g. my-entity/my_entity, $users/users). When that happens, buildPy will emit duplicate class ${className(entName)}(BaseModel) definitions and the "entities" / "records" mappings will end up pointing at the last emitted class, silently dropping the earlier entity.
🛠️ Proposed guard
function findIdentifierCollisions(schema: SchemaLike): string | undefined {
+ const classNames = new Map<string, string>();
+ for (const entName of Object.keys(schema.entities)) {
+ const cls = className(entName);
+ const prev = classNames.get(cls);
+ if (prev !== undefined) {
+ return (
+ `Entities "${prev}" and "${entName}" both map to the Python class ` +
+ `"${cls}". Rename one in your schema.`
+ );
+ }
+ classNames.set(cls, entName);
+ }
for (const entName of Object.keys(schema.entities)) {
const seen = new Map<string, string>([['id', 'implicit `id`']]);
const check = (rawKey: string, kind: string): string | undefined => {
const safe = safePyIdent(rawKey);
const existing = seen.get(safe);
if (existing !== undefined) {
return (
`Entity "${entName}": ${existing} and ${kind} "${rawKey}" both ` +
`map to the Python identifier "${safe}". Rename one in your schema.`
);
}
seen.set(safe, `${kind} "${rawKey}"`);
return undefined;
};
for (const attrName of Object.keys(schema.entities[entName].attrs)) {
const err = check(attrName, 'attribute');
if (err) return err;
}
for (const link of Object.values(schema.links)) {
if (link.forward.on === entName) {
const err = check(link.forward.label, 'link');
if (err) return err;
}
if (link.reverse.on === entName) {
const err = check(link.reverse.label, 'link');
if (err) return err;
}
}
}
return undefined;
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it says on the tin.
Testing instructions coming soon!