Skip to content

Add v3 package documentation and code review#33

Closed
prjseal wants to merge 15 commits into
masterfrom
dev/v3
Closed

Add v3 package documentation and code review#33
prjseal wants to merge 15 commits into
masterfrom
dev/v3

Conversation

@prjseal
Copy link
Copy Markdown
Owner

@prjseal prjseal commented May 24, 2026

Detailed single-file reference covering the public API, internal
generation flow, confirmed bugs (error-string returns, off-by-one
character selection, modulo bias, non-uniform shuffle, static RNG),
build/test results, packaging hygiene, feature gaps, and a prioritised
v3 plan.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd

claude and others added 15 commits May 24, 2026 11:00
Detailed single-file reference covering the public API, internal
generation flow, confirmed bugs (error-string returns, off-by-one
character selection, modulo bias, non-uniform shuffle, static RNG),
build/test results, packaging hygiene, feature gaps, and a prioritised
v3 plan.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
The default MSBuild step on the old image used MSBuild 14, which cannot
parse SDK-style csproj files (MSB4041). Switch to dotnet restore/build/
test/pack on the VS2019 image and publish the nupkg as a build artifact.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
Add 'deploy: off' so no NuGet (or other) deployment runs, regardless of
any deploy provider configured in the AppVeyor project UI. CI only
builds, tests, and packs the nupkg as a downloadable artifact.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
Re-checks every original-review bug (§5) and feature gap (§8) against the
current master source, marking each Confirmed / Already Fixed / Partially
Fixed with code references. Corrects the addendum's premise that the Guid
shuffle was removed (it still exists at Password.cs:247-249 on both master
and dev/v2) and confirms master is ahead of the older dev/v2 line.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
New docs/ index plus paired current-state/ (as-is architecture, generation
flow, API surface) and v3-target/ (target architecture, generation flow,
API surface, config & DI, before/after, roadmap) sets. Diagrams show what
the package does today, where v3 is headed, and why each change is better,
tied back to verified issues. All 33 mermaid diagrams validated with the
mermaid parser.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
Phase 0 (toolchain: install .NET SDK via bash + green baseline) through
Phase 6 (docs/migration). Each phase has objective, tasks, files, and
exit criteria, plus an issue-to-phase traceability table and a per-session
checklist. Linked from the docs index. All 34 mermaid diagrams validated.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
Add a "Definition of done" loop (implement -> test -> green -> commit/push)
that applies to every phase, strengthen the working principles, and append
an explicit tests-green gate plus a per-phase commit step to each phase's
exit criteria and the per-session checklist.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
…5dOd

Add v3 package documentation and code review
* Phase 1: correctness & security core

Replace biased/static randomness and error-string returns with a fail-loud,
unbiased, instance-scoped design:

- Add IRandomSource + CryptoRandomSource (rejection sampling, no modulo bias,
  no off-by-one; instance-based and disposable) (§5.2, §5.3, §5.6)
- Delete the Guid shuffle and dead GetRngCryptoSeed; use crypto Fisher-Yates
  via IRandomSource (§5.5, §5.7)
- Guarantee included classes by construction (seed one per class, fill, shuffle)
  and drop the validate-and-retry loop / "Try again" string
- Next() throws ArgumentException on invalid config; add TryNext(out string);
  validate empty custom special set up front (§5.1, §5.8)
- Expose IPasswordSettings.CharacterGroups; make Password IDisposable
- Update tests to expect exceptions; add Phase1CorrectnessTests

All 32 tests pass (run on net8 since netcoreapp2.2 is EOL); library and
netcoreapp2.2 test build both succeed.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd

* Phase 2: targets & test modernisation

- Multi-target netstandard2.0;net8.0 with nullable enabled; net8 uses
  RandomNumberGenerator.GetInt32, netstandard2.0 keeps rejection sampling
- Migrate tests to net8.0 + NUnit 4 (constraint-model asserts), dropping the
  EOL netcoreapp2.2 target and its vulnerable Microsoft.NETCore.App 2.2.0
- Add edge-case/property tests (uniqueness, custom-pool exclusivity, length
  boundaries, no-classes, null random source)
- Add a BenchmarkDotNet project (single vs batch, sizes 1/100/1000/10000,
  MemoryDiagnoser) and wire it into the solution
- Bump AppVeyor image to Visual Studio 2022 for the net8 SDK
- Tighten .gitignore for bin/obj/artifacts and BenchmarkDotNet output

39/39 tests pass on net8; both library TFMs and the full solution build with
no nullable warnings.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd

* Phase 3: async API, DI registration, remove v2 wrappers

- Add IPasswordGenerator (Next/TryNext/NextAsync/Generate/GenerateAsync);
  Password implements it alongside IPassword. Async honours CancellationToken
- Keep sync methods fully supported (no [Obsolete]): generation is CPU-bound,
  so obsoleting sync in favour of async would be an anti-pattern
- Add opt-in DI: AddPasswordGenerator(Action<PasswordOptions>) and
  AddPasswordGenerator(IConfiguration) in the core package, taking
  Microsoft.Extensions.DependencyInjection.Abstractions and
  Configuration.Binder dependencies; new vs DI behave identically
- Add PasswordOptions for code/appSettings configuration
- Remove the [Obsolete] PasswordGenerator/PasswordGeneratorSettings wrappers
  and their tests, clearing all 5 CS0108 warnings
- Enable nullable in the test project; add Phase3Tests (async, cancellation,
  batch, DI binding + equivalence)

Library builds with 0 warnings/0 errors; 39/39 tests pass on net8; pack
declares the new dependencies per target framework.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd

* Phase 4: presets, custom pools, exclude-ambiguous, min-counts, entropy

- Custom pools: WithCharacters(string) and WithAllAscii() alongside Include*
- ExcludeAmbiguous() strips look-alike characters (CharacterFilter) from the
  pool and per-class groups
- RequireAtLeast(CharacterClass, count): generalises the one-per-class
  guarantee; auto-enables the class; validated against length
- IEntropyEstimator/PoolEntropyEstimator and Password.EstimateEntropyBits()
  (length * log2(effective pool size))
- Presets ForOwasp/ForNist/ForOtp/ForApiKey/ForEnvironmentName (static
  factories over the fluent builder) and ForPassphrase backed by a small
  built-in word list (PassphraseGenerator)
- Batch: parameterless Generate()/GenerateAsync() driven by a configurable
  DefaultBatchCount; kept Generate(count) (no .Count(n) chaining)
- appSettings precedence via AddPasswordGenerator(IConfiguration, Action):
  code-configure > appSettings > default; PasswordOptions gains
  ExcludeAmbiguous and DefaultBatchCount

Library builds with 0 warnings/0 errors; 55/55 tests pass on net8
(16 new Phase 4 tests).

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd

* build: clean packaging, SourceLink, snupkg, bump to 3.0.0

- Delete stale PasswordGenerator.nuspec (2.0.5); csproj is now the single
  source of version truth, bumped to 3.0.0
- Replace PackageIconUrl with PackageIcon (clears NU5048) and add
  PackageReadmeFile (root Readme.md packed as README.md)
- Add SourceLink (Microsoft.SourceLink.GitHub), deterministic build,
  CI build flag, symbol package (.snupkg), embedded untracked sources
- Refresh Description/ReleaseNotes/Tags/Copyright for v3
- appveyor: bump build version label to 3.0, capture .snupkg artifact,
  keep deploy: off

dotnet pack -c Release produces PasswordGenerator.3.0.0.nupkg + .snupkg with
no NU5048 / no missing-readme warnings; 55/55 tests still pass.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd

* docs: v2->v3 migration guide, standards mapping, readme refresh

- Add docs/v3-target/migration-v2-to-v3.md: error-string->exception/TryNext
  breaking change, opt-in DI/async/batch, OWASP/NIST preset mapping, and the
  broader use cases (OTP/API key/identifier/passphrase)
- Add root CHANGELOG.md with the 3.0.0 entry
- Refresh root Readme.md: fix the stale 8-128 length claim (actual 4-256),
  replace ```javascript fences with ```csharp, document presets, quality
  controls, error handling, async/batch and DI; link docs + changelog
- Mark current-state/ docs as historical (issues resolved in v3) and link the
  migration guide from docs/README.md
- Back Readme/migration snippets with DocumentationSnippetTests

59/59 tests pass.

https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd

---------

Co-authored-by: Claude <noreply@anthropic.com>
* docs: reconcile v3-target design docs with shipped v3 API

The v3-target docs were written as a pre-implementation proposal and
described API that diverged from what shipped (IPasswordBuilder/Build(),
Generate().Count(n), [Obsolete] sync methods, IRandomSource.Fill, net10.0,
and a "Special" appSettings key). Update them to match the released v3.0.0
surface: IPassword as the fluent builder, static Password presets,
parameterless Generate(), sync kept and not obsoleted, and the correct
"SpecialCharacters" option key. Also drop the "not yet implemented"
framing now that v3 has shipped.

https://claude.ai/code/session_01FshNNNTHNHupkfMJVLmotu

* ci: migrate from AppVeyor to GitHub Actions

Add a combined CI workflow (build + test + pack on push to master and on
every pull request, uploading test results and the .nupkg/.snupkg) and a
Release workflow that publishes to NuGet.org when a GitHub Release is
published. The release tag drives the published package version, and the
.snupkg symbol package is pushed alongside the main package. Remove the
AppVeyor configuration it replaces.

Requires a NUGET_API_KEY repository secret for the publish step.

https://claude.ai/code/session_01FshNNNTHNHupkfMJVLmotu

---------

Co-authored-by: Claude <noreply@anthropic.com>
* Add v3.0.0 local NuGet package test report

Documents an end-to-end consumer test of the PasswordGenerator 3.0.0
package built from dev/v3: pack, local feed registration, install into a
fresh console app, and 12 usage scenarios with the code added, the actual
output, and whether each result was expected.

https://claude.ai/code/session_01Hk3he4x9TKXfuopS3caYKE

* Ignore PasswordGenerator/bin build output

Broaden the bin ignore from bin/Debug to the whole bin folder so Release
pack artifacts are not tracked.

https://claude.ai/code/session_01Hk3he4x9TKXfuopS3caYKE

---------

Co-authored-by: Claude <noreply@anthropic.com>
* Add BenchmarkDotNet pipeline with GitHub Actions summary

Expand the benchmark project into structured scenarios (single, batch,
async, presets, DI-vs-direct, and a v3 batch-vs-loop comparison), all
with memory diagnostics. Multi-target net8.0/net10.0 and emit GitHub
markdown reports via the default exporter.

Add a Benchmarks workflow that runs on push/dispatch, publishes the
report tables to the job summary, and uploads the raw results.

The planned v2-vs-v3 comparison via a second PasswordGenerator package
reference is omitted: v2 and v3 both produce PasswordGenerator.dll and
collide in one bin folder (extern alias fails with CS0430), so the
comparison instead measures v3 usage patterns.

https://claude.ai/code/session_01NZJPrT8QFtXsMG6JJ5AJfv

* Target net10.0 and benchmark across both runtimes

Add net10.0 to the package's target frameworks (the existing
NET8_0_OR_GREATER conditionals cover it) and to the CI/release SDK setup
so the new target builds and packs.

Upgrade BenchmarkDotNet to 0.15.8 (0.14.0 has no .NET 10 moniker) and add
.NET 8 and .NET 10 runtime jobs so every benchmark runs on both, with a
Runtime column comparing them side by side in one report.

https://claude.ai/code/session_01NZJPrT8QFtXsMG6JJ5AJfv

* Add v3.0.0 benchmark results for .NET 8 and .NET 10

Record a sample BenchmarkDotNet run (both runtimes) under benchmarks/ as a
readable historical reference. Reduced-sampling numbers; the workflow
publishes full-precision results to the run summary.

https://claude.ai/code/session_01NZJPrT8QFtXsMG6JJ5AJfv

---------

Co-authored-by: Claude <noreply@anthropic.com>
* Update non-shipped test/benchmark packages and SourceLink to latest

Bump test tooling (NUnit, NUnit3TestAdapter, Test.Sdk) and the
Microsoft.Extensions.* packages used only by the test and benchmark
projects to their latest stable versions, plus SourceLink in the
library (build-only, PrivateAssets=All).

The shipped library's Microsoft.Extensions.* dependencies stay pinned
at 8.0.0 on purpose: for a multi-target library those versions are a
minimum floor imposed on consumers, so keeping the LTS floor maximizes
compatibility across netstandard2.0/net8.0/net10.0.

https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL

* Correct SourceLink.GitHub to latest stable 10.0.300

11.0.100 is preview-only and does not exist as a stable release, so
restore failed. Pin to the actual latest stable, 10.0.300.

https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL

* Drop netstandard2.0 from v3; make v3 docs the current docs

Target net8.0;net10.0 only and remove the netstandard2.0 fallback in
CryptoRandomSource (the #if rejection-sampling path, the RNG field, and
IDisposable), which are no longer needed now that GetInt32 is available
on every target.

Restructure docs so v3 is the current state: flatten the former
v3-target/* into docs/ (present tense, no "target/future" framing) and
move the v2.1.0 snapshot, review/verification docs, and v3 planning docs
into docs/archive/ with historical banners noting netstandard2.0 was
dropped. Update Readme, CHANGELOG, migration guide, and cross-links.

https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL

* Run tests against both net8.0 and net10.0

Multi-target the test project so the suite exercises the library on both
shipped target frameworks rather than net8.0 only.

https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL

---------

Co-authored-by: Claude <noreply@anthropic.com>
* Ship XML documentation file in NuGet package

Enable GenerateDocumentationFile so the heavily-documented public API
provides IntelliSense to consumers of the package.

https://claude.ai/code/session_01AikHWpKd7zQCfPDJcE2ZkX

* Use ValueTask and cancelled-task semantics for async APIs

Switch NextAsync/GenerateAsync to ValueTask<T> so the synchronous
completion path allocates no Task, and surface cancellation as a
cancelled task (ValueTask.FromCanceled) instead of throwing
synchronously, so the result composes correctly when not awaited
immediately. Argument validation still throws synchronously, matching
BCL conventions.

https://claude.ai/code/session_01AikHWpKd7zQCfPDJcE2ZkX

* Fix benchmark build against ValueTask async APIs and silence CS1591

The async APIs now return ValueTask, so the AsyncBenchmarks methods must
return ValueTask too (the Task return types no longer compile on either
target framework). Add CS1591 to NoWarn so enabling the XML doc file does
not warn on intentionally-undocumented public members.

https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL

* Document the public API instead of suppressing CS1591

With the XML doc file enabled, add XML comments to every public type and
member so the shipped documentation is complete and CS1591 no longer
warns. Interface implementations use <inheritdoc/>; the NoWarn CS1591
suppression added earlier is removed.

https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL

* Bump GitHub Actions to Node 24 runtimes

The v4 majors of checkout, setup-dotnet, and upload-artifact run on the
deprecated Node.js 20 runner. Bump to checkout@v6, setup-dotnet@v5, and
upload-artifact@v7, which run on Node.js 24. Inputs used here are
unchanged across these majors.

https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL

---------

Co-authored-by: Claude <noreply@anthropic.com>
* Use EFF Large Wordlist for passphrases

Replace the 390-word built-in list with the EFF Large Wordlist (7,776
words, ~12.9 bits/word), raising a 6-word passphrase to ~77 bits. Add
CC BY 3.0 attribution (WordList.cs header, THIRD-PARTY-NOTICES.md,
Readme, CHANGELOG), an InternalsVisibleTo for the test project, and
word-list integrity tests. Switch the passphrase smoke test to a '.'
separator since a few EFF words are hyphenated.

https://claude.ai/code/session_01Y5PCRCxt2rJFBLnbaQdQRB

* Add entropy-targeted passphrases and an entropy floor

Add ForPassphraseWithEntropy(targetBits) which derives the word count
needed to reach a target and enforces it, plus an optional
minimumEntropyBits floor on ForPassphrase that rejects weak configs.
Expose PassphraseGenerator.WordCountForEntropy and surface
EstimateEntropyBits on the IPasswordGenerator interface.

https://claude.ai/code/session_01Y5PCRCxt2rJFBLnbaQdQRB

* Add optional symbol injection for composition rules

Add includeSymbol to the passphrase generator and factories: a random
symbol is attached to one randomly chosen word so phrases satisfy
"needs a number and a symbol" validators while staying memorable.
Entropy estimation now accounts for the symbol's value and position.
Document that some EFF words are hyphenated, so callers tokenizing the
output should use a separator that does not appear in any word.

https://claude.ai/code/session_01Y5PCRCxt2rJFBLnbaQdQRB

* Wire passphrases through DI and add ForMemorable preset

Add PassphraseOptions and a PasswordOptions.Passphrase property so the
DI registration can resolve a passphrase IPasswordGenerator, in code or
bound from a "Passphrase" configuration section. Add a ForMemorable()
preset (capitalized words sized to at least 80 bits).

https://claude.ai/code/session_01Y5PCRCxt2rJFBLnbaQdQRB

* Add deterministic passphrase tests and update design docs

Add seeded-RNG tests covering word selection by index, capitalization,
symbol placement, plus a broad-sampling distribution check. Update
api-surface.md and architecture.md to reflect the EFF Large Wordlist,
entropy targeting/floor, symbol injection, EstimateEntropyBits on the
interface, and passphrase configuration through DI.

https://claude.ai/code/session_01Y5PCRCxt2rJFBLnbaQdQRB

---------

Co-authored-by: Claude <noreply@anthropic.com>
@prjseal prjseal closed this May 25, 2026
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.

2 participants