Skip to content

docs: clarify swap-state return value is pre-update state - #193

Closed
felipeofdev-ai wants to merge 383 commits into
nubank:masterfrom
felipeofdev-ai:docs/141-clarify-swap-state-return
Closed

felipeofdev-ai wants to merge 383 commits into
nubank:masterfrom
felipeofdev-ai:docs/141-clarify-swap-state-return

Conversation

@felipeofdev-ai

Copy link
Copy Markdown

Summary

Fixes #141

Why

Readers expected swap-state to return the new state (like clojure.core/swap!). The walkthrough already explains the cats State monad behavior; the public API docstring did not.

Test plan

  • Docstring-only change — no runtime behavior change
  • lein test / project CI green

— Felipe Fernandes · Systems & Agentic AI Engineer
https://github.com/felipeofdev-ai · https://felipeofdev-ai.github.io/

dchelimsky and others added 30 commits February 13, 2020 10:16
Introduce new match? fn in a new namespace and deprecate old one
…one" (nubank#85)

* Revert "Introduce new match? fn in a new namespace and deprecate old one"

* bump version
This makes them both work more like e.g. swap, update, etc
Enhance state/modify and state/gets to accept varargs (and apply args to fn)
Also remove obsolete comment
Introduce new match? fn in a new namespace and deprecate old one (take 2)
Defaults assigned by :or don't end up in the map bound to :as
when destructuring.

https://clojure.atlassian.net/browse/CLJ-2559
kdmoreira and others added 19 commits December 13, 2024 11:42
* Bump libraries

* Update CHANGELOG.md

* Fix typo

* Update CHANGELOG.md


* Bump library version in project.clj

* Setup leiningen on CI

* Bump lein

* Setup leiningen on release workflow

---------
Always log failures to stdout, removing timbre (and some other transitive deps) as a dependency.

Printing to stdout is the default behavior for timbre, so for users that do not configure it explicitly no change is expected (other than a small format difference).

Exceptions are still printed using pretty, so the output for stack traces is exactly the same.

Output before:
```
2025-01-15T19:57:07.903Z marco.local INFO [state-flow.core:241] - Flow "root (core_test.clj:26) -> child2 (core_test.clj:28) -> bogus" failed with exception
             clojure.core/with-bindings*       core.clj: 1990 (repeats 2 times)
                                     ...
                       clojure.core/eval       core.clj: 3232
                          user/eval16619     REPL Input:
                  clojure.test/run-tests       test.clj:  768 (repeats 2 times)
                                     ...
java.lang.Exception: My exception
```

Output after:
```
Flow "root (core_test.clj:26) -> child2 (core_test.clj:28) -> bogus" failed with exception
             clojure.core/with-bindings*       core.clj: 1990 (repeats 2 times)
                                     ...
                       clojure.core/eval       core.clj: 3232
                          user/eval16619     REPL Input:
                  clojure.test/run-tests       test.clj:  768 (repeats 2 times)
                                     ...
java.lang.Exception: My exception
```

Classpath difference:
```
|                     :dep | :upstream | :head |
|--------------------------+-----------+-------|
|      com.taoensso/encore |   3.128.0 |       |
|      com.taoensso/timbre |     6.6.1 |       |
|       com.taoensso/truss |    1.12.0 |       |
| org.clojure/tools.reader |     1.5.0 |       |
|   org.clj-commons/pretty |     3.2.0 | 3.3.0 |
```
It also sets the meta value which was the only thing that was being done
before
…-in-api

Use setMacro to flag macros in the api namespace
Support missing destructuring scenarios in mlet for version 2.4.3
…ings

Allow `format` for a flow description
Include a System/nanoTime timestamp in every match report so that
assertion results accumulated in state metadata can be ordered and
correlated with other trace events.
Document that the flow return value is the unmodified pre-update
state, while subsequent steps see the updated state. Matches the
walkthrough and resolves the confusion reported in nubank#140.

Fixes nubank#141

Signed-off-by: Felipe Fernandes <felipe.of.dev@gmail.com>
Copilot AI review requested due to automatic review settings August 3, 2026 21:48
@felipeofdev-ai
felipeofdev-ai requested a review from a team as a code owner August 3, 2026 21:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Not ready to approve

The new equivalence examples omit & args semantics (suggesting a single-arg “setter”), which can still mislead users who call swap-state with additional arguments.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR updates the public documentation for swap-state/modify to clarify that the flow’s return value is the pre-update state, while the updated state is only visible to subsequent steps—aligning the API docstring with the walkthrough and resolving the confusion reported in #140 / #141.

Changes:

  • Expanded state-flow.state/modify docstring to explain pre-update return value semantics.
  • Updated state-flow.api/swap-state doc metadata to mirror the clarified semantics.
File summaries
File Description
src/state_flow/state.clj Clarifies modify docstring semantics about return value vs updated state.
src/state_flow/api.clj Aligns swap-state public API docstring with the clarified semantics.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread src/state_flow/api.clj Outdated

(def ^{:arglists '([f & args]),
:doc "Creates a flow that replaces state with the result of applying f to\nstate with any additional args."}
:doc "Creates a flow that replaces state with the result of applying f to\nstate with any additional args.\n\nThe flow's *return value* is the unmodified (pre-update) state; the\nupdated state is only visible to subsequent steps. Equivalently:\n\n (swap-state setter) => (fn [s] [s (setter s)])"}
Comment thread src/state_flow/state.clj Outdated
Comment on lines +90 to +93
The flow's *return value* is the unmodified (pre-update) state; the
updated state is only visible to subsequent steps. Equivalently:

(swap-state setter) => (fn [s] [s (setter s)])"
@felipeofdev-ai

Copy link
Copy Markdown
Author

Addressed Copilot feedback: equivalence examples now use (swap-state f & args) => (fn [s] [s (apply f s args)]) / (modify f & args) so multi-arg callers (e.g. update :k inc) are not misled by a single-arg setter sketch.

@felipeofdev-ai

Copy link
Copy Markdown
Author

Follow-up pushed: Copilot & args feedback addressed — equivalence examples now use (swap-state f & args) => (fn [s] [s (apply f s args)]) / (modify f & args). Nu workflow processing errors remain outside this PR's control.

@felipeofdev-ai

Copy link
Copy Markdown
Author

Copilot’s apply/& args docstring note is already covered on this branch (3a08532): both swap-state and modify document (f & args) => (fn [s] [s (apply f s args)]).

— Felipe Fernandes · Systems & Agentic AI Engineer
https://github.com/felipeofdev-ai · https://felipeofdev-ai.github.io/

@felipeofdev-ai
felipeofdev-ai force-pushed the docs/141-clarify-swap-state-return branch from 3a08532 to 81bc264 Compare August 4, 2026 02:54
@felipeofdev-ai
felipeofdev-ai force-pushed the docs/141-clarify-swap-state-return branch from 81bc264 to 32f5c01 Compare August 4, 2026 03:13
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.

Clarify the doc for swap-state