Skip to content

Bump responses from 0.21.0 to 0.22.0#4

Merged
JSv4 merged 1 commit into
mainfrom
dependabot/pip/responses-0.22.0
Oct 24, 2022
Merged

Bump responses from 0.21.0 to 0.22.0#4
JSv4 merged 1 commit into
mainfrom
dependabot/pip/responses-0.22.0

Conversation

@dependabot
Copy link
Copy Markdown
Contributor

@dependabot dependabot Bot commented on behalf of github Oct 24, 2022

Bumps responses from 0.21.0 to 0.22.0.

Release notes

Sourced from responses's releases.

0.22.0

  • Update requests dependency to the version of 2.22.0 or higher. See #584.
  • [BETA] Added possibility to record responses to TOML files via @_recorder.record(file_path="out.toml") decorator.
  • [BETA] Added possibility to replay responses (populate registry) from TOML files via responses._add_from_file(file_path="out.toml") method.
  • Fix type for the mock's patcher object. See #556
  • Fix type annotation for CallList
  • Add passthrough argument to BaseResponse object. See #557
  • Fix registries leak. See #563
  • OriginalResponseShim is removed. See #585
  • Add support for the loose version of json_params_matcher via named argument strict_match. See #551
  • Add lists support as JSON objects in json_params_matcher. See #559
  • Added project links to pypi listing.
  • delete, get, head, options, patch, post, put shortcuts are now implemented using functools.partialmethod.
  • Fix MaxRetryError exception. Replace exception by RetryError according to requests implementation. See #572.
  • Adjust error message when Retry is exhausted. See #580.
Changelog

Sourced from responses's changelog.

0.22.0

  • Update requests dependency to the version of 2.22.0 or higher. See #584.
  • [BETA] Added possibility to record responses to TOML files via @_recorder.record(file_path="out.toml") decorator.
  • [BETA] Added possibility to replay responses (populate registry) from TOML files via responses._add_from_file(file_path="out.toml") method.
  • Fix type for the mock's patcher object. See #556
  • Fix type annotation for CallList
  • Add passthrough argument to BaseResponse object. See #557
  • Fix registries leak. See #563
  • OriginalResponseShim is removed. See #585
  • Add support for the loose version of json_params_matcher via named argument strict_match. See #551
  • Add lists support as JSON objects in json_params_matcher. See #559
  • Added project links to pypi listing.
  • delete, get, head, options, patch, post, put shortcuts are now implemented using functools.partialmethod.
  • Fix MaxRetryError exception. Replace exception by RetryError according to requests implementation. See #572.
  • Adjust error message when Retry is exhausted. See #580.
Commits

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

Bumps [responses](https://github.com/getsentry/responses) from 0.21.0 to 0.22.0.
- [Release notes](https://github.com/getsentry/responses/releases)
- [Changelog](https://github.com/getsentry/responses/blob/master/CHANGES)
- [Commits](getsentry/responses@0.21.0...0.22.0)

---
updated-dependencies:
- dependency-name: responses
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot Bot added dependencies Pull requests that update a dependency file python Pull requests that update Python code labels Oct 24, 2022
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (main@cb6f7c8). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main       #4   +/-   ##
=======================================
  Coverage        ?   66.19%           
=======================================
  Files           ?       47           
  Lines           ?     1855           
  Branches        ?        0           
=======================================
  Hits            ?     1228           
  Misses          ?      627           
  Partials        ?        0           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@JSv4 JSv4 merged commit 49f2381 into main Oct 24, 2022
@JSv4 JSv4 deleted the dependabot/pip/responses-0.22.0 branch October 24, 2022 06:05
@claude claude Bot mentioned this pull request Nov 4, 2025
JSv4 added a commit that referenced this pull request Dec 7, 2025
Critical Issue #1 - Data Integrity:
- Add CheckConstraint to Annotation model ensuring annotations in
  structural_set must have structural=True (enforces assumption in
  query_optimizer.py:207-209)
- Add same constraint to Relationship model for consistency
- Add migration 0050 for the new database constraints
- Add 6 test cases in test_structural_annotation_sets.py

Critical Issue #2 - IDOR Prevention:
- Fix CreateThreadMessageMutation to check permissions BEFORE checking
  locked status (prevents information disclosure via different error msgs)
- Fix ReplyToMessageMutation with same pattern
- Use generic "Cannot post/reply in this thread" for both cases

Issue #4 - Frontend Test Configuration:
- Add --reporter=list to test:ct script in package.json to prevent
  test hanging as documented in CLAUDE.md
JSv4 added a commit that referenced this pull request Jan 2, 2026
- Add FILE_SIZE and TIME_UNITS constants to constants.ts
- Create formatters.ts with shared formatting utilities:
  - formatFileSize: Human-readable file size formatting
  - formatRelativeTime: Relative time formatting (e.g., "5 hours ago")
  - formatCompactRelativeTime: Compact format for activity feeds
  - getInitials: Extract initials from name/email for avatars
- Update Documents.tsx to use shared utilities
- Remove duplicate helper functions from Documents.tsx

This addresses CLAUDE.md Critical Concept #4 (no magic numbers) and
improves DRY compliance by consolidating formatting logic.
JSv4 added a commit that referenced this pull request Jan 2, 2026
- Add concept #6: Utility functions belong in utility files
- Remind agents to check existing utils before writing new ones
- Guide agents to extract inline utilities during code review
- Update concept #4 to reference both backend and frontend constants
JSv4 added a commit that referenced this pull request May 14, 2026
* Harden Auth0 permissioning: superuser allowlist, IDOR oracles, callback token at-rest

End-to-end audit of every Auth0-touching authorization pathway. Twelve
verified findings shipped; agent-reported false positives (intentional
IDOR-safe SetCorpusVisibility, AnnotationImagesView, moderation mutations,
moderation_metrics, etc.) verified in source and dropped.

- F1: AUTH0_SUPERUSER_SUB_ALLOWLIST gates is_superuser elevation in
  sync_admin_claims_from_payload regardless of JWT claim, blocking the
  user_metadata-sourced privilege escalation path. New users.W001 system
  check warns on empty allowlist when USE_AUTH0=True.
- F2: ADMIN_CLAIMS_CACHE_TTL 300s -> 30s, tightening the privilege
  retention window after Auth0 demotion.
- F3+F4: AddRelationship now uses Annotation/Corpus visible_to_user with
  count comparison and a single unified not-found message; broad except
  no longer surfaces ORM text.
- F5+F6: CreateMetadataColumn and UpdateMetadataColumn use
  visible_to_user + unified not-found-or-no-permission message.
- F7: Datacell mutations catch DoesNotExist explicitly and stop
  echoing ORM exception text.
- F9: Bounded JWKS stale-cache fallback (1h grace past TTL); fails
  closed beyond the window so a key compromise + Auth0 outage cannot
  let the old key keep verifying tokens indefinitely.
- F10: Analysis.callback_token is now stored as SHA-256 hex
  (callback_token_hash). rotate_callback_token() returns a fresh
  plaintext at submit time; verify_callback_token() does the
  constant-time check on incoming requests. Migration backfills
  existing UUID tokens by hash so in-flight analyzers continue to
  authenticate. Submit log redacted.
- F8+F11: Documentation-only. Step 7 walkthrough for the allowlist,
  env-var rows for AUTH0_SUPERUSER_SUB_ALLOWLIST + AUTH0_CREATE_NEW_USERS,
  app_metadata-only requirement called out in danger callout, inline
  comment that User.email is informational (not an identity field).

Operator action required before deploy: populate
AUTH0_SUPERUSER_SUB_ALLOWLIST with the Auth0 sub of every user who must
retain Django is_superuser. Existing superusers whose subs are not in
the allowlist will be demoted within ~30s of their next API call.

* Address PR #1641 review: fix AddRelationship document IDOR + black format

* Fix pytest failures and lift patch coverage for PR #1641

CI was failing because the AUTH0_SUPERUSER_SUB_ALLOWLIST gate (added in
this PR) blocks is_superuser elevation, but several admin-auth tests
were still asserting elevation without populating the allowlist. The
test_metadata_columns_graphql test was also still asserting the old
"don't have permission" string, which the PR replaced with the IDOR-safe
unified message.

Test updates (failing -> passing):
- test_admin_auth.TestAdminClaimsSync.test_sync_is_superuser_true_from_claims
- test_admin_auth.TestAdminClaimsSync.test_sync_both_claims
- test_admin_auth.TestGetUserByPayloadWithClaimSync.test_sync_claims_cached_handles_cache_failure
- test_metadata_columns_graphql.test_create_metadata_column_without_permission

Address Claude review findings:
- (#2) AUTH0_ADMIN_CLAIMS_CACHE_TTL is now configurable via env var; the
  module constant stays the default.
- (#4) Reverse migration 0021 uses QuerySet.update() instead of
  model.save() so historical-model signals do not fire on downgrade.
- (#5) Token hash backfill iterates with chunk_size=500 to bound memory.
- (#7) Wire AUTH0_CREATE_NEW_USERS into AUTH0_JWT from env so deployments
  can disable user auto-provisioning without editing settings.
- (#8) Silence users.W001 in test settings so the deliberate
  empty-allowlist tests don't print system-check noise.
- (#1) _can_serve_stale now asserts it is called under the JWKS lock and
  carries a prominent docstring warning.

New tests added to cover patch lines lacking coverage:
- TestAuth0SuperuserAllowlist (3 tests) — empty allowlist, sub not in
  allowlist, and demote-bypasses-allowlist paths.
- TestAuth0SuperuserAllowlistSystemCheck (3 tests) — direct unit tests
  for users.checks.check_auth0_superuser_allowlist.
- TestAnalysisCallbackTokenHelpers (6 tests) — direct unit tests for
  Analysis.rotate_callback_token / verify_callback_token covering empty
  hash, None/empty candidate, plaintext-not-stored, and rotation
  invalidating prior plaintexts.
- TestCanServeStale (3 tests) — direct unit tests for _can_serve_stale
  (no-data, inside-grace, outside-grace).
- test_network_error_after_stale_grace_fails_closed and
  test_json_error_after_stale_grace_fails_closed — JWKS fails closed
  outside the stale-grace window.
- test_admin_claims_cache_ttl_env_override and
  test_admin_claims_cached_noop_when_auth0_disabled — exercise the new
  env-configurable TTL and the USE_AUTH0=False short-circuit.

* Update CHANGELOG with PR #1641 review follow-ups

* Cover Datacell mutation not-found IDOR paths

The PR introduced explicit Datacell.DoesNotExist branches in
ApproveDatacell / RejectDatacell / EditDatacell so the broad-except
no longer leaks ORM text. These paths were not covered by the existing
happy-path datacell tests.

Adds four tests:
- approve / reject / edit returning the unified Datacell not found.
  message for an unknown id
- approve on a datacell owned by another user (queryset filters by
  creator, so DoesNotExist fires) returning the same message — verifies
  the IDOR-safe property end to end

* Cover AddRelationship bad-id and unknown-id IDOR paths

The PR's AddRelationship mutation collapses both unparseable global IDs
(caught by the outer try/except) and unknown but well-formed IDs (caught
by the visible_to_user count comparison) into a single not-found
message. Adds two tests:

- test_garbage_global_ids_yield_not_found_message: a base64-decodable but
  bogus global ID exercises the outer broad-except branch.
- test_unknown_source_annotation_id_yields_not_found: a syntactically
  valid global ID pointing at no annotation exercises the count-mismatch
  branch.

Both paths must produce the unified not-found message so the response
shape gives no enumeration oracle.

* Address PR #1641 review round 2

Latest Claude review:
- (#1) Lock assert was misleading because threading.Lock.locked() returns
  True if any thread holds the lock, not the caller. Replaced the assert
  with a sharper docstring note that documents the limitation and points
  future refactors at threading.RLock.
- (#2 / #10) Migration backfill and reverse now use bulk_update batches
  to keep DB round-trips bounded.
- (#3) AddRelationship now visibility-checks relationship_label_pk too,
  closing the residual oracle where an attacker could probe private
  AnnotationLabel IDs.
- (#5) Added an inline comment at the allowlist gate documenting why
  is_staff is intentionally NOT allowlist-gated and how to layer one.
- (#7) test.py SILENCED_SYSTEM_CHECKS comment now cross-references the
  direct-call test that bypasses the silencing.
- (#9) submit_corpus_documents_to_analyzer logging switched from f-string
  to %-style for consistency with the new logger.exception calls.

New test:
- test_inaccessible_relationship_label_yields_not_found verifies the
  new label visibility check returns the unified not-found message and
  blocks the Relationship row from being created.

* Address PR #1641 review round 3 (security tag, auto-save, log cleanup)

Earlier Claude review notes:
- rotate_callback_token now auto-saves when self.pk is set, so callers
  can no longer ship a plaintext to the worker while leaving the DB
  hash stale. Unsaved-instance behaviour is unchanged (caller still
  owns the first save). Two new tests pin both branches.
- check_auth0_superuser_allowlist is now registered under Tags.security
  so it shows up under ``manage.py check --tag security``.
- logger.exception(...) calls in annotation_mutations and
  extract_mutations no longer pass ``e``; ``logger.exception`` captures
  the traceback automatically and the extra arg double-logged the
  exception text.

* Document AUTH0_ADMIN_CLAIMS_CACHE_TTL env var

Adds the AUTH0_ADMIN_CLAIMS_CACHE_TTL row to the env-var table so
operators can find the configurable knob added in this PR.

* Address PR #1641 review round 4 (non-integer pk, atomic note)

- (#1) AddRelationship now casts each parsed pk to ``int()`` inside the
  outer try/except so a base64-decodable but non-numeric global ID
  (``"BogusType:not-an-int"``) fails closed at parse time instead of
  later at the queryset boundary. Updated
  test_garbage_global_ids_yield_not_found_message to assert the
  not-found response directly now that the path is deterministic.
- (#5) Migration 0021 now carries a module docstring explaining why
  the AddField + backfill + RemoveField operations stay in one atomic
  transaction (partial runs would leave live rows without a hash) and
  flags the expected lock window for large tables. bulk_update keeps
  the transaction short.

Items addressed from this review but already in place: AnnotationLabel
inherits BaseOCModel so ``.objects.visible_to_user()`` works
out-of-the-box; AUTH0_CREATE_NEW_USERS is consumed via
``auth0_settings.AUTH0_CREATE_NEW_USERS`` in
``get_auth0_user_from_token`` which reads from the AUTH0_JWT dict;
AUTH0_JWT is only assigned in base.py (verified via grep).

* Escalate users.W001 to users.E001 when existing superusers will be demoted

Per latest review: when ``USE_AUTH0=True`` AND
``AUTH0_SUPERUSER_SUB_ALLOWLIST`` is empty AND at least one is_superuser
row already exists, the next claim sync will silently demote that
superuser within ~30s of their next API call. The previous Warning was
too quiet for this footgun.

The check now:
- Emits ``users.E001`` (Critical) when an existing superuser would be
  silently demoted. Critical errors block ``manage.py check --deploy``
  so a deploy that would lock out the only admin account fails fast.
- Falls back to ``users.W001`` (Warning) when no superuser rows exist,
  preserving the original advisory behaviour.
- Tolerates DB-unreachable contexts (e.g. ``makemigrations``) by
  catching ORM errors and emitting the lower-severity warning.

Added a test class setUp that demotes the migration-created initial
superuser so the W001 vs E001 branches can be tested deterministically,
plus a new test pinning the E001 path with an explicitly-created
superuser. ``users.E001`` is added to SILENCED_SYSTEM_CHECKS in
test settings so the elevated severity does not break unrelated
test runs that happen to leave a superuser in the DB.

* Fix pre-existing mypy errors in requests.post headers callers

A new ``urllib3`` type stub release narrowed ``requests.post(..., headers=)``
to ``MutableMapping[str, str | bytes]``. The four call sites below were
passing ``dict[str, str]``, which is invariant in the value type and so
fails the narrower stub even though ``str`` is a subtype of ``str | bytes``.

These mypy errors fail the linter step on ``main`` as well — they are
unrelated to PR #1641's security work — but they block the Backend CI's
pytest job from running because pytest depends on linter passing.

Fix is uniform: keep the local ``dict[str, str]`` typing (matches every
caller of ``maybe_add_cloud_run_auth`` so we don't have to widen that
shared helper too), and cast to ``Any`` at the ``requests.post/get`` call
site so mypy lets the narrower dict satisfy the wider stub. No runtime
behaviour changes; the dict is still ``str -> str`` at runtime.

Files:
- opencontractserver/users/tasks.py (lines 45, 165)
- opencontractserver/pipeline/rerankers/cohere_reranker.py (line 132)
- opencontractserver/pipeline/parsers/docxodus_parser.py (line 156)
- opencontractserver/pipeline/parsers/docling_parser_rest.py (line 344)

---------

Co-authored-by: Claude <noreply@anthropic.com>
JSv4 added a commit that referenced this pull request May 17, 2026
After merging main into PR 1665, the UserCanMixin.user_can supertype now
declares request: Any = None, but five subclass overrides added by
PR #1663 did not include that parameter and mypy flagged the signature
mismatch.

- DocumentManager / RelationshipManager / UserFeedbackManager
  (shared/Managers.py): add request kwarg and forward to super() /
  recursive Manager.user_can calls.
- AnnotationManager / NoteManager (shared/Managers.py): add request
  kwarg and forward to Analysis.objects.user_can / Extract.objects.user_can
  / Document.objects.user_can / Corpus.objects.user_can so the optimizer
  cache flows through the recursion.
- ConversationManager / ChatMessageManager (conversations/models.py):
  add request kwarg and forward to _default_user_can.

Also addresses Claude review item #7 (audit folder-service callers):
config/graphql/document_types.py:1153 (resolve_folder_in_corpus) now
passes request=info.context to DocumentFolderService.get_document_folder.
get_folder_document_count and get_folder_path have no in-tree callers.

Adds review item #4: negative regression test pinning that
refresh_from_db() does NOT clear Tier 1 (the documented footgun) so a
future change to that contract is caught by CI.
JSv4 added a commit that referenced this pull request May 17, 2026
Discovery tools (list_fieldsets, list_analyzers, list_recent_extracts,
list_recent_analyses) now check corpus visibility via
visible_to_user(user).filter(pk=...).exists() instead of bare
Corpus.objects.get(pk=...), so a caller cannot probe private corpus IDs
through an existence oracle (review item #8). The error message is
unchanged across both branches per CLAUDE.md's IDOR guidance.

list_recent_extracts and list_recent_analyses fold the per-row M2M
count into the SELECT with annotate(Count(...)) so the document_count
field no longer issues N queries on N rows (review item #1).

start_extract and start_analysis collapse the two-step
visible_to_user().exists() + .get() pattern into a single
visible_to_user().filter(pk=...).first() lookup (review item #5).
Same fix applied to the Fieldset/Analyzer lookups in those functions.

list_fieldsets now includes fieldsets pinned to the *current* corpus
(via Q(corpus__isnull=True) | Q(corpus_id=corpus_id)) so the discovery
surface matches what start_extract actually accepts — previously a
caller listing fieldsets would not see one that start_extract would
have allowed if supplied directly (review item #7).

MAX_LIST_LIMIT / DEFAULT_LIST_LIMIT / DEFAULT_RECENT_LIMIT hoisted to
opencontractserver/constants/tools.py per CLAUDE.md's no-magic-numbers
rule (review item #4).

Test hygiene: dead GremlinEngine fixture removed from
TestListRecentAnalyses.setUp (review item #10); inspect import lifted
out of test_to_core_tool_returns_async_function loop (review item #11).
JSv4 added a commit that referenced this pull request May 17, 2026
* Add two-tier permission cache for Manager.user_can (closes #1640)

PR #1637 centralized authorization in _default_user_can (corpuses Phase 0).
This follow-up keeps the cold path cheap as more models adopt the API:

Tier 1 (transparent) — per-instance memoization inside
get_users_permissions_for_obj. Stored on the instance as
_oc_granted_perms_cache, keyed by (user_id, include_group_permissions).
Anonymous users bypass. Cache is a defensive frozenset; callers receive
a set copy so _default_user_can can still fold read_<model> in for
CRUD/ALL checks without poisoning the cache.

Tier 2 (opt-in) — request-scoped PermissionQueryOptimizer mirroring
ConversationQueryOptimizer. Attached lazily to the request as
_permission_query_optimizer via get_request_optimizer(request). Keyed by
(user_id, content_type_id, instance_pk, include_group_permissions) so
multiple instances of the same model share a cache. None-tolerant for
Celery / management-command contexts.

Plumbing:
- _default_user_can / UserCanMixin / InstanceUserCanMixin / CorpusFolder.user_can
  gained a request= kwarg (kw-only, default None — backward compatible).
- DocumentFolderService classmethods (~25 entry points) accept and forward
  request; corpus GraphQL queries + mutations + corpus_folder_mutations
  pass request=info.context.
- set_permissions_for_obj_to_user gained a request= kwarg and now
  self-invalidates both tiers for the affected (user, instance) pair when
  called inside the HTTP lifecycle. Phase-0 Corpus mutation call sites
  in corpus_mutations.py and document_mutations.py pass it through;
  out-of-band callers (Celery / fixtures / importing) keep the
  positional signature. Tier 1 is still scrubbed for the target user
  even when request is None so reused instances see the new grant.

Tests: opencontractserver/tests/permissioning/test_permission_optimizer.py
covers per-instance memoization (zero queries on repeat, no leakage
across include_group_permissions, fast-path short-circuits don't
populate cache, defensive-copy contract), the request-scoped optimizer
(lazy attach, None-safe, distinct-user keying, per-user / per-instance /
total invalidation, anonymous skip), and the mutation invalidation
contract (mid-request grant visible after set_permissions_for_obj_to_user
with request; Tier 1 scrubbed when request is None).

Docs: docs/architecture/query_permission_patterns.md gains a row in the
Layer 2 table and a new "Two-Tier user_can Caching" subsection that
documents the attribute names, keying, and invalidation contract.

* Fix CI linter failures

- Apply pyupgrade rewrite of Optional[X] -> X | None in permission_optimizer.py
- Drop now-unused Optional import
- Suppress mypy arg-type warning on intentional AnonymousUser test path

* Address Claude review on PR #1665

- Document group-permission and refresh_from_db staleness boundaries on
  INSTANCE_PERMS_CACHE_ATTR, set_permissions_for_obj_to_user, and the
  arch doc so callers know when to invalidate manually
- Widen PermissionQueryOptimizer._cache type annotation: instance_pk is
  Any (UUID/str-PK models would have tripped mypy)
- Raise ValueError when invalidate() receives both `instance` and an
  explicit `content_type_id`/`instance_pk` instead of silently letting
  one win; add a regression test
- Trim the long inline two-tier-cache-invalidation comment in
  set_permissions_for_obj_to_user per the one-line WHY convention

* Add coverage for _default_user_can branches and folder_service request kwarg

Three new test classes target the codecov patch-coverage gap in
permissioning.py (71.64% → expected ~95%):

DefaultUserCanCoverageTestCase pins each user-resolution path
(None / AnonymousUser / int id / str id / unknown id / unauthenticated
double) and each permission-type branch (READ/CREATE/UPDATE/EDIT/DELETE/
COMMENT/PUBLISH/PERMISSION/CRUD/ALL) plus the is_public-fold-in path
that keeps CRUD passing on public corpora.

SetPermissionsInvalidationCoverageTestCase covers the per-user scrub
selectivity (other users' cache entries survive) and the no-request
Celery path (Tier 1 still drops).

FolderServiceRequestKwargCoverageTestCase exercises the request= kwarg
on DocumentFolderService and the permission-denial empty-queryset
branch (returns empty rather than raising).

* Address Claude review on PR #1665 and close codecov patch gaps

- Clarify the docstring on ``PermissionQueryOptimizer.get_granted``:
  the "without double-caching" wording was inaccurate. A Tier 2 cold
  miss does warm Tier 1 as a side-effect, so the same frozenset
  intentionally ends up in both caches. Spell out why that
  redundancy is desired (out-of-request callers still get Tier 1).
- Add two coverage tests targeting the previously-uncovered warming
  paths in ``get_users_permissions_for_obj``:
  * direct superuser call → guardian-superuser return path
  * pre-attached guardian prefetch → fast-path return
  Both paths exist because ``_default_user_can`` short-circuits
  before reaching the helper, so they are only exercised by callers
  that invoke the helper directly.

* Fix mypy errors in test_permission_optimizer.py

Replace assertIsNotNone with assert for type narrowing on cache
lookups, and drop an unused type:ignore comment on a Django
reverse-relation accessor that mypy now resolves correctly.

* Address Claude review: clarify Tier 2 default vs Tier 1

Add a docstring note to PermissionQueryOptimizer.get_granted explaining
why include_group_permissions defaults to True (mirrors user_can) while
the lower-level get_users_permissions_for_obj defaults to False.
Production always passes the flag explicitly, so the divergence is
benign — the note prevents future maintainers from chasing a phantom
inconsistency.

* Address review: guard partial-coordinate invalidate against PK collisions

invalidate(instance_pk=X) without content_type_id was a silent footgun: a
Corpus with pk=5 and a Document with pk=5 would both be evicted because
the wildcard match ignores model identity. Raise ValueError when an
instance_pk arrives without its content_type_id so callers must either
pair the two or use instance= shorthand. Add a regression test.

* Address review #3, #5: document Tier 2 absence in Celery + pickling staleness

* Address review: thread request= through manager.user_can overrides

After merging main into PR 1665, the UserCanMixin.user_can supertype now
declares request: Any = None, but five subclass overrides added by
PR #1663 did not include that parameter and mypy flagged the signature
mismatch.

- DocumentManager / RelationshipManager / UserFeedbackManager
  (shared/Managers.py): add request kwarg and forward to super() /
  recursive Manager.user_can calls.
- AnnotationManager / NoteManager (shared/Managers.py): add request
  kwarg and forward to Analysis.objects.user_can / Extract.objects.user_can
  / Document.objects.user_can / Corpus.objects.user_can so the optimizer
  cache flows through the recursion.
- ConversationManager / ChatMessageManager (conversations/models.py):
  add request kwarg and forward to _default_user_can.

Also addresses Claude review item #7 (audit folder-service callers):
config/graphql/document_types.py:1153 (resolve_folder_in_corpus) now
passes request=info.context to DocumentFolderService.get_document_folder.
get_folder_document_count and get_folder_path have no in-tree callers.

Adds review item #4: negative regression test pinning that
refresh_from_db() does NOT clear Tier 1 (the documented footgun) so a
future change to that contract is caught by CI.

* Address review #1665: align defaults, lock optimizer cache, scrub pickled cache

- Tier 1 `get_users_permissions_for_obj` default now `include_group_permissions=True`
  to match `PermissionQueryOptimizer.get_granted` / `_default_user_can` / every
  `Manager.user_can` surface. One default, one answer.
- `PermissionQueryOptimizer._cache` is now guarded by `threading.Lock()` for
  reads, writes, and the invalidate-loop iteration. DB work stays outside the
  lock so guardian lookups never serialise on it.
- `InstanceUserCanMixin.__getstate__` strips `INSTANCE_PERMS_CACHE_ATTR` so
  models pickled into Celery tasks can never carry a stale Tier 1 frozenset
  to the worker (footgun → impossible, not just documented).
- Comments pin reliance on `ContentType._cache` and clarify that creator-based
  models on the Tier 1 path are always cache-safe (no guardian mutation path
  exists for them).
- Added Tier1PicklingScrubTestCase to lock in the new __getstate__ contract.

* Address review #1665: forward request= through _compute_effective_permissions

The optimizer's _compute_effective_permissions invokes Document.objects.user_can /
Corpus.objects.user_can repeatedly per request. Threading request= through to those
calls activates the Tier 2 PermissionQueryOptimizer, deduplicating guardian lookups
across distinct Document/Corpus instances within a single request.

Annotation/Relationship manager user_can() overrides now forward request= as
context= to the optimizer, completing the request-scoped cache plumbing so the
kwarg doesn't silently no-op for these checks.

* Address review #1665: warn on invalidate() clear-all + assert cold-path query budget

Two follow-ups to the latest Claude review:

- PermissionQueryOptimizer.invalidate() no-args docstring now points at
  invalidate_caches() for the clear-all intent so an accidental call
  doesn't silently nuke the request cache.
- test_repeat_user_can_zero_queries wraps the warm-up call in an
  explicit assertNumQueries(6) so a regression that blows up the cold
  path is caught here instead of only the second-call zero-query assertion.

* Address review #1665: thread-safe Tier 1 cache + type annotations

- Replace the raw `dict` on `instance._oc_granted_perms_cache` with
  `_InstancePermsCache(dict)`, a thread-safe subclass exposing
  `drop_for_user(user_id)` that holds an internal Lock for the
  iterate-then-delete sweep in `set_permissions_for_obj_to_user`.
  Closes the ASGI / async-view exposure flagged in the latest review
  (the sweep would otherwise risk `RuntimeError: dictionary changed
  size during iteration` if two threads ever touched the same
  instance). Individual reads/writes still ride CPython's GIL
  atomicity; the lock is for the compound op only.
- `_get_or_create_instance_perms_cache` uses `__dict__.setdefault`
  so two threads first-touching the same instance converge on one
  cache object rather than racing via `getattr`+`setattr`.
- Backwards-compat fallback in `set_permissions_for_obj_to_user`:
  if an instance carries a plain-`dict` cache (older worker
  unpickle, test fixture), invalidation falls back to a
  snapshot-then-`.pop()` pattern so `RuntimeError` cannot escape.
- Add explicit `request: Any = None` annotations on
  `ConversationManager.user_can` / `ChatMessageManager.user_can`
  (previously inferred as `None`, which would reject real request
  objects at the type level).
- Drop rotting inline `# PR #1663` comment references per CLAUDE.md.
- New `Tier1CacheThreadSafetyTestCase` pins the contract: cache is
  the thread-safe wrapper, concurrent reader threads + repeated
  invalidate sweeps don't race, mutation invalidation routes
  through `drop_for_user` (mocked).

* ci: trigger Backend CI for 524c9e7

* Address review #1665: defer lazy import past cache hits + superuser cache caveat

Two minor polish items from the latest review:

- ``PermissionQueryOptimizer.get_granted`` previously ran the lazy
  ``from opencontractserver.utils.permissioning import …`` on every
  call. On warm requests where most calls are cache hits, that's a
  needless ``sys.modules`` lookup (~30 ns each). Move the import inside
  the two branches that actually call ``get_users_permissions_for_obj``
  (anonymous-user fallback, cache miss) so the cache-hit path stays
  pure-Python with no module table touch.
- Add a NOTE at the superuser cache-write site in
  ``get_users_permissions_for_obj`` warning that the cache key omits
  ``is_superuser``, so test scenarios that promote/demote the same
  Python instance need to drop the cache attribute manually. The same
  caveat already lived in the constants module; mirroring it at the
  write site makes the boundary visible to anyone tracing a stale-cache
  bug in tests.

Tests still pass (44 in test_permission_optimizer.py, 490 across the
full permissioning suite locally).

* Address PR #1665 review: invalidate_caches direct path, lock trade-off note, MRO comment, group-perm doc section, magic-count comment

- PermissionQueryOptimizer.invalidate_caches() now clears self._cache directly
  instead of delegating to invalidate() with no args (which it warned against).
- Added a DESIGN NOTE inside get_granted explaining why the DB call lives
  outside the lock — preempts future well-meaning serialization fixes.
- InstanceUserCanMixin.__getstate__ now documents the MRO assumption
  (Django 4.2+ Model.__getstate__ / Python 3.11+ object.__getstate__).
- test_repeat_user_can_zero_queries now flags assertNumQueries(6) as
  upgrade-sensitive with explicit guidance on bumping the count.
- query_permission_patterns.md gains an operator note enumerating the
  group-targeted guardian call sites in this repo and the manual
  invalidation contract for tenants that wire group perms themselves.

---------

Signed-off-by: JSIV <5049984+JSv4@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
JSv4 added a commit that referenced this pull request May 17, 2026
* Add extract & analyzer agent tools

Six new tools let an LLM agent discover the Fieldsets and Analyzers
visible to the calling user and dispatch them just like a human would:

- list_fieldsets / start_extract / list_recent_extracts
- list_analyzers / start_analysis / list_recent_analyses

start_extract and start_analysis are approval-gated and require WRITE on
the corpus; they reuse the same Celery pipeline (run_extract,
process_analyzer) as the existing GraphQL mutations. The agent always
picks from existing Fieldsets/Analyzers — it cannot invent new schemas.
Document scope follows the agent context (corpus agent = whole corpus,
doc agent = current doc) and requested document_ids are intersected
with corpus.get_documents() so the agent can never escape its scope.

Not added to DEFAULT_DOCUMENT_ACTION_TOOLS: opt-in only via a
CorpusAction's pre_authorized_tools.

Docs updated in docs/architecture/llms/README.md,
docs/corpus_actions/intro_to_corpus_actions.md, and
docs/extract_and_retrieval/data_extraction.md.

* Fix failing tests in PR 1675

- test_async_variant_matches: wrap sync ORM calls in sync_to_async since
  test methods are async (was raising SynchronousOnlyOperation in CI under
  pytest-xdist parallel runners).
- test_start_extract/start_analysis_requires_approval: supply all required
  kwargs since PydanticAIToolWrapper._maybe_raise uses Signature.bind (not
  bind_partial), so omitting required kwargs raised TypeError before the
  approval gate could fire.

* Apply black formatting to test_extract_analyzer_tools.py

* Address review: security, N+1, double-fetch, magic numbers, test hygiene

Discovery tools (list_fieldsets, list_analyzers, list_recent_extracts,
list_recent_analyses) now check corpus visibility via
visible_to_user(user).filter(pk=...).exists() instead of bare
Corpus.objects.get(pk=...), so a caller cannot probe private corpus IDs
through an existence oracle (review item #8). The error message is
unchanged across both branches per CLAUDE.md's IDOR guidance.

list_recent_extracts and list_recent_analyses fold the per-row M2M
count into the SELECT with annotate(Count(...)) so the document_count
field no longer issues N queries on N rows (review item #1).

start_extract and start_analysis collapse the two-step
visible_to_user().exists() + .get() pattern into a single
visible_to_user().filter(pk=...).first() lookup (review item #5).
Same fix applied to the Fieldset/Analyzer lookups in those functions.

list_fieldsets now includes fieldsets pinned to the *current* corpus
(via Q(corpus__isnull=True) | Q(corpus_id=corpus_id)) so the discovery
surface matches what start_extract actually accepts — previously a
caller listing fieldsets would not see one that start_extract would
have allowed if supplied directly (review item #7).

MAX_LIST_LIMIT / DEFAULT_LIST_LIMIT / DEFAULT_RECENT_LIMIT hoisted to
opencontractserver/constants/tools.py per CLAUDE.md's no-magic-numbers
rule (review item #4).

Test hygiene: dead GremlinEngine fixture removed from
TestListRecentAnalyses.setUp (review item #10); inspect import lifted
out of test_to_core_tool_returns_async_function loop (review item #11).

* Address Claude review on PR 1675

- Prefetch columns with is_manual_entry filter at DB level
- Pin corpus_action_id lookup to the operating corpus (security:
  prevent cross-corpus lineage attribution from direct callers)
- Move process_analyzer import to module scope (no actual circular
  dependency justifies the local import)
- Reject fieldsets whose only columns are manual-entry (LLM cannot
  extract from them)
- Drop decorative section-banner comments and explain-the-what comments
- Tests: cross-corpus action ID is dropped (extract + analysis),
  outside-corpus document_id warns and falls back to full corpus,
  manual-entry-only fieldset raises, corpus_action_id lineage on
  start_analysis

* Address review: distinguish user-not-found from unauthenticated, clamp_limit tests, comment fixes

- start_extract / start_analysis now distinguish 'no user_id supplied'
  (unauthenticated) from 'user_id refers to no row' (not found), so the
  error message no longer misattributes the failure.
- Add parametrized test for _clamp_limit covering None, 0, negative,
  oversized, non-numeric, and numeric-string inputs.
- Add test pinning the user-not-found vs unauthenticated split.
- Fix misleading _patch_process_analyzer docstring (it patches the import
  site, not the source module).
- Note the intentional transaction asymmetry between start_extract and
  start_analysis so future readers don't 'fix' it.

* Address review #1675: document edge contracts + cover zero-column discovery

extracts_and_analyzers.py:
- _resolve_target_document_ids: comment the memory trade-off for very
  large corpora and point at the SQL-pushdown alternative for future
  optimization.
- start_analysis: comment that analysis_input_data is intentionally not
  pre-validated against analyzer.input_schema — validation lives in
  run_task_name_analyzer where the payload is spread into the task
  function call (TypeError on unknown kwargs). Keeps the agent path
  bug-compatible with the human GraphQL/CorpusAction path.

test_extract_analyzer_tools.py:
- Add test_listing_shows_zero_columns_then_start_rejects pinning the
  deliberate two-step failure mode: list_fieldsets shows a
  manual-entry-only fieldset with column_count=0 (discovery surface) and
  start_extract rejects it as ValueError (dispatch surface).

* Address review #1675: wrap start_analysis in atomic + clarify LLM tool schema

- Wrap process_analyzer call in transaction.atomic() so the
  on_commit task dispatch can be rolled back if anything raises
  afterwards. The previous comment claimed process_analyzer managed
  its own transaction; it does not — it just registers on_commit
  inside whatever transaction is open. In autocommit mode (Celery
  workers) the callback would fire immediately, kicking off the
  downstream task even on an error path. start_extract already wraps
  in atomic; this aligns start_analysis.
- Lift _extract_status magic strings into
  constants/tools.py (EXTRACT_STATUS_FAILED / COMPLETED / RUNNING
  / QUEUED) and use them at every call site for parity with the human
  GraphQL surface. Analyses keep using the model-sourced
  analysis.status directly.
- Tool registry descriptions for fieldset_id / analyzer_id now say
  explicitly which type they are (integer vs CharField string) and how
  to map them from the discovery tool output, since list_fieldsets
  returns int ids and list_analyzers returns string ids and the LLM
  could otherwise conflate the two.
- start_analysis comment now documents the analysis_input_data ->
  task-function injection surface so future readers know the analyzer
  task is the validation boundary.

* Address review #1675: status vocab, payload hardening, listing UX, SQL perf

- Unify analysis status vocabulary across start_analysis and
  list_recent_analyses. New ``_normalize_analysis_status`` lowercases
  the model-sourced JobStatus value; ``_ANALYSIS_STATUS_QUEUED`` is
  what start_analysis returns now (derived from
  ``JobStatus.QUEUED.value`` rather than the Extract constant), so an
  LLM polling the listing right after dispatch sees the same string
  rather than an apparent case-shift mid-lifecycle.
- Add ``ANALYSIS_INPUT_DATA_RESERVED_KEYS`` and
  ``_strip_reserved_input_keys`` to strip framework-owned kwargs
  (``analysis_id``, ``user_id``, scoping IDs) from agent-supplied
  ``analysis_input_data`` before dispatch. Warn on strip so production
  traces can flag override attempts. Closes the unique
  prompt-injection escalation surface on the agent path; the task
  itself remains the canonical validation boundary.
- Truncate oversized ``Analyzer.input_schema`` in ``list_analyzers``
  via ``_summarise_input_schema`` + ``ANALYZER_INPUT_SCHEMA_MAX_INLINE_CHARS``
  (4_000). Replaces the schema with a placeholder when its JSON dump
  exceeds the cap so a misbehaving analyzer can't inflate every
  agent's context window.
- ``list_fieldsets`` now exposes an ``extractable`` flag and orders by
  ``-created`` to match ``list_recent_extracts``. The flag surfaces
  the listing-vs-dispatch contract for manual-only/empty fieldsets
  up-front; the ordering removes the silent position shift that
  occurred when a fieldset was edited mid-session.
- Push ``_resolve_target_document_ids`` intersection into SQL via
  ``corpus.get_documents().filter(pk__in=...)`` / ``.exists()`` so
  large corpora don't load the entire active doc ID set into a
  Python ``set`` on every dispatch.
- New tests pin each contract: status normalisation, reserved-key
  strip + warning, legitimate payload pass-through, oversized
  ``input_schema`` truncation, and the ``extractable`` flag.

* Address review #1675: move asyncio marker to method level on TestApprovalGate

Stacking ``@pytest.mark.asyncio`` on the class combined with
``TransactionTestCase`` is fragile across pytest-django versions —
pytest-asyncio tries to drive every method as a native coroutine while
``TransactionTestCase`` wants to route them through Django's own
sync/async test runner. Per the latest review, the safer pattern (and
the one used elsewhere in this codebase, e.g.
``test_conversation_search.py``) is ``@pytest.mark.django_db`` on the
class + ``@pytest.mark.asyncio`` on each async test method.

The reviewer also flagged the file's five separate
``from opencontractserver.constants.tools import …`` blocks. Confirmed
non-actionable: this project's ``isort`` settings (no
``combine_as_imports``) split ``as`` aliases into their own statements,
so consolidating fails ``pre-commit``. Leaving the structure as-is.

53 tests pass after this change.

* Address review #1675: raise on scope mismatch, slim list_fieldsets, lift coverage

- _resolve_target_document_ids now raises ValueError when a document
  agent's injected document_id is not in the corpus, instead of silently
  expanding to the full corpus scope (which a single-doc agent should
  never reach). Test updated to assert the new contract.
- list_fieldsets returns column names by default; full column
  definitions (query/match_text/output_type/instructions) gated behind
  a new include_columns=true flag. Bounds the discovery payload for
  corpora with many fieldsets of many long-form columns. Tool
  description and tests updated.
- list_analyzers ordered by -created (was order_by("id"), lexicographic
  over CharField IDs and unpredictable as new analyzers are registered).
- start_extract / start_analysis descriptions now warn that omitting
  document_ids on a corpus agent dispatches against the whole corpus,
  so admins working over very large corpora know to bound it.
- Added tests: limit pagination clamp end-to-end for list_fieldsets
  and list_analyzers; all four _extract_status branches (queued /
  running / completed / failed) in list_recent_extracts.
- Removed redundant @pytest.mark.django_db on TestExtractAnalyzerRegistryIntegration
  (TransactionTestCase already provides DB isolation).
- CHANGELOG: rewrote the "extract/analyzer review follow-ups" entry to
  describe the behaviour changes rather than the review process.

* Address review #1675: lift CRUD grant into the analyzer/extract framework

Mirrors the existing create_and_setup_analysis chokepoint with a new
create_and_setup_extract helper so every Extract created through a
user-attributable flow gets guardian CRUD on the creator without the
implementer having to remember. start_extract (agent tool) and
process_corpus_action's get_or_create path now route through (or
inline) the framework grant. Docstrings on both helpers pin the
contract so a future refactor can't quietly drop it.

Other review follow-ups in the same sweep:
- Add `created` to list_recent_analyses output so an LLM polling
  right after start_analysis sees a timestamp instead of three nulls.
- list_fieldsets builds rows with stable key order (common fields
  first, payload last) for readability.
- Consolidate redundant `from opencontractserver.constants.tools`
  blocks.
- Clarify list_analyzers description: the corpus_id is a visibility
  check, not a corpus-affinity filter.
- Update include_columns description to list extract_is_list.
- _get_user_or_none gets a docstring explaining why the return type
  stays inferred (User = get_user_model() is a runtime variable, not
  a valid mypy type).

Tests:
- test_create_and_setup_analysis_grants_creator_crud — pins the
  Analysis chokepoint's CRUD contract.
- test_extract_utils.py — full coverage for the new Extract helper
  (creation, CRUD grant, document linking, started timestamp,
  corpus_action lineage, name defaulting).

Closes #1674

---------

Signed-off-by: JSIV <5049984+JSv4@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
JSv4 added a commit that referenced this pull request May 25, 2026
…Markup note

Addresses the latest claude-bot review on PR #1781:

#1 — overflowMenuItems.ts audit drift fixed
- Earlier commit added "About cite → /about" to the audit comment but
  did not add the link to OVERFLOW_MENU_LINKS, leaving the comment
  misleading. Add the About entry to the array so the always-on
  overflow stays in parity with the Footer (essential links remain
  reachable from long-scroll views where the in-flow Footer is
  unreachable).
- tests/NavMenu.ct.tsx: assert the About menuitem in both desktop
  + mobile, anonymous + authenticated. Rewrite the arrow-key wrap
  tests to walk every item dynamically so future audit additions or
  removals don't require touching them again.

#3 — renderInlineMarkup index-as-key annotated
- The reviewer flagged the bare index key as fragile. Reality: the
  segment array is the deterministic output of `String.split(regex)`
  on static JSON content, so positions never reorder. Added a
  comment explaining why composite keys would be a stable-for-stable
  swap rather than an improvement.

#4 + #5 — magic colour / font-family literals extracted to OS_LEGAL_*
- OS_LEGAL_COLORS gains the cite-brand neutrals: `ink` (#0f172a, the
  navy primary button background), `inkHover` (#1e293b, paired hover
  state — same value as textPrimary but kept as a semantic alias so
  future divergence is cheap), `warmPaper` (#FAFAF7, brand page
  background). Each carries a docstring explaining intent vs raw
  slate-scale position.
- OS_LEGAL_TYPOGRAPHY.fontFamilySerif upgraded from the legacy
  Georgia-first stack to the cite Source Serif 4 / Source Serif Pro
  chain (Georgia stays in the fallback list — strictly additive, no
  regression for existing serif consumers). This is the reviewer's
  suggested fix path: update the constant once, not the call sites
  six times.
- Refactored every touched component to reference the tokens
  instead of inlining the values:
    CallToAction.tsx (2 colors, 2 serif stacks)
    CookieConsent.tsx (6 warmPaper, 2 ink + 2 inkHover, 1 accent,
      1 sans stack, 2 serif stacks)
    Footer.tsx (1 serif stack)
    Login.tsx (1 accent, 1 ink, 1 inkHover, 1 warmPaper)
    NavMenu.tsx (CiteMark bracketColor + nodeColor via tokens,
      navbar brand-name override via fontFamilySerif)
    NewHeroSection.tsx (2 serif stacks)
    About.tsx (4 serif stacks)
    CiteWordmark.tsx (variant fill ternary now uses textPrimary /
      warmPaper; inline SVG text style uses fontFamilySerif)
    renderInlineMarkup.tsx (1 serif stack)

- CiteWordmark.test.tsx: compare rendered fill against
  OS_LEGAL_COLORS.{textPrimary, warmPaper} rather than typed hex
  literals so brand-palette edits never require a parallel test edit.

#6 (font dedup) and #7 (CiteMark decorative aria) noted only — the
sans fallback chain was retained verbatim per the CookieConsent
Windows-rendering case, and the empty-ariaLabel → aria-hidden path
is already correct (covered by an existing unit test).

PR description updated separately to keep VERSION_TAG = v3.0.0.rc1
and reframe this PR as the rebrand release-candidate (not the GA
cut) — matches the constants.ts comment and the releaseScreenshot
calls that lock the v3.0.0.rc1 surface in amber.
JSv4 added a commit that referenced this pull request May 25, 2026
…1781)

* brand(v3): rebrand to [cite] — landing, about, and chrome wordmark

Brings in the cite brand system (handoff in /home/jman/Downloads/
cite-rebrand-handoff): the project ships v3 as cite — the citation
layer underneath the public record.

Scope is the public surface — assets, landing, About, and the
wordmark/favicon/meta that flow from them. The deeper product UI
chrome and the wider OpenContracts naming inside the code stay
untouched in this pass.

Changes:
- Add CiteMark + CiteWordmark React components (inline SVG, font
  fallback chain) plus the source SVGs under src/assets/brand/.
- Swap PNG favicon for SVG icon mark (favicon.svg, favicon-32.svg,
  favicon-16.svg) — modern browsers pick up SVG, .ico kept as
  alternate fallback. Update manifest.json + index.html meta and
  JSON-LD to use the cite name, taglines, and cite.opensource.legal
  URL. Switch theme color to warm paper #FAFAF7.
- Pull Source Serif 4 in alongside Inter from Google Fonts.
- Bump VERSION_TAG from v3.0.0.rc1 → v3.0.0 (rebrand is the v3 GA
  milestone per migration plan).
- DiscoveryLanding hero: rewrite as "[•] The citation layer /
  underneath the public record." with Source Serif headline, teal
  second line, and the cite mark at cap-height. Subhead, search
  placeholder, GetStarted action list, and StatsSection labels
  follow the home_page.md copy.
- CallToAction: replace the gradient/Rocket "Ready to dive in?"
  block with an editorial-voice About-cite paragraph and a quiet
  sign-in / read-more pair, per the brand voice rules.
- /about: new route + page rendering the four-section long-form copy
  from about.md (Why cite exists / Why it's broken / What cite is /
  Why we think we can do this). Linked from the new footer.
- NavMenu: switch the os_legal_128 PNG logo for the inline cite icon
  mark; restyle the brand-name slot to Source Serif 4 at 22px with
  -0.5px tracking so [cite] reads as a typographic wordmark instead
  of a UI label.
- Footer: replace the os_legal_128 PNG with the stacked
  opensource.legal · [cite] lockup; tighten the copy and add About
  + GitHub links.
- Login: drop the legacy PNG/turquoise theme for the cite icon mark
  + Source Serif title; navy primary button.
- Component tests: update NavMenu Branding and CallToAction
  assertions to the new wordmark / copy. Refresh the
  landing--call-to-action--anonymous doc screenshot.

Verification: tsc --noEmit + vite build clean; targeted Playwright
component tests pass (CallToAction Component x2, NewHeroSection x2,
NavMenu Branding x3).

Closes #N/A (rebrand prep).

* docs(cite): README rebrand, new About/Login CT screenshots, RC1 version

- README.md: rewrite around the cite framing while keeping the
  GitHub repo name OpenContracts (per migration plan — repo rename
  is a coordinated cut after launch). Lead with the opensource.legal
  · [cite] lockup, restate the "why cite / why broken / what cite
  is" arc, add a domain-migration table, and fix the cite product
  tagline throughout. Add docs/assets/images/brand/ with the
  wordmark, lockup, icon mark, and inline marker SVGs so the README
  and other docs can reference the production marks.

- v3.0.0.rc1: revert VERSION_TAG from v3.0.0 back to v3.0.0.rc1 —
  the rebrand ships as RC1, GA drops the suffix. Bump the release
  screenshot key to v3.0.0.rc1 to match. Release-screenshot regex
  (^v\d+\.\d+\.\d+) still accepts the suffixed form.

- DiscoveryLanding CT: replace the now-stale "The open platform for"
  assertion with the new cite hero text + capture
  landing--discovery-page--anonymous.png (regenerated) and the
  v3.0.0.rc1 release landing-page snapshot.

- New CT tests + doc screenshots:
    * NewHeroSection close-up
      → landing--hero--cite-headline.png
    * GetStarted card with cite [•] bullets
      → landing--get-started--cite-bullets.png
    * Full About page (asserts headings + named incumbents)
      → about--full-page--anonymous.png
      → releases/v3.0.0.rc1/about-page.png
    * Login card (cite mark + wordmark + tagline)
      → login--cite-brand--anonymous.png
      → releases/v3.0.0.rc1/login-page.png

Verification: tsc --noEmit clean; 17/17 rebrand-related Playwright
component tests pass (CompactLeaderboard ×3, CallToAction ×2,
DiscoveryLanding ×1, FeaturedCollections ×3, NewHeroSection ×3,
GetStarted ×2, About ×2, Login ×1).

* brand(cite): rebrand CookieConsent modal — beautiful on mobile

Every information element from the OpenContracts-era cookie modal is
preserved verbatim (demo-system caveat, cookie-usage explainer, data
we collect, data you agree to share, analytics breakdown when PostHog
is configured, MIT-style warranty disclaimer). The visual language is
rewritten against the cite brand system.

Brand alignment:
- Source Serif 4 modal title ("Cookies and terms" — sentence case)
  with a small [•] PRIVACY eyebrow above it that puts the brand atom
  on the surface that introduces the project.
- Warm paper #FAFAF7 throughout (header, body, footer) instead of
  pure white; thin slate borders define structure.
- Demo banner: drops the warning-yellow #fefce8/#854d0e palette
  (off-brand — teal is the only saturated color) for a quiet
  slate-on-paper card with a 3px teal left stripe and slate
  AlertTriangle icon. Same information, matter-of-fact voice.
- Lead paragraph set in Source Serif with the word *cite*
  italicized, per the brand voice rule.
- Section eyebrows in muted slate, 10px / 0.6px tracking, sentence
  case ("Cookie usage", "Data we collect", "Data you agree to
  share") — within the brand's "ALL CAPS only in eyebrow labels"
  carve-out.
- Cards: white, 1px slate border, 6px radius, no shadow on desktop
  (brand: shadows subtle to absent). On mobile, list rows pick up a
  surfaceLight tint so each item stays scannable.
- Disclaimer: ALL CAPS warranty text is preserved (it's a legal
  convention, not marketing) but quieted to 12px muted slate inside
  a brand-correct white card with its own "Warranty disclaimer"
  eyebrow.
- Primary CTA: navy chrome #0F172A button — brand spec puts navy on
  primary buttons and reserves teal for accent / links / active
  states. Full-width 48px touch target on mobile.

Implementation note: the @os-legal/ui Modal renders via
createPortal(content, document.body), so a styled-components wrapper
around <Modal> can't reach the portaled DOM with descendant
selectors. Replaced StyledModalWrapper with an inline <style> tag
plus className="cookie-consent-modal" /
overlayClassName="cookie-consent-overlay" props on <Modal> — same
pattern NavMenu uses for its .oc-navbar overrides. This is what
finally lets the navy button override and Source Serif title
actually paint.

Tests + screenshots:
- CookieConsent.ct.tsx updated for the new copy ("Cookies and
  terms", "Accept and continue", "Cookie usage", etc.) and the
  added "Warranty disclaimer" eyebrow. Both desktop and mobile
  variants now also capture a v3.0.0.rc1 release screenshot.
- cookie-consent--modal--default.png and
  cookie-consent--modal--mobile.png regenerated.
- New release screenshots: releases/v3.0.0.rc1/
    cookie-consent-modal.png (desktop)
    cookie-consent-modal-mobile.png (iPhone 13 viewport)

Verification: tsc --noEmit clean; 2/2 CookieConsent CT tests pass.

* config(landing): JSON-drive Discover + About; ship default and public-record variants

The Discover landing page and the /about long-form page are now driven
by a typed JSON content pack. Two variants ship in the repo, switchable
at runtime via REACT_APP_LANDING_VARIANT:

  - default          → "The citation layer for agentic workflows."
                       World-facing pitch aimed at developers, AI labs,
                       research teams, and anyone landing on the repo
                       or self-hosting cite against their own corpus.
                       This is the repo default and the framing used in
                       README, index.html OG tags, and the bundled
                       Discover/About screenshots.

  - public-record    → "The citation layer underneath the public record."
                       End-user pitch with the named-incumbent argument
                       (Westlaw, Lexis, JSTOR, USPTO, Wheaton v. Peters,
                       Public.Resource.Org, Free Law Project). Lands well
                       on cite.opensource.legal where the audience is
                       end-users of a specific corpus of public-domain
                       documents.

A deployer flips between them by setting REACT_APP_LANDING_VARIANT in
frontend/public/env-config.js — no rebuild, no fork. Unknown variants
fall back to default rather than crashing. Adding a new pitch is a
three-step process documented in README: drop a JSON file in
src/config/landingContent/, register it in index.ts, set the env var.

Architecture:
- src/config/landingContent/
    types.ts            — typed schema (HeroContent, StatConfig,
                          GetStartedContent, CallToActionContent,
                          AboutContent + footer links). Stat keys are
                          constrained to a CommunityStatKey union so a
                          typo in a variant fails at build.
    default.json        — world-facing pitch.
    publicRecord.json   — JSv4's deployment pitch.
    index.ts            — variant registry + useLandingContent() hook
                          backed by useEnv().REACT_APP_LANDING_VARIANT
                          with useMemo memoization.
    renderInlineMarkup  — tiny inline-italic renderer that wraps `*…*`
                          spans in Source Serif italic <em>. The brand
                          rule restricts italic to (a) the cite product
                          name and (b) publication/platform names in
                          body copy — both are flat inline spans, so a
                          full Markdown parser would be overkill.
    __tests__/          — vitest checks both bundled variants pass the
                          type contract, diverge on hero copy (guards
                          against a copy-paste collapse), and reference
                          only stable GraphQL stat keys.

Component refactor:
- NewHeroSection, StatsSection, GetStarted, CallToAction, About all
  read from useLandingContent() and stop hard-coding copy. The
  components stayed visually identical — same layout, same brand
  treatment — only the strings/lists are now external. Italics inside
  body copy route through renderInlineMarkup so the *cite* product
  name picks up its Source Serif italic treatment automatically.

Tests:
- landing-components.ct.tsx and About.ct.tsx import the same JSON the
  components do and assert against `defaultLandingContent.hero.primary`,
  `…callToAction.headline`, every Get Started action label, every
  About section heading, etc. Future copy edits to the default JSON
  keep the tests green automatically.

Screenshots:
- landing--discovery-page--anonymous.png, landing--hero--cite-headline.png,
  landing--call-to-action--anonymous.png, about--full-page--anonymous.png
  regenerated to show the world-facing pitch (the repo default).
- releases/v3.0.0.rc1/landing-page.png and about-page.png replaced
  (write-once was holding the old public-record framing — rc1 ships
  the world-facing default, so the snapshots needed to match).

Docs + meta:
- README rewritten with the "Why cite" arc in the agentic-workflows
  voice. New "Customizing the landing and About copy" section
  documents the variant mechanism and the three-step recipe for
  adding deployment-specific pitches.
- index.html <title>, OG tags, and JSON-LD updated to the repo
  default ("citation layer for agentic workflows"). JSv4's
  deployment will need to swap these too on the production build
  (separate concern — meta is build-time-baked, not runtime).
- EnvConfig gains REACT_APP_LANDING_VARIANT (default "default") so
  the type system enforces the contract for deployers reading from
  window._env_.

Verification: tsc --noEmit clean; 6/6 new vitest variant checks pass;
17/17 Playwright CT tests pass (CompactLeaderboard ×3, CallToAction
×2, DiscoveryLanding ×1, FeaturedCollections ×3, NewHeroSection ×3,
GetStarted ×2, About ×2, Login ×1).

* copy(landing): emphasize human/agent ground-truth collaboration; speak to bots too

Tweaks the default (world-facing) landing + About content pack so the
human/agent collaboration loop is the lede, not a subordinate clause —
and so an LLM crawling the surfaces actually finds the affordances it
needs.

About page:
- Title: "The citation layer that agents and humans can both read."
- Lede sharpened around the ground-truth framing: "Humans curate the
  ground truth, agents reason over it, and the graph compounds."
- "What cite is" extended with the protocol-surface line that answers
  "how do I use this?" in one breath: "Same graph, two interfaces: a
  GraphQL and REST API for humans and applications, a Model Context
  Protocol endpoint for agents. The substrate is identical; only the
  surface you call differs."
- New "How the graph gets built" section between "What cite is" and
  "Who it's for" — describes the three-step loop concretely:
    Humans curate. → Agents reason. → Humans review. → (compound)
  Closing fourth paragraph addresses an LLM reader directly so the
  capability information survives chunking and shows up cleanly in
  retrieval: "If you are an LLM-based agent reading this: the corpus
  is exposed to you over Model Context Protocol at /mcp/. You can list
  documents, search annotations, follow citation edges across the
  graph, and propose new edges by writing annotations of your own.
  Cite the document and span you pulled from in every answer …"

Hero subhead:
- Verbs sharpened from abstract ("humans curate / agents reason over")
  to concrete agent-meaningful action ("humans annotate, agents
  traverse, both propose new edges"). One ground truth, two
  collaborators — the framing humans appreciate; verbs an agent's
  planner can match against its action space.

CTA body:
- Closing line: "Humans and agents work the same graph; the ground
  truth is shared."

README:
- "Why cite" arc mirrors the in-product framing: same graph / two
  interfaces / agents propose, humans review / graph compounds.
- New direct-address blockquote at the end of the section for LLMs
  parsing the README, naming /mcp/, /llms.txt, and
  /.well-known/mcp.json so a crawler finds the discovery surfaces in
  one hop.

Screenshots:
- landing--discovery-page--anonymous.png, landing--hero--cite-headline.png,
  landing--call-to-action--anonymous.png, about--full-page--anonymous.png
  regenerated for the sharpened copy.
- releases/v3.0.0.rc1/landing-page.png + about-page.png replaced
  (write-once was holding the prior wording — rc1 ships this copy).

Verification: tsc --noEmit clean; 6/6 vitest variant checks pass;
16/16 Playwright CT tests in the rebrand suite pass.

* test: cover cite-brand components and Footer for codecov patch target

The PR introduced CiteMark, CiteWordmark, renderInlineMarkup, and the
new Footer lockup with no direct test coverage, dragging codecov/patch
to 79.27% (auto target). Add focused unit + CT tests:

- src/components/brand/__tests__/CiteMark.test.tsx — strokeFor branches
  for all four size bands, prop pass-through, aria semantics.
- src/components/brand/__tests__/CiteWordmark.test.tsx — both variants,
  default sizing, prop pass-through.
- src/config/landingContent/__tests__/renderInlineMarkup.test.tsx —
  empty, plain, single italic, multi-italic interleaved with text.
- tests/Footer.ct.tsx + tests/FooterTestWrapper.tsx — three viewport
  breakpoints (>1000px, compact, <400px) exercising the $compact and
  $small ternaries plus two new docScreenshots
  (layout--footer--full-width, layout--footer--compact).

Test wrapper split is required because Playwright CT cannot mount
components defined inside the test file (per CLAUDE.md split-import
rule). MemoryRouter wrap is needed because the CT root provides
Jotai/Apollo/Theme but no router context.

* review: address PR #1781 feedback (broken /contact, AGPL→MIT, a11y, dead code)

Addresses 10 of the issues raised across the three claude-bot reviews:

Bugs / broken refs
- Footer.tsx: drop the broken /contact Link (no route registered in
  App.tsx). overflowMenuItems.ts audit comment refreshed to reflect
  that the Footer now matches the overflow's omission.
- index.html: JSON-LD license fields flipped from AGPL to MIT to match
  the README + CookieConsent copy.
- index.html: og:title now reads "[cite]" with brackets to match the
  brand spec (the wordmark IS the brackets).
- index.html: SVG <link rel="icon"> entries switched from pixel
  sizes="32x32" etc. to sizes="any" (the spec-correct value for
  scalable formats).
- index.html: dead apple-touch-icon link to the no-longer-shipped
  /logo192.png removed; comment notes brand-correct PWA icons are
  intentionally deferred.

Accessibility
- CiteMark.tsx: when ariaLabel is empty (decorative usage in
  GetStarted, CallToAction, About eyebrow, CookieConsent), the SVG
  now renders aria-hidden="true" and drops role="img" so screen
  readers skip it instead of announcing it as an unlabelled image.
  New unit test pins the behavior.

Dead code / variant coupling
- Login.tsx: replace the hardcoded styled <Title>[cite]</Title> +
  variant-specific subtitle with the existing CiteWordmark component
  + variant-neutral "Sign in to continue." copy. This both wires up
  the previously-unused CiteWordmark and stops the Login screen from
  shipping public-record copy regardless of REACT_APP_LANDING_VARIANT.
- src/assets/images/os_legal_128*.png: delete three unused old-brand
  PNGs (no remaining imports in src/).

Test updates
- Login.ct.tsx: assert against the new CiteWordmark SVG + neutral
  subtitle instead of the removed <Title> + public-record tagline.

Docs
- CHANGELOG.md: add an "Added" entry under [Unreleased] for the v3
  cite surface rebrand, including the explicit out-of-scope list
  (codebase-wide string scrub, PWA install icons, OG image refresh).

* brand: generate PWA + OG PNG assets for the cite rebrand

The v3 rebrand stripped the old logo192.png / logo512.png from the
public dir without replacements; the resulting manifest only carried
the SVG favicon, which is not reliably accepted by Chrome for Android's
"Add to Home Screen" install criteria, and the OG/Twitter card still
linked to the OpenContracts product screenshot.

- frontend/scripts/generate-brand-pngs.js (new): renders the cite
  icon mark SVG through headless Chromium at the exact target
  pixel dimensions. Chosen over librsvg / sharp / inkscape because
  Playwright is already a dev dependency for the CT suite — no new
  install footprint. Outputs four files:
    public/cite-192.png        (192×192, transparent, purpose: any)
    public/cite-512.png        (512×512, transparent, purpose: any)
    public/cite-maskable.png   (512×512, brand bg, mark inside the
                                central 80% safe area for Android
                                adaptive-icon shape masking)
    public/OpenContractsScreenshot.png  (1200×630 OG card with the
                                cite wordmark + public-record
                                tagline + opensource.legal eyebrow;
                                legacy filename retained so the
                                existing <meta og:image> reference
                                resolves to the new card)

- frontend/public/manifest.json: add the three PNG icon entries
  alongside the existing SVG and ICO, with the correct purpose:any
  / purpose:maskable split.

- frontend/index.html: re-introduce <link rel="apple-touch-icon">
  pointing to cite-192.png so iOS picks up a brand-correct home
  screen icon (iOS does not reliably support SVG apple-touch-icon).
  Replace the deferred-PWA-icons comment with one that points readers
  at the generator script.

- CHANGELOG.md: drop the deferred-PWA-icons / OG image bullets from
  the "Out of scope" list and call out the generator + new assets
  under a dedicated "PWA + OG assets" bullet under the rebrand entry.

* review: extract cite-brand tokens + fix overflow audit + renderInlineMarkup note

Addresses the latest claude-bot review on PR #1781:

#1 — overflowMenuItems.ts audit drift fixed
- Earlier commit added "About cite → /about" to the audit comment but
  did not add the link to OVERFLOW_MENU_LINKS, leaving the comment
  misleading. Add the About entry to the array so the always-on
  overflow stays in parity with the Footer (essential links remain
  reachable from long-scroll views where the in-flow Footer is
  unreachable).
- tests/NavMenu.ct.tsx: assert the About menuitem in both desktop
  + mobile, anonymous + authenticated. Rewrite the arrow-key wrap
  tests to walk every item dynamically so future audit additions or
  removals don't require touching them again.

#3 — renderInlineMarkup index-as-key annotated
- The reviewer flagged the bare index key as fragile. Reality: the
  segment array is the deterministic output of `String.split(regex)`
  on static JSON content, so positions never reorder. Added a
  comment explaining why composite keys would be a stable-for-stable
  swap rather than an improvement.

#4 + #5 — magic colour / font-family literals extracted to OS_LEGAL_*
- OS_LEGAL_COLORS gains the cite-brand neutrals: `ink` (#0f172a, the
  navy primary button background), `inkHover` (#1e293b, paired hover
  state — same value as textPrimary but kept as a semantic alias so
  future divergence is cheap), `warmPaper` (#FAFAF7, brand page
  background). Each carries a docstring explaining intent vs raw
  slate-scale position.
- OS_LEGAL_TYPOGRAPHY.fontFamilySerif upgraded from the legacy
  Georgia-first stack to the cite Source Serif 4 / Source Serif Pro
  chain (Georgia stays in the fallback list — strictly additive, no
  regression for existing serif consumers). This is the reviewer's
  suggested fix path: update the constant once, not the call sites
  six times.
- Refactored every touched component to reference the tokens
  instead of inlining the values:
    CallToAction.tsx (2 colors, 2 serif stacks)
    CookieConsent.tsx (6 warmPaper, 2 ink + 2 inkHover, 1 accent,
      1 sans stack, 2 serif stacks)
    Footer.tsx (1 serif stack)
    Login.tsx (1 accent, 1 ink, 1 inkHover, 1 warmPaper)
    NavMenu.tsx (CiteMark bracketColor + nodeColor via tokens,
      navbar brand-name override via fontFamilySerif)
    NewHeroSection.tsx (2 serif stacks)
    About.tsx (4 serif stacks)
    CiteWordmark.tsx (variant fill ternary now uses textPrimary /
      warmPaper; inline SVG text style uses fontFamilySerif)
    renderInlineMarkup.tsx (1 serif stack)

- CiteWordmark.test.tsx: compare rendered fill against
  OS_LEGAL_COLORS.{textPrimary, warmPaper} rather than typed hex
  literals so brand-palette edits never require a parallel test edit.

#6 (font dedup) and #7 (CiteMark decorative aria) noted only — the
sans fallback chain was retained verbatim per the CookieConsent
Windows-rendering case, and the empty-ariaLabel → aria-hidden path
is already correct (covered by an existing unit test).

PR description updated separately to keep VERSION_TAG = v3.0.0.rc1
and reframe this PR as the rebrand release-candidate (not the GA
cut) — matches the constants.ts comment and the releaseScreenshot
calls that lock the v3.0.0.rc1 surface in amber.

* review: address PR #1781 polish — a11y, keys, memoization, variant tests

About.tsx
  - composite-index keys for sections + paragraphs (no longer rely on
    section.title uniqueness)
  - aria-label on external footer links so screen readers announce the
    new-tab navigation

CookieConsent.tsx
  - DemoBanner gets role="note" + aria-hidden on the decorative icon
    (role="alert" would double-announce on top of the dialog open event)
  - corrected the CSS-scoping comment block — selectors are qualified by
    cookie-consent-modal / cookie-consent-overlay classes, not the stale
    data-cookie-consent attribute that was never wired up

NewHeroSection.tsx
  - hardcoded 1023px / 767px breakpoints replaced with the shared
    TABLET_LANDSCAPE_BREAKPOINT / TABLET_BREAKPOINT constants

CiteMark.tsx, CiteWordmark.tsx
  - wrap in React.memo so the chrome (NavMenu, Footer, About, Login, CTA)
    doesn't re-render SVGs on unrelated parent state changes

renderInlineMarkup.tsx
  - reframe the index-as-key comment around the real invariant (static
    immutable JSON content) instead of "split() is deterministic"
  - re-export renderInlineMarkup from landingContent/index.ts so callers
    can import from the package root

generate-brand-pngs.js
  - hoist BRAND_COLORS, MASKABLE_FRAME, MASKABLE_SAFE_AREA, OG_WIDTH,
    OG_HEIGHT to the top so a future brand-color or spec change is a
    one-line edit

useLandingContent.test.tsx (new)
  - direct hook coverage: default-variant fallback when env is unset,
    public-record selection, variant switching diverges on hero/about
    copy, unknown-key falls back to default

* test(useLandingContent): mock useEnv instead of (window as any)._env_

The previous version cast through `any` to satisfy the strict ambient
`Window._env_` type. That breached the project's any-baseline gate
(scripts/check-any-baseline.js) by +7. Switch the test to vi.mock the
`useEnv` hook directly — same coverage, no `any` cost, no global
window manipulation. Mirrors the pattern in utils/__tests__/analytics.test.ts.

* style: prettier-format useLandingContent.test.tsx

---------

Signed-off-by: JSIV <5049984+JSv4@users.noreply.github.com>
JSv4 added a commit that referenced this pull request May 25, 2026
- Replace hardcoded ``z-index: 2`` in NativeLinkLayer with new
  ``Z_INDEX.PDF_NATIVE_LINK_LAYER`` constant + sibling
  ``PDF_SELECTION_LAYER`` entry, removing the magic-number rule violation
  flagged in review.
- Switch ``NativeLinkLayer`` from ``React.FC`` (which the project's
  ``"jsx": "react-jsx"`` transform leaves out of scope unless React is
  explicitly imported) to a named ``FC`` import from "react" — same
  pattern recommended by review.
- Drop the ``as any`` cast in NativeLinkLayer.test.tsx for a proper
  ``as unknown as PDFPageProxy`` so the any-baseline lint check is back
  to clean.
- Slim down the CHANGELOG entry per review minor #4 — the existing entry
  doubled as a design doc; the format guidance asks for a short summary
  with file paths + a coverage pointer. Also drop the duplicate
  "Discovery endpoints rebranded" entry that local-main drift surfaced
  here (PR #1785 already owns that entry on main).
JSv4 added a commit that referenced this pull request May 25, 2026
…on bug

Addresses Claude review on #1790 (plus restoring the back button that the
PR accidentally hid):

Critical (back button gone for sidebar discussion view):
- ThreadDetail dropped its compact BackButton whenever ``customOnBack``
  was passed. But ``customOnBack`` is the back-navigation HANDLER the
  parent supplies, not a signal that the parent renders its own button.
  ``ThreadDetailWithContext`` delegates click handling without showing
  its own back affordance, so the new gate stranded the user with no
  way to return to the list — and broke the threads-discussions.spec.ts
  e2e flow (the "Back to discussions" button locator timed out after
  3 retries). Reverted to ``compact &&`` so the button is always shown
  in compact mode; the handler still respects customOnBack.

CLAUDE.md rule #4 (no magic numbers):
- MessageBadges.tsx hoisted TOOLTIP_HEIGHT_EST + TOOLTIP_MAX_WIDTH to
  module scope, swapped its local ``GAP = 8`` and ``VIEWPORT_PADDING = 8``
  for the existing shared ``POPOVER_GAP`` and
  ``CONTEXT_MENU_VIEWPORT_PADDING`` from
  ``frontend/src/assets/configurations/constants.ts``.

Stacking-context bug:
- TooltipPopup z-index was 1000 — below ModernContextMenu (10000) and
  the canonical POPOVER_Z_INDEX (100002). Switched to POPOVER_Z_INDEX
  so the portalled tooltip outranks context menus and modal overlays.

Stale-read race:
- The JSX was computing ``bottom: window.innerHeight - tooltipPos.top``
  at render time, racing with a resize that could fire between
  computePosition and the next paint (1-frame misalignment). Capture
  both ``top`` and ``bottom`` inside the effect and read them straight
  through in the JSX.

Orphan tooltip:
- When the badge scrolled fully out of the viewport the tooltip stayed
  fixed at its last computed coords. ``computePosition`` now early-
  returns + clears state when ``rect.bottom < 0 || rect.top >
  window.innerHeight``.

Test fixture typo:
- "What de y'all think?" -> "What do y'all think?" in BadgeClipHarness
  and ThreadDetailSidebarLayout.ct (both fixture and assertion).

Verified:
- yarn tsc --noEmit: clean
- yarn lint: clean
- 8/8 CT (ThreadDetail* + DocumentDiscussionsContent + BadgeClip)
JSv4 added a commit that referenced this pull request May 25, 2026
…#1790)

* sidebar discussion view: portal badge tooltip + rework cramped header

The badge tooltip in the sidebar discussion view was clipped by the
overflow:hidden chain (ContextSidebarContainer / DocumentDiscussionsContent
Container / FlexColumnPanel). Render TooltipPopup via createPortal to
document.body with fixed coords derived from the badge rect; viewport-edge
clamping plus a flip-below-the-badge fallback prevents the same clip near
the top of any scroll container. Arrow stays anchored under the badge
centre via a --arrow-offset CSS variable.

While in the same view, the ThreadDetail compact header packed back-arrow
+ type badge + title + description + document link + status pills into one
flex-wrap row, and a duplicate compact back button shadowed the parent's
"Back to List". Reworked so each piece gets its own line (title row /
description / context link / meta row separated by a dashed border) and
the redundant back button is suppressed when customOnBack is provided.

Added Playwright CT coverage (ThreadDetailSidebarLayout.ct.tsx +
harnesses) that pins both the new header layout and the no-clip tooltip
behaviour, with auto screenshots for the docs surface.

* Address review feedback (#1790)

- MessageBadges tooltip: re-position on capture-phase scroll + resize
  while the tooltip is visible.  ``position: fixed`` doesn't follow
  the badge when an ancestor scroll container moves under it; without
  this listener the tooltip drifts to the wrong place when the
  sidebar scrolls while the cursor is held still over a badge
  (review item #1).
- Replace the ``["--arrow-offset" as any]`` style cast with a
  ``Record<string, string>`` spread so the any-baseline gate stays
  honest (was the new offender that pushed current=438 baseline=437).
- ThreadDetailSidebarLayout test: assert structurally that the
  tooltip is NOT a descendant of the clip-frame
  (``el.closest('[data-testid=clip-frame]') === null``).  The
  previous "nonzero width/height" assertion could pass even if the
  portal failed to escape the clip frame (review item: test).

* Address PR review on #1790: constants, z-index, stale read, back-button bug

Addresses Claude review on #1790 (plus restoring the back button that the
PR accidentally hid):

Critical (back button gone for sidebar discussion view):
- ThreadDetail dropped its compact BackButton whenever ``customOnBack``
  was passed. But ``customOnBack`` is the back-navigation HANDLER the
  parent supplies, not a signal that the parent renders its own button.
  ``ThreadDetailWithContext`` delegates click handling without showing
  its own back affordance, so the new gate stranded the user with no
  way to return to the list — and broke the threads-discussions.spec.ts
  e2e flow (the "Back to discussions" button locator timed out after
  3 retries). Reverted to ``compact &&`` so the button is always shown
  in compact mode; the handler still respects customOnBack.

CLAUDE.md rule #4 (no magic numbers):
- MessageBadges.tsx hoisted TOOLTIP_HEIGHT_EST + TOOLTIP_MAX_WIDTH to
  module scope, swapped its local ``GAP = 8`` and ``VIEWPORT_PADDING = 8``
  for the existing shared ``POPOVER_GAP`` and
  ``CONTEXT_MENU_VIEWPORT_PADDING`` from
  ``frontend/src/assets/configurations/constants.ts``.

Stacking-context bug:
- TooltipPopup z-index was 1000 — below ModernContextMenu (10000) and
  the canonical POPOVER_Z_INDEX (100002). Switched to POPOVER_Z_INDEX
  so the portalled tooltip outranks context menus and modal overlays.

Stale-read race:
- The JSX was computing ``bottom: window.innerHeight - tooltipPos.top``
  at render time, racing with a resize that could fire between
  computePosition and the next paint (1-frame misalignment). Capture
  both ``top`` and ``bottom`` inside the effect and read them straight
  through in the JSX.

Orphan tooltip:
- When the badge scrolled fully out of the viewport the tooltip stayed
  fixed at its last computed coords. ``computePosition`` now early-
  returns + clears state when ``rect.bottom < 0 || rect.top >
  window.innerHeight``.

Test fixture typo:
- "What de y'all think?" -> "What do y'all think?" in BadgeClipHarness
  and ThreadDetailSidebarLayout.ct (both fixture and assertion).

Verified:
- yarn tsc --noEmit: clean
- yarn lint: clean
- 8/8 CT (ThreadDetail* + DocumentDiscussionsContent + BadgeClip)

---------

Co-authored-by: Claude <noreply@anthropic.com>
JSv4 added a commit that referenced this pull request May 25, 2026
* Make native PDF link annotations clickable in the viewer

Embedded hyperlinks in PDFs were unreachable because the SelectionLayer
overlay sits at z-index 1 across the entire page and intercepts every
mouse event. PDF.js painted the link rects into the canvas as pixels,
but page.getAnnotations() was never called, so no DOM target existed
to receive the click.

New NativeLinkLayer extracts Link annotations from each page, filters
to safe protocols (http/https/mailto), projects each rect through the
viewport, and renders transparent <a> elements at z-index 2 (above
SelectionLayer, below the SelectionActionMenu portal). The overlay
container is pointer-events: none so empty space still falls through
to the selection logic; each anchor is pointer-events: auto so the
browser handles the click (with hover state, right-click menu, and
keyboard focus). Internal dest-based navigation is not yet wired.

* Address review feedback (#1788)

- Replace hardcoded ``z-index: 2`` in NativeLinkLayer with new
  ``Z_INDEX.PDF_NATIVE_LINK_LAYER`` constant + sibling
  ``PDF_SELECTION_LAYER`` entry, removing the magic-number rule violation
  flagged in review.
- Switch ``NativeLinkLayer`` from ``React.FC`` (which the project's
  ``"jsx": "react-jsx"`` transform leaves out of scope unless React is
  explicitly imported) to a named ``FC`` import from "react" — same
  pattern recommended by review.
- Drop the ``as any`` cast in NativeLinkLayer.test.tsx for a proper
  ``as unknown as PDFPageProxy`` so the any-baseline lint check is back
  to clean.
- Slim down the CHANGELOG entry per review minor #4 — the existing entry
  doubled as a design doc; the format guidance asks for a short summary
  with file paths + a coverage pointer. Also drop the duplicate
  "Discovery endpoints rebranded" entry that local-main drift surfaced
  here (PR #1785 already owns that entry on main).

* Address #1788 review: wire SelectionLayer to constant, reset rawLinks, add aria-label

- SelectionLayer was still using a literal `zIndex: 1` even though
  the PR introduced `Z_INDEX.PDF_SELECTION_LAYER` to make the
  layering relationship between SelectionLayer (1) and the new
  NativeLinkLayer (2) explicit. Wire it through so the constant is
  not dead.
- NativeLinkLayer now clears `rawLinks` at the start of the page
  effect so a future recycled component instance cannot briefly
  paint the previous page's link positions while the new fetch is
  in flight.
- Transparent anchors now expose `aria-label={link.url}` so
  screen readers (VoiceOver in particular ignores the `title`
  attribute as an accessible-name source) get a robust name.
- Narrow rect type from `number[]` to a 4-tuple, with an explicit
  `length === 4` guard before the cast, so the
  `[x1, y1, x2, y2]` destructuring is type-safe.
- Strengthen the unit test mock: `getViewport` now honours the
  passed `scale` (was hard-coded to ×2), and the test asserts
  `getViewport` was called with the expected scale. Adds an
  aria-label regression test.

---------

Signed-off-by: JSIV <5049984+JSv4@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file python Pull requests that update Python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants