Embedded toc - #11
Closed
rejojer wants to merge 27 commits into
Closed
Conversation
Feat/mistral.fix
vcpandya
pushed a commit
to vcpandya/IDP
that referenced
this pull request
Jul 4, 2026
Original task (VectifyAI#11): the OAuth state token was stored in a per-process `_STATE_STORE` dict in `idpkit/connectors/oauth.py`. Under multi-worker deployments (e.g. Gunicorn + UvicornWorker) the worker handling the callback was often not the one that issued the token, causing "Invalid state" errors and broken Google logins. Changes: - New `OAuthState` SQLAlchemy model in `idpkit/db/models.py` (`oauth_states` table: token PK, JSON payload, indexed expires_at, created_at). Picked up automatically by `Base.metadata.create_all` in `init_db`, so no manual migration is required. - `idpkit/connectors/oauth.py`: * Removed `_STATE_STORE` dict. * `new_state(db, payload)` and `consume_state(db, token)` are now async, take an AsyncSession, and persist/atomically pop rows. * 10-minute TTL (`STATE_TTL`); expired rows are pruned best-effort on every issue/consume call (no separate sweeper needed). * Consume returns None for unknown OR expired tokens; expired rows are deleted in the same transaction. * Consume uses a single `DELETE ... RETURNING` so concurrent callbacks racing on the same token cannot both succeed (addresses code-review note about atomicity). - `idpkit/api/routes/connectors.py`: `/oauth/start` now depends on `get_db` and awaits `new_state`; `/oauth/callback` awaits `consume_state(db, state)`. Verification: - `python -c "from idpkit.connectors.oauth import ..."` imports cleanly and the new table is registered with `Base.metadata`. - pytest could not run end-to-end in this sandbox due to a pre-existing environment issue (`libstdc++.so.6` missing, breaking `tokenizers`/`greenlet` at import time) — unrelated to this change. Existing OAuth tests in `tests/test_connectors.py` only touch `_oauth_redirect_uri` and runtime refresh paths, which are unchanged. No deviations from the task scope. Replit-Task-Id: 178ac967-325f-4b70-b5a2-ba380f0b7482
vcpandya
pushed a commit
to vcpandya/IDP
that referenced
this pull request
Jul 4, 2026
Original task (VectifyAI#11): the OAuth state token was stored in a per-process `_STATE_STORE` dict in `idpkit/connectors/oauth.py`. Under multi-worker deployments (e.g. Gunicorn + UvicornWorker) the worker handling the callback was often not the one that issued the token, causing "Invalid state" errors and broken Google logins. Changes: - New `OAuthState` SQLAlchemy model in `idpkit/db/models.py` (`oauth_states` table: token PK, JSON payload, indexed expires_at, created_at). Picked up automatically by `Base.metadata.create_all` in `init_db`, so no manual migration is required. - `idpkit/connectors/oauth.py`: * Removed `_STATE_STORE` dict. * `new_state(db, payload)` and `consume_state(db, token)` are now async, take an AsyncSession, and persist/atomically pop rows. * 10-minute TTL (`STATE_TTL`); expired rows are pruned best-effort on every issue/consume call (no separate sweeper needed). * Consume returns None for unknown OR expired tokens; expired rows are deleted in the same transaction. * Consume uses a single `DELETE ... RETURNING` so concurrent callbacks racing on the same token cannot both succeed (addresses code-review note about atomicity). - `idpkit/api/routes/connectors.py`: `/oauth/start` now depends on `get_db` and awaits `new_state`; `/oauth/callback` awaits `consume_state(db, state)`. Verification: - `python -c "from idpkit.connectors.oauth import ..."` imports cleanly and the new table is registered with `Base.metadata`. - pytest could not run end-to-end in this sandbox due to a pre-existing environment issue (`libstdc++.so.6` missing, breaking `tokenizers`/`greenlet` at import time) — unrelated to this change. Existing OAuth tests in `tests/test_connectors.py` only touch `_oauth_redirect_uri` and runtime refresh paths, which are unchanged. No deviations from the task scope. Replit-Task-Id: 178ac967-325f-4b70-b5a2-ba380f0b7482
KylinMountain
added a commit
that referenced
this pull request
Jul 8, 2026
Verified against current dev; the compat/behavior decisions (#7 api_key semantics, #10 CLI flags, #11 doc-description default) are deferred. Crashes: - page_index(): snapshot args before importing IndexConfig — locals() was capturing the imported class and IndexConfig(extra='forbid') made every call raise ValidationError. - process_none_page_numbers: pop('page', None) instead of del (a TOC item without 'page' raised KeyError mid-pipeline). - pipeline._run_async: guard only the loop detection, not the run, so a real RuntimeError from the coroutine isn't masked as "asyncio.run() cannot be called from a running event loop". Silent-wrong / robustness: - LocalBackend.get_document_structure and the agent get_document / get_document_structure tools now surface a missing doc (raise / error-JSON) instead of returning empty, matching get_page_content and the cloud backend. - cloud delete_collection drops the cached folder_id. - cloud query raises on an empty collection instead of POSTing doc_id:[]. - LocalClient skips the API-key check for keyless providers (ollama, lm_studio, …) so keyless LiteLLM models aren't rejected at construction. Compat / cleanup: - md_to_tree coerces legacy 'yes'/'no' string flags (a bare 'no' was truthy). - FileTypeError also subclasses ValueError (0.2.x raised ValueError). - _validate_llm_provider no longer mutates global litellm.model_cost_map_url. - __all__ re-includes legacy exports (page_index, md_to_tree, get_*). - Rewrite examples/agentic_vectorless_rag_demo.py to the Collection API and use the in-repo attention.pdf (the old workspace=/client.index/client.documents API no longer exists). Adds tests/test_review_fixes.py (10 regressions). Full suite: 189 passed. Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
KylinMountain
added a commit
that referenced
this pull request
Jul 8, 2026
Resolves PR #272 review #10/P5. The --if-add-node-id / -node-summary / -doc-description / -node-text args were store_true, which rejected the documented yes/no values and left default-on options impossible to disable from the CLI. They now use nargs='?' + const=True + a yes/no-coercing type: --if-add-node-id -> on --if-add-node-id no -> off (legacy form still works) (omitted) -> use the IndexConfig default README updated to the flag usage (noting the legacy `no` off-switch), and --if-add-node-text is now documented too. Decisions from the review: - #7 (api_key semantics): verified FALSE POSITIVE — 0.2.x is a cloud SDK whose api_key is a PageIndex cloud key (cloud_api.LegacyCloudAPI + docs.pageindex.ai/sdk), matching the new SDK. No change. - #11 (if_add_doc_description default True): kept intentionally (open mode). Claude-Session: https://claude.ai/code/session_01Kx5DgKbhK1N8autqXH8SmS
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.
No description provided.