CBG-5715: fix _changes filter name, compound seq, and dumpchannel since - #8657
Open
torcolvin wants to merge 5 commits into
Open
CBG-5715: fix _changes filter name, compound seq, and dumpchannel since#8657torcolvin wants to merge 5 commits into
torcolvin wants to merge 5 commits into
Conversation
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 |
![factory-droid[bot]](https://avatars.githubusercontent.com/in/358017?s=60&v=4){kind=small}
| 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 |
Contributor
There was a problem hiding this comment.
[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 |
Contributor
There was a problem hiding this comment.
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
_changeschannel filter documentation to usesync_gateway/bychannel(matchingbase.ByChannelFilter). - Documents that
Changes-feed.results[].seqcan be either an integer or a compound string sequence value (matchingdb.SequenceID.MarshalJSONbehavior). - Fixes admin
_dumpchannel?sinceto be a plain integer sequence number (matchingrest/handler.getIntQueryusage) and expandsstaleenum 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
force-pushed
the
CBG-5715-09-descriptions-and-seq
branch
from
August 23, 2026 18:33
0e81ca8 to
74fbc3f
Compare
torcolvin
force-pushed
the
CBG-5715-09-descriptions-and-seq
branch
from
August 23, 2026 18:44
74fbc3f to
e90b398
Compare
torcolvin
force-pushed
the
CBG-5715-09-descriptions-and-seq
branch
from
August 24, 2026 17:31
e90b398 to
300aad6
Compare
The `mark`/`sweep`/`cleanup` enum was indented one property too far down, so it constrained `dry_run` (a `bool` in Go, previously also documented as a string) while `phase` — the field that actually carries those values — was documented as an unconstrained string. Move the enum onto `phase`, document each value, and correct `dry_run` to boolean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- `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
force-pushed
the
CBG-5715-09-descriptions-and-seq
branch
from
August 24, 2026 20:22
300aad6 to
36321d5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CBG-5715
Split out of #8589 — stack 9/9, based on #8656.
sync_gateway/bychannel(base.ByChannelFilter,base/constants.go:137), notsync_gateway/bychannels. Thefilterenum already had it right; fourchannelsdescriptions did not. (Raised by @factory-droid on CBG-5715 docs(api): fix OpenAPI errors #8589.)Changes-feed.seqwas documented as an integer, butSequenceID.MarshalJSON(db/sequence_id.go:155) emits a quoted compound string wheneverTriggeredByorLowSeqis set (e.g. during a channel backfill), so clients must handle both._dumpchannel?sinceis read withgetIntQuery(rest/bulk_api.go:359), so it is a plain sequence number, not the opaquelast_seqvalue the description described.stalealso acceptsfalse, which is the value Sync Gateway's own view queries use. Worth a reviewer's eye:rest/view_api.go:100passesstalethrough 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.Pre-review checklist
docs/api🤖 Generated with Claude Code