Skip to content

CBG-5715: fix _changes filter name, compound seq, and dumpchannel since - #8657

Merged
torcolvin merged 4 commits into
mainfrom
CBG-5715-09-descriptions-and-seq
Sep 7, 2026
Merged

CBG-5715: fix _changes filter name, compound seq, and dumpchannel since#8657
torcolvin merged 4 commits into
mainfrom
CBG-5715-09-descriptions-and-seq

Conversation

@torcolvin

Copy link
Copy Markdown
Collaborator

CBG-5715

Split out of #8589 — stack 9/9, based on #8656.

  • The channel filter is sync_gateway/bychannel (base.ByChannelFilter, base/constants.go:137), not sync_gateway/bychannels. The filter enum already had it right; four channels descriptions did not. (Raised by @factory-droid on CBG-5715 docs(api): fix OpenAPI errors #8589.)
  • Changes-feed.seq was documented as an integer, but SequenceID.MarshalJSON (db/sequence_id.go:155) emits a quoted compound string whenever TriggeredBy or LowSeq is set (e.g. during a channel backfill), so clients must handle both.
  • _dumpchannel?since is read with getIntQuery (rest/bulk_api.go:359), so it is a plain sequence number, not the opaque last_seq value the description described.
  • stale also accepts false, which is the value Sync Gateway's own view queries use. Worth a reviewer's eye: rest/view_api.go:100 passes stale through to the view engine verbatim, so nothing in SG validates the value set — the enum as a whole is unverifiable from the code, and dropping it may be more honest than extending it.
  • The stats logging block was described as "Trace logging configuration".

Pre-review checklist

  • Logging sensitive data? N/A — docs only
  • Updated relevant information in the API specifications in docs/api

🤖 Generated with Claude Code

@factory-droid

factory-droid Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Droid finished @torcolvin's task —— View job


Validation complete (Pass 2). Posted 2 approved inline comments.

Summary: The spec updates align the docs with current server behavior, but a couple of sequence-number fields are still underspecified for code generation. Adding explicit int64 formats would prevent client overflow/truncation issues while keeping the rest of the contract intact.

type: integer
description: The change sequence number. This is usually a plain integer, but can be a compound (string) sequence value in some cases, such as during a channel backfill.
oneOf:
- type: integer

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Mark changes feed sequence numbers as int64

seq is backed by db.SequenceID (uint64 for the simple form), and leaving the integer branch unformatted means many OpenAPI generators default this to int32, which can overflow/truncate real-world sequence values. Add format: int64 to the integer variant so generated clients treat sequence numbers as 64-bit.

Suggested change
- type: integer
- type: integer
format: int64

Comment thread docs/api/paths/admin/keyspace-_dumpchannel-channel.yaml
@torcolvin
torcolvin requested review from a team and a lite review from Copilot August 23, 2026 18:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Sync Gateway’s OpenAPI documentation to match the actual server behavior and constants around _changes, _dumpchannel, and view query parameters.

Changes:

  • Corrects the _changes channel filter documentation to use sync_gateway/bychannel (matching base.ByChannelFilter).
  • Documents that Changes-feed.results[].seq can be either an integer or a compound string sequence value (matching db.SequenceID.MarshalJSON behavior).
  • Fixes admin _dumpchannel?since to be a plain integer sequence number (matching rest/handler.getIntQuery usage) and expands stale enum support to include "false".

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
docs/api/paths/public/keyspace-_changes.yaml Fixes channels parameter description to reference the correct sync_gateway/bychannel filter.
docs/api/paths/admin/keyspace-_dumpchannel-channel.yaml Changes since query parameter to an integer sequence number with int64/min bounds.
docs/api/paths/admin/keyspace-_changes.yaml Fixes channels parameter description to reference the correct sync_gateway/bychannel filter.
docs/api/components/schemas.yaml Updates changes-feed seq to oneOf [integer,int64,string], adds int64 format for channel access sequences, and fixes stats logging description text.
docs/api/components/parameters.yaml Adds "false" to the allowed stale query parameter enum values.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@torcolvin
torcolvin force-pushed the CBG-5715-09-descriptions-and-seq branch from 0e81ca8 to 74fbc3f Compare August 23, 2026 18:33
@torcolvin
torcolvin force-pushed the CBG-5715-09-descriptions-and-seq branch from 74fbc3f to e90b398 Compare August 23, 2026 18:44
@torcolvin
torcolvin force-pushed the CBG-5715-09-descriptions-and-seq branch 2 times, most recently from 300aad6 to 36321d5 Compare August 24, 2026 20:22
@bbrks bbrks self-assigned this Sep 4, 2026
bbrks
bbrks previously approved these changes Sep 7, 2026
@bbrks
bbrks force-pushed the CBG-5715-09-descriptions-and-seq branch from 36321d5 to 4556d81 Compare September 7, 2026 12:44
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Redocly previews

stack merge was automatically disabled September 7, 2026 15:33

Pull Request is not mergeable

@torcolvin
torcolvin force-pushed the CBG-5715-09-descriptions-and-seq branch from 4556d81 to b37db30 Compare September 7, 2026 18:24
@torcolvin
torcolvin force-pushed the CBG-5715-09-descriptions-and-seq branch from b37db30 to 6fc5591 Compare September 7, 2026 19:32
@torcolvin
torcolvin force-pushed the CBG-5715-09-descriptions-and-seq branch from 6fc5591 to 1e66445 Compare September 7, 2026 20:02
@torcolvin
torcolvin force-pushed the CBG-5715-09-descriptions-and-seq branch from 1e66445 to bbf5642 Compare September 7, 2026 20:16
@torcolvin
torcolvin force-pushed the CBG-5715-09-descriptions-and-seq branch from bbf5642 to c587b96 Compare September 7, 2026 20:27
@torcolvin
torcolvin force-pushed the CBG-5715-09-descriptions-and-seq branch from c587b96 to eaf21f7 Compare September 7, 2026 20:55
@torcolvin
torcolvin force-pushed the CBG-5715-09-descriptions-and-seq branch from eaf21f7 to 31af4ba Compare September 7, 2026 21:19
Base automatically changed from CBG-5715-08-defaults-and-bounds to main September 7, 2026 21:41
torcolvin and others added 4 commits September 7, 2026 17:41
- `javascript_timeout_secs` documented a default of 60, but
  `base.DefaultJavascriptTimeoutSecs` is 0 (uncapped).
- `revs_limit` documented `minimum: 0`, which config validation rejects
  ("must be greater than zero"). Use 1, and note the higher floor and
  different default that apply when `allow_conflicts` is enabled.
- The stats logger's `max_age` documented a default of 6. Unlike the other
  loggers it does not derive its default from `minAge`; it uses the fixed
  `statsDefaultMaxAgeOverride` of 90. The error/warn/info/debug/trace and
  audit defaults were checked and are correct.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
max_processes is a uint, and db/event_manager.go:69-70 maps 0 to
kMaxActiveEvents (500), so document minimum: 0 with the effective
default and say what 0 means.  Raised by Copilot on #8654.

heartbeat and timeout in the _changes POST bodies now carry the same
defaults and bounds already documented on the GET query parameters:
heartbeat defaults to 0 and, when non-zero, is clamped up to
kMinHeartbeatMS (25000); timeout defaults to kDefaultTimeoutMS (300000)
and is capped at kMaxTimeoutMS (900000).  See GetRestrictedInt, which
readChangesOptionsFromJSON calls with allowZero=true.  Copilot flagged
these as heartbeat-only, but the timeout default is 300000, not 0.

Also drops the revs_limit allow_conflicts note.  Both branches it
describes exist (db/database.go:504-508, rest/config.go:936-940) but
neither is reachable from config: allow_conflicts=true is rejected in
the same validateVersion (rest/config.go:1149-1150) and
DefaultAllowConflicts is false, so only the test-only
EnableAllowConflicts gets there.  Raised by @factory-droid on #8656.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- The channel filter is `sync_gateway/bychannel` (`base.ByChannelFilter`),
  not `sync_gateway/bychannels`. The `filter` enum already had it right;
  four `channels` descriptions did not.
- `Changes-feed.seq` was documented as an integer, but
  `SequenceID.MarshalJSON` emits a quoted compound string whenever
  `TriggeredBy` or `LowSeq` is set (e.g. during a channel backfill), so
  clients must handle both.
- `_dumpchannel?since` is read with `getIntQuery`, so it is a plain
  sequence number, not the opaque `last_seq` value the description
  described.
- `stale` also accepts `false`, which is the value Sync Gateway's own view
  queries use.
- The stats logging block was described as "Trace logging configuration".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@torcolvin
torcolvin force-pushed the CBG-5715-09-descriptions-and-seq branch from 31af4ba to 28b3cca Compare September 7, 2026 21:41
@torcolvin
torcolvin merged commit 6047f56 into main Sep 7, 2026
53 checks passed
@torcolvin
torcolvin deleted the CBG-5715-09-descriptions-and-seq branch September 7, 2026 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants