feat: add deterministic PTC rollout percentage gating#2266
Conversation
📝 WalkthroughWalkthroughAdds deterministic per-user rollout gating for PTC: two new config fields (rolloutPercent, rolloutSalt), a stable hash-based bucket computation, a public isPtcEnabledForRollout(uid, config) function, and integration of the rollout gate into the existing toolset eligibility flow with tests and docs updated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying refly-branch-test with
|
| Latest commit: |
6e4e5f1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3275db19.refly-branch-test.pages.dev |
| Branch Preview URL: | https://ptc-gray-pr.refly-branch-test.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/api/src/modules/config/app.config.ts (1)
264-265: Consider usingNumber.parseIntfor consistency with other numeric configs in this file.The rest of the file uses
Number.parseInt()for integer parsing (e.g.,PORT,WS_PORT,REDIS_PORT), butrolloutPercentusesNumber(). While both work,Number.parseIntis more explicit for integer values and aligns with the existing pattern.Additionally, if
PTC_ROLLOUT_PERCENTis set to an invalid value (e.g.,"abc"),Number()returnsNaN. The downstream clamping ingetPtcConfigwon't catch this (Math.max(0, NaN)returnsNaN), though the behavior is fail-safe (rollout disabled). For robustness, consider adding|| 100as a fallback.♻️ Optional: align with existing patterns
- rolloutPercent: Number(process.env.PTC_ROLLOUT_PERCENT ?? 100), + rolloutPercent: Number.parseInt(process.env.PTC_ROLLOUT_PERCENT) || 100,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/modules/config/app.config.ts` around lines 264 - 265, Change parsing of the PTC_ROLLOUT_PERCENT env var to use Number.parseInt to match other integer configs: replace the Number(...) usage for rolloutPercent with Number.parseInt(process.env.PTC_ROLLOUT_PERCENT ?? '100', 10) (or equivalent) and include a fallback (|| 100) so invalid values produce the default; ensure this aligns with the downstream getPtcConfig behavior and references rolloutPercent and PTC_ROLLOUT_PERCENT in the change.apps/api/src/modules/tool/ptc/ptc-config.ts (2)
82-86: NaN from invalid config passes through clamping unchanged.If
configService.get<number>('ptc.rolloutPercent')returnsNaN(from invalid env var parsing upstream), theMath.min/Math.maxclamping won't catch it sinceMath.max(0, NaN)returnsNaN.The behavior is fail-safe (NaN effectively disables rollout since
bucket < NaNis always false), but for defensive coding, consider adding an explicitisNaNcheck with a fallback to the default.🛡️ Optional defensive fix
- const rolloutPercent = Math.min( - 100, - Math.max(0, configService.get<number>('ptc.rolloutPercent') ?? 100), - ); + const rawPercent = configService.get<number>('ptc.rolloutPercent') ?? 100; + const rolloutPercent = Number.isNaN(rawPercent) + ? 100 + : Math.min(100, Math.max(0, rawPercent));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/modules/tool/ptc/ptc-config.ts` around lines 82 - 86, The computed rolloutPercent uses Math.min/Math.max on configService.get<number>('ptc.rolloutPercent') but a NaN value will pass through; update the logic where rolloutPercent is set to explicitly validate the fetched value (from configService.get<number>('ptc.rolloutPercent')) with Number.isFinite or Number.isNaN and fall back to the default 100 before clamping, so rolloutPercent ends up as a finite number between 0 and 100; keep the existing rolloutSalt logic unchanged.
135-147: Clarify in JSDoc that this function assumes prior eligibility checks.The exported
isPtcEnabledForRolloutcould be called directly by other code without the preceding user/toolset checks. Consider enhancing the JSDoc to clarify that this function is typically called afterisPtcEnabledForUserandisToolsetAllowedchecks, or that it only determines rollout eligibility (not full PTC eligibility).📝 Suggested JSDoc enhancement
/** * Check if a user falls within the rollout percentage. * Users are bucketed deterministically so their assignment is stable. + * + * Note: This only checks rollout eligibility. For full PTC eligibility, + * use `isPtcEnabledForToolsets` which applies mode, user, toolset, and + * rollout checks in priority order. * * `@param` uid - The user ID * `@param` config - PTC configuration * `@returns` true if the user is within the rollout percentage */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/modules/tool/ptc/ptc-config.ts` around lines 135 - 147, The JSDoc for isPtcEnabledForRollout should state that it only computes deterministic rollout membership and does not perform other eligibility checks; update the comment on isPtcEnabledForRollout to clarify it assumes prior checks like isPtcEnabledForUser and isToolsetAllowed have been performed and that it only uses computeRolloutBucket and PtcConfig.rolloutSalt/rolloutPercent to decide rollout inclusion.apps/api/src/modules/tool/ptc/ptc-config.spec.ts (2)
263-301: Consider adding edge case tests for rollout percentage clamping.The test suite covers core rollout scenarios well (0%, 100%, determinism, salt changes). However, there are no tests verifying that
getPtcConfigcorrectly clamps out-of-range values (e.g.,-10→0,150→100). This would validate the clamping logic ingetPtcConfigand document expected behavior.🧪 Suggested additional test cases
it('should clamp rolloutPercent below 0 to 0', () => { mockConfigService.get = jest.fn((key: string) => { const configs: Record<string, string | number> = { 'ptc.mode': 'on', 'ptc.debug': '', 'ptc.userAllowlist': '', 'ptc.toolsetAllowlist': '', 'ptc.toolsetBlocklist': '', 'ptc.rolloutPercent': -10, 'ptc.rolloutSalt': 'test-salt', }; return configs[key]; }); const config = getPtcConfig(mockConfigService as ConfigService); expect(config.rolloutPercent).toBe(0); }); it('should clamp rolloutPercent above 100 to 100', () => { mockConfigService.get = jest.fn((key: string) => { const configs: Record<string, string | number> = { 'ptc.mode': 'on', 'ptc.debug': '', 'ptc.userAllowlist': '', 'ptc.toolsetAllowlist': '', 'ptc.toolsetBlocklist': '', 'ptc.rolloutPercent': 150, 'ptc.rolloutSalt': 'test-salt', }; return configs[key]; }); const config = getPtcConfig(mockConfigService as ConfigService); expect(config.rolloutPercent).toBe(100); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/modules/tool/ptc/ptc-config.spec.ts` around lines 263 - 301, Add tests to verify getPtcConfig clamps out-of-range rolloutPercent values: mock the ConfigService (mockConfigService.get) to return rolloutPercent -10 and assert getPtcConfig(...).rolloutPercent === 0, and similarly return 150 and assert === 100; reference getPtcConfig and the rolloutPercent property in your new test cases to ensure clamping behavior is validated.
75-89: Add missing rollout fields to debugMode test mocks for consistency.Several
debugModetest cases (lines 75-137) define mock configs withoutrolloutPercentandrolloutSalt. While the code handles missing values via defaults, adding these fields would maintain consistency with the updated test patterns elsewhere in this file.♻️ Example fix for one test case
mockConfigService.get = jest.fn((key: string) => { - const configs: Record<string, string> = { + const configs: Record<string, string | number> = { 'ptc.mode': 'on', 'ptc.debug': 'true', 'ptc.userAllowlist': '', 'ptc.toolsetAllowlist': '', 'ptc.toolsetBlocklist': '', + 'ptc.rolloutPercent': 100, + 'ptc.rolloutSalt': 'ptc-rollout', }; return configs[key]; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/modules/tool/ptc/ptc-config.spec.ts` around lines 75 - 89, The test for parsing legacy debugMode omits rolloutPercent and rolloutSalt in the mock configs; update the mockConfigService.get used in the spec (the test invoking getPtcConfig and expecting PtcDebugMode.OPT_IN) to include rolloutPercent and rolloutSalt keys (e.g., empty string or explicit defaults) alongside 'ptc.mode', 'ptc.debug', 'ptc.userAllowlist', 'ptc.toolsetAllowlist', and 'ptc.toolsetBlocklist' so the mock shape matches other tests and the config parsing behavior remains consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@specs/change/20260309-ptc-rollout-percentage-priority/spec.md`:
- Line 95: Replace the PII in the sentence that reads "Live rollout test with
`anchori@163.com` (`u-kxuho6my1092vt8d1xshdrvx`): bucket = **2** → excluded at
`PTC_ROLLOUT_PERCENT=2`, included at `PTC_ROLLOUT_PERCENT=3`" by removing the
real email "anchori@163.com" and the user id "u-kxuho6my1092vt8d1xshdrvx" and
substituting anonymized placeholders (e.g., `user@example.com` or
`<REDACTED_EMAIL>` and `user-id-placeholder` or `<REDACTED_ID>`), keeping the
rest of the sentence and the `PTC_ROLLOUT_PERCENT` references intact.
In `@specs/current/ptc/PTC.md`:
- Around line 306-313: Remove the stray empty code fence after the environment
block in the "7. Gradual Rollout — Enable PTC for 10% of Users" section: locate
the orphaned opening triple backticks following the ```env block that contains
PTC_MODE and PTC_ROLLOUT_PERCENT and delete that extra ``` so the markdown has
only the intended fenced env block and no unmatched fence causing MD040.
---
Nitpick comments:
In `@apps/api/src/modules/config/app.config.ts`:
- Around line 264-265: Change parsing of the PTC_ROLLOUT_PERCENT env var to use
Number.parseInt to match other integer configs: replace the Number(...) usage
for rolloutPercent with Number.parseInt(process.env.PTC_ROLLOUT_PERCENT ??
'100', 10) (or equivalent) and include a fallback (|| 100) so invalid values
produce the default; ensure this aligns with the downstream getPtcConfig
behavior and references rolloutPercent and PTC_ROLLOUT_PERCENT in the change.
In `@apps/api/src/modules/tool/ptc/ptc-config.spec.ts`:
- Around line 263-301: Add tests to verify getPtcConfig clamps out-of-range
rolloutPercent values: mock the ConfigService (mockConfigService.get) to return
rolloutPercent -10 and assert getPtcConfig(...).rolloutPercent === 0, and
similarly return 150 and assert === 100; reference getPtcConfig and the
rolloutPercent property in your new test cases to ensure clamping behavior is
validated.
- Around line 75-89: The test for parsing legacy debugMode omits rolloutPercent
and rolloutSalt in the mock configs; update the mockConfigService.get used in
the spec (the test invoking getPtcConfig and expecting PtcDebugMode.OPT_IN) to
include rolloutPercent and rolloutSalt keys (e.g., empty string or explicit
defaults) alongside 'ptc.mode', 'ptc.debug', 'ptc.userAllowlist',
'ptc.toolsetAllowlist', and 'ptc.toolsetBlocklist' so the mock shape matches
other tests and the config parsing behavior remains consistent.
In `@apps/api/src/modules/tool/ptc/ptc-config.ts`:
- Around line 82-86: The computed rolloutPercent uses Math.min/Math.max on
configService.get<number>('ptc.rolloutPercent') but a NaN value will pass
through; update the logic where rolloutPercent is set to explicitly validate the
fetched value (from configService.get<number>('ptc.rolloutPercent')) with
Number.isFinite or Number.isNaN and fall back to the default 100 before
clamping, so rolloutPercent ends up as a finite number between 0 and 100; keep
the existing rolloutSalt logic unchanged.
- Around line 135-147: The JSDoc for isPtcEnabledForRollout should state that it
only computes deterministic rollout membership and does not perform other
eligibility checks; update the comment on isPtcEnabledForRollout to clarify it
assumes prior checks like isPtcEnabledForUser and isToolsetAllowed have been
performed and that it only uses computeRolloutBucket and
PtcConfig.rolloutSalt/rolloutPercent to decide rollout inclusion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a74ebe5-b9f1-4f88-9fa9-2a1f4bf7d52b
📒 Files selected for processing (5)
apps/api/src/modules/config/app.config.tsapps/api/src/modules/tool/ptc/ptc-config.spec.tsapps/api/src/modules/tool/ptc/ptc-config.tsspecs/change/20260309-ptc-rollout-percentage-priority/spec.mdspecs/current/ptc/PTC.md
- add PTC_ROLLOUT_PERCENT and PTC_ROLLOUT_SALT config values with safe defaults - introduce deterministic user bucketing and apply rollout gating after mode and toolset eligibility checks - expand ptc-config tests to cover rollout boundaries, determinism, and salt rebucketing behavior - document rollout priority rules and add implementation notes in PTC specs
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
specs/current/ptc/PTC.md (1)
306-313:⚠️ Potential issue | 🟡 MinorRemove the stray empty code fence.
There is still an orphaned triple-backtick after the env example, which breaks the markdown block structure.
Suggested fix
#### 7. Gradual Rollout — Enable PTC for 10% of Users ```env PTC_MODE=on PTC_ROLLOUT_PERCENT=10
-```
</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@specs/current/ptc/PTC.mdaround lines 306 - 313, Remove the stray closing
code fence after the env example under the "7. Gradual Rollout — Enable PTC for
10% of Users" section; specifically, keep a single fenced code block containing
the env lines (PTC_MODE=on and PTC_ROLLOUT_PERCENT=10) and delete the orphaned
triple-backtick that follows it so the Markdown block structure is correct.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>apps/api/src/modules/tool/ptc/ptc-config.spec.ts (1)</summary><blockquote> `35-45`: **Add a regression case for invalid rollout values.** The new coverage only exercises valid percentages. A `PTC_ROLLOUT_PERCENT='abc'` / `NaN` parse case would lock down the “safe default = 100” contract and catch the silent 0% behavior here. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@apps/api/src/modules/tool/ptc/ptc-config.spec.tsaround lines 35 - 45, Add a
regression test in ptc-config.spec.ts to cover invalid rollout values by calling
getPtcConfig with a mock ConfigService that returns an invalid string (e.g.
'abc' or NaN) for PTC_ROLLOUT_PERCENT and assert that the resulting
config.rolloutPercent falls back to the safe default of 100 (and rolloutSalt
remains 'ptc-rollout' if applicable); locate the test near the existing
getPtcConfig spec so it exercises the parsing logic in getPtcConfig and prevents
silent 0% behavior.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@apps/api/src/modules/tool/ptc/ptc-config.ts:
- Around line 128-132: The computeRolloutBucket function currently hashes uid +
salt which is ambiguous (e.g., 'ab'+'c' == 'a'+'bc'); change the hash input to
an unambiguous encoding (for example, join with a delimiter or length-prefix the
parts) so uid and salt cannot collide, e.g., use a clear separator or
Buffer.concat with a null/length byte between uid and salt before calling
createHash('sha256'). Ensure you update computeRolloutBucket to use that new
unambiguous input construction.- Around line 82-85: The rolloutPercent calculation should guard against NaN
before clamping: when reading the value from
configService.get('ptc.rolloutPercent') (used to compute
rolloutPercent), detect NaN or non-finite values and replace them with the safe
default (100) before applying Math.max/Math.min so isPtcEnabledForRollout()
doesn't get NaN; update the expression that assigns rolloutPercent to first
validate the fetched value (e.g., via Number.isFinite or isNaN) and fall back to
100 if invalid, then clamp to 0–100.In
@specs/current/ptc/PTC.md:
- Around line 240-246: Add a new paragraph documenting the PTC_ROLLOUT_SALT env
var alongside PTC_ROLLOUT_PERCENT: specify the variable name PTC_ROLLOUT_SALT,
its default value "ptc-rollout", and that it is concatenated with uid in the
bucketing computation (SHA-256(uid + salt) % 100 < percent); also note
recommended operator guidance (change the salt to reset/stagger bucket
assignments across environments or releases, and keep it stable for a given
rollout) so readers know when and why to rotate it.
Duplicate comments:
In@specs/current/ptc/PTC.md:
- Around line 306-313: Remove the stray closing code fence after the env example
under the "7. Gradual Rollout — Enable PTC for 10% of Users" section;
specifically, keep a single fenced code block containing the env lines
(PTC_MODE=on and PTC_ROLLOUT_PERCENT=10) and delete the orphaned triple-backtick
that follows it so the Markdown block structure is correct.
Nitpick comments:
In@apps/api/src/modules/tool/ptc/ptc-config.spec.ts:
- Around line 35-45: Add a regression test in ptc-config.spec.ts to cover
invalid rollout values by calling getPtcConfig with a mock ConfigService that
returns an invalid string (e.g. 'abc' or NaN) for PTC_ROLLOUT_PERCENT and assert
that the resulting config.rolloutPercent falls back to the safe default of 100
(and rolloutSalt remains 'ptc-rollout' if applicable); locate the test near the
existing getPtcConfig spec so it exercises the parsing logic in getPtcConfig and
prevents silent 0% behavior.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `d0220726-7063-411b-9920-af4200c211ff` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between fcc177a4d12c7be585deaf88332c9d8a5a3f4613 and 6e4e5f1ffc645ef01c20e113751b2e2bffe471f8. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `apps/api/src/modules/config/app.config.ts` * `apps/api/src/modules/tool/ptc/ptc-config.spec.ts` * `apps/api/src/modules/tool/ptc/ptc-config.ts` * `specs/change/20260309-ptc-rollout-percentage-priority/spec.md` * `specs/current/ptc/PTC.md` </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * apps/api/src/modules/config/app.config.ts </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| const rolloutPercent = Math.min( | ||
| 100, | ||
| Math.max(0, configService.get<number>('ptc.rolloutPercent') ?? 100), | ||
| ); |
There was a problem hiding this comment.
Guard rolloutPercent against NaN before clamping.
app.config.ts uses Number(process.env.PTC_ROLLOUT_PERCENT ?? 100), so a misconfigured env like PTC_ROLLOUT_PERCENT=abc reaches this code as NaN. The current clamp preserves NaN, and isPtcEnabledForRollout() then disables PTC for everyone instead of falling back to the safe default.
Suggested fix
- const rolloutPercent = Math.min(
- 100,
- Math.max(0, configService.get<number>('ptc.rolloutPercent') ?? 100),
- );
+ const rawRolloutPercent = Number(configService.get<number>('ptc.rolloutPercent'));
+ const rolloutPercent = Number.isFinite(rawRolloutPercent)
+ ? Math.min(100, Math.max(0, rawRolloutPercent))
+ : 100;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/modules/tool/ptc/ptc-config.ts` around lines 82 - 85, The
rolloutPercent calculation should guard against NaN before clamping: when
reading the value from configService.get<number>('ptc.rolloutPercent') (used to
compute rolloutPercent), detect NaN or non-finite values and replace them with
the safe default (100) before applying Math.max/Math.min so
isPtcEnabledForRollout() doesn't get NaN; update the expression that assigns
rolloutPercent to first validate the fetched value (e.g., via Number.isFinite or
isNaN) and fall back to 100 if invalid, then clamp to 0–100.
| function computeRolloutBucket(uid: string, salt: string): number { | ||
| const hash = createHash('sha256') | ||
| .update(uid + salt) | ||
| .digest(); | ||
| return hash.readUInt32BE(0) % 100; |
There was a problem hiding this comment.
Make the rollout hash input unambiguous.
Hashing uid + salt can collide for different pairs, e.g. ('ab', 'c') and ('a', 'bc'). That means changing PTC_ROLLOUT_SALT can fail to re-bucket some users in edge cases.
Suggested fix
function computeRolloutBucket(uid: string, salt: string): number {
const hash = createHash('sha256')
- .update(uid + salt)
+ .update(JSON.stringify([uid, salt]))
.digest();
return hash.readUInt32BE(0) % 100;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function computeRolloutBucket(uid: string, salt: string): number { | |
| const hash = createHash('sha256') | |
| .update(uid + salt) | |
| .digest(); | |
| return hash.readUInt32BE(0) % 100; | |
| function computeRolloutBucket(uid: string, salt: string): number { | |
| const hash = createHash('sha256') | |
| .update(JSON.stringify([uid, salt])) | |
| .digest(); | |
| return hash.readUInt32BE(0) % 100; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/modules/tool/ptc/ptc-config.ts` around lines 128 - 132, The
computeRolloutBucket function currently hashes uid + salt which is ambiguous
(e.g., 'ab'+'c' == 'a'+'bc'); change the hash input to an unambiguous encoding
(for example, join with a delimiter or length-prefix the parts) so uid and salt
cannot collide, e.g., use a clear separator or Buffer.concat with a null/length
byte between uid and salt before calling createHash('sha256'). Ensure you update
computeRolloutBucket to use that new unambiguous input construction.
| ### Priority 4 — PTC_ROLLOUT_PERCENT | ||
| Integer 0–100, default `100`. Controls what percentage of otherwise-eligible users can use PTC via deterministic per-user bucketing. | ||
|
|
||
| - `100` (default): all eligible users included — no change to existing behaviour. | ||
| - `0`: kill switch — blocks all users without touching mode/toolset config. | ||
| - `1–99`: gradual rollout. Bucket is computed as `SHA-256(uid + salt) % 100 < percent`. Assignment is stable: the same user always gets the same result for a given salt. | ||
|
|
There was a problem hiding this comment.
Document PTC_ROLLOUT_SALT alongside the rollout percent.
This section explains that bucketing depends on a salt, but it never defines the env var itself, its default (ptc-rollout), or when operators should change it. That leaves the rollout docs incomplete for the new feature.
Suggested doc update
### Priority 4 — PTC_ROLLOUT_PERCENT
Integer 0–100, default `100`. Controls what percentage of otherwise-eligible users can use PTC via deterministic per-user bucketing.
- `100` (default): all eligible users included — no change to existing behaviour.
- `0`: kill switch — blocks all users without touching mode/toolset config.
- `1–99`: gradual rollout. Bucket is computed as `SHA-256(uid + salt) % 100 < percent`. Assignment is stable: the same user always gets the same result for a given salt.
+
+### Priority 4 — PTC_ROLLOUT_SALT
+String, default `ptc-rollout`. Seeds the deterministic rollout bucket used by `PTC_ROLLOUT_PERCENT`.
+
+- Keep it unchanged to preserve user assignment during a rollout.
+- Change it intentionally to re-bucket users for a new rollout wave.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@specs/current/ptc/PTC.md` around lines 240 - 246, Add a new paragraph
documenting the PTC_ROLLOUT_SALT env var alongside PTC_ROLLOUT_PERCENT: specify
the variable name PTC_ROLLOUT_SALT, its default value "ptc-rollout", and that it
is concatenated with uid in the bucketing computation (SHA-256(uid + salt) % 100
< percent); also note recommended operator guidance (change the salt to
reset/stagger bucket assignments across environments or releases, and keep it
stable for a given rollout) so readers know when and why to rotate it.
2 participants
Summary
This PR adds deterministic percentage-based rollout control for PTC while preserving existing mode, user, and toolset gating behavior.
Changes
PTC_ROLLOUT_PERCENTandPTC_ROLLOUT_SALTto app/PTC configuration with clamped defaultsptc-config.spec.tsfor 0/100 boundaries, deterministic behavior, and salt-based rebucketingSummary by CodeRabbit
New Features
Documentation
Tests