Skip to content

Move programmability sample handlers out of its constructor - #8489

Merged
Amaury Chamayou (achamayou) merged 12 commits into
mainfrom
achamayou-studious-guide
Oct 5, 2026
Merged

Amaury Chamayou (achamayou) merged 12 commits into
mainfrom
achamayou-studious-guide

Conversation

@achamayou

@achamayou Amaury Chamayou (achamayou) commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Motivation

Part of #7358. The ProgrammabilityHandlers constructor in samples/apps/programmability/programmability.cpp had a cognitive complexity of 55, almost entirely from its eight inline endpoint handlers.

The independent node frontend refactor is now in #8504. This PR changes only the programmability sample.

Implementation summary

  • Each [this] handler lambda's body becomes a private member function with the same name and parameter list, registered via a thin forwarding lambda (e.g. auto put = [this](Ctx& ctx) { this->put(ctx); };); the one capture-less handler (post) becomes a private static member function instead, forwarded via ProgrammabilityHandlers::post(ctx). No naming clashes with base-class members were found, so no renaming was needed.
  • Complexity: constructor 55 -> 5; new member functions range 1-10, all well under the threshold of 50.
  • Verified as a pure reshuffle: old lambda bodies and new function bodies were diffed with comments/whitespace stripped and zero expected edits, and the file outside the class confirmed byte-identical; all matched.
  • Removed the redundant handler-extraction comments raised in review.
  • Local validation after removing the node frontend changes: built programmability/logging/js_generic, passed the complete programmability_and_jwt e2e test, and passed all 15 scripts/ci-checks.sh checks without autofix.

Safety and compatibility

Pure refactor of a sample application with no behaviour change; no API, KV, consensus, or data-format changes.

Move the three endpoint handler bodies (get_state_digest,
update_state_digest, ack_state_digest) out of the lambdas in
init_ack_handlers() into named function templates in a nested detail
namespace, registered via thin forwarding lambdas. This removes all
the cognitive complexity from init_ack_handlers() (now 0), which
previously inherited it from its inline lambda bodies (up to 69
combined, mostly from ack_state_digest at ~34 standalone), letting the
readability-function-cognitive-complexity NOLINTNEXTLINE be dropped.

Pure reshuffle: handler bodies are token-identical to the former
lambda bodies (captures become explicit parameters where needed,
e.g. ShareManager& for ack_state_digest); registration call sites,
paths, verbs, adapters, auth policies, and install() chains are
unchanged.

Part of #7358. Layer 1 of a stack of refactors applying this same
pattern to the other init_*_handlers functions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract each of the 13 endpoint handler lambdas in
init_service_state_handlers() into named functions in a detail
namespace, following the convention established for init_ack_handlers()
in acks.h. The registration function keeps thin forwarding lambdas,
so each handler's cognitive complexity is measured on its own instead
of being rolled up into one large function.

Pure reshuffle: handler bodies and endpoint registration (paths,
verbs, adapters, auth policies, chained set_* calls, install() calls,
registration order) are unchanged.

Removes the NOLINTNEXTLINE(readability-function-cognitive-complexity)
suppression, since the registration function's own complexity is now
effectively 0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move the seven endpoint handler bodies (create_proposal,
withdraw_proposal, get_proposal, list_proposals, get_actions,
submit_ballot, get_ballot) out of the lambdas in
init_proposals_handlers() into named functions in a nested detail
namespace, registered via thin forwarding lambdas. This removes all
the cognitive complexity from init_proposals_handlers() (now 0),
which previously inherited it from its inline lambda bodies (127
combined, mostly from create_proposal at 29 and submit_ballot at 23
standalone), letting the readability-function-cognitive-complexity
NOLINTNEXTLINE be dropped.

submit_ballot is not templated on Ctx, unlike the other six: its
original lambda took a concrete ccf::endpoints::EndpointContext&, so
its body relies on non-dependent name lookup that a template
parameter would turn into dependent names requiring '.template'
disambiguators.

Pure reshuffle: handler bodies are token-identical to the former
lambda bodies (captures become explicit parameters where needed,
e.g. NetworkState& and AbstractNodeContext& for create_proposal and
submit_ballot); registration call sites, paths, verbs, adapters, auth
policies, and install() chains are unchanged.

Part of #7358. Layer 3 of a stack of refactors applying this same
pattern to the other init_*_handlers functions, on top of #8486.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
init_file_serving_handlers() in file_serving_handlers.h had a cognitive
complexity of 66, entirely from its 4 inline handler lambdas
(find_snapshot, find_chunk, get_snapshot, get_ledger_chunk). Move each
lambda's body into a named function in a ccf::node::detail namespace,
keeping thin forwarding lambdas in init_file_serving_handlers() for
registration. Remove the now-unneeded NOLINTNEXTLINE.

fill_range_response_from_file() is untouched; its own complexity is
handled separately.

Moving find_chunk, get_snapshot and get_ledger_chunk out of their
lambdas exposed 3 pre-existing trailing `return;` statements as
flagged by readability-function-cognitive-complexity's sibling check
readability-redundant-control-flow (which, like
bugprone-unchecked-optional-access, does not look inside lambda
bodies). These are removed as a minimal, behaviour-preserving fix.

Part of #7358

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The ProgrammabilityHandlers constructor in programmability.cpp had a
cognitive complexity of 55, entirely from its 8 inline [this] handler
lambdas (put, get, post, put_custom_endpoints, get_custom_endpoints,
get_custom_endpoints_module, patch_runtime_options,
get_runtime_options). Move each lambda's body into a private member
function of ProgrammabilityHandlers with the same name, parameter
list and (for the 7 that capture [this]) implicit access to the
registry's inherited members, keeping thin forwarding lambdas in the
constructor for registration.

post is the only handler with no captures (it doesn't use `this`),
so it becomes a private static member function instead of an
instance one, and its forwarding lambda calls it via the qualified
ProgrammabilityHandlers::post(ctx) rather than this->post(ctx). All
other forwarders call this->name(ctx); none of the chosen names
clash with members of DynamicJSEndpointRegistry or its bases, so no
renaming was needed. This mirrors the free-function convention used
in earlier layers of this stack (named functions registered via
forwarding lambdas), adapted to a member-function registry.

Pure reshuffle: handler bodies are token-identical to the former
lambda bodies; registration call sites, paths, verbs, auth policies,
schemas, and install() chains are unchanged. The small capture-less
lambda passed inline to set_js_kv_namespace_restriction(), and the
pre-existing private helpers (try_get_user_id, get_action_content,
set_error_details), are untouched.

This drops the constructor's complexity from 55 to 5; the moved
functions range from 1 to 10, well under the 50 threshold, so the
NOLINTNEXTLINE(readability-function-cognitive-complexity) is
removed.

Part of #7358. Layer 5 of a stack of refactors applying this same
pattern to the other init_*_handlers / constructor functions, on
top of #8488.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Amaury Chamayou (achamayou) added a commit that referenced this pull request Oct 2, 2026
Refactor ccf::NodeEndpoints::init_handlers() in src/node/rpc/node_frontend.h
following the member-function convention from #8489: each [this] handler
lambda body becomes a private member function (templated where the lambda
was generic), with a thin forwarding lambda left in place for registration.
Capture-less handlers (version, service_previous_identity) become private
static member functions. No naming clashes were found against
CommonEndpointRegistry/BaseEndpointRegistry/EndpointRegistry or free
functions, so all 32 handlers keep their original names.

Removes the NOLINTNEXTLINE(readability-function-cognitive-complexity) above
init_handlers(). Cognitive complexity drops from 266 (all contributed by the
24 inline handler lambdas) to effectively 0 for the new init_handlers(); the
largest extracted function (accept) measures 38, well under the threshold
of 50.

Non-move changes, all required because some clang-tidy checks skip lambda
bodies but apply once the code is a real named function:
- 20 unnamed trailing nlohmann::json&& parameters given a /*params*/ comment
  name (readability-named-parameter).
- 5 NOLINT(bugprone-unchecked-optional-access) added at pre-existing unchecked
  optional accesses now visible to the checker; placed as NOLINTNEXTLINE
  immediately above the flagged expression to match clang-tidy's diagnostic
  location for these multi-line statements.
- 2 redundant trailing `return;` statements removed from get_ready_app and
  get_ready_gov (readability-redundant-control-flow), matching the precedent
  set in file_serving_handlers.h.

Verified as a pure reshuffle with a script that strips comments/whitespace
from each old lambda body and new function body and requires exact equality
after applying only the explicit edit list above; all 33 units match.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Base automatically changed from achamayou-curly-tribble to main October 2, 2026 17:41
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 2, 2026 20:03

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.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 Medium severity · 2 Low severity

Open (3)
What changed in this PR

Refactors multiple RPC handler registration sites to reduce cognitive complexity by moving inline lambdas into named helper functions (and using thin forwarding lambdas), including the programmability sample handlers mentioned in the PR title.

Changes:

  • Refactored NodeEndpoints::init_handlers() by extracting many inline endpoint lambdas into private member functions.
  • Refactored file-serving endpoint registration by extracting handler bodies into ccf::node::detail::* functions.
  • Refactored several governance handler registries (service_state, proposals, acks) similarly by introducing detail::* handler functions, plus updated the programmability sample.
File Description
src/​node/​rpc/​node_frontend.h Extracts node RPC handler bodies from init_handlers() into class member functions.
src/​node/​rpc/​file_serving_handlers.h Extracts file-serving handler bodies into detail helpers and forwards from registration function.
src/​node/​gov/​handlers/​service_state.h Extracts service-state handler bodies into detail helpers and forwards from registry init.
src/​node/​gov/​handlers/​proposals.h Extracts proposals/ballots handler bodies into detail helpers and forwards from registry init.
src/​node/​gov/​handlers/​acks.h Extracts ack handler bodies into detail helpers and forwards from registry init.
samples/​apps/​programmability/​programmability.cpp Extracts inline endpoint lambda bodies into member/static functions and forwards from constructor.

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

Comment thread src/node/rpc/node_frontend.h Outdated
Comment thread src/node/rpc/file_serving_handlers.h
Comment thread src/node/rpc/node_frontend.h Outdated
@achamayou

Copy link
Copy Markdown
Member Author

Copilot resolve the merge conflicts in this pull request

# Conflicts:
#	src/node/gov/handlers/acks.h

Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>

Copilot AI commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Merged main and resolved the conflict in acks.h. Merge commit: 48b9e6cf2.

Comment thread src/node/gov/handlers/acks.h Outdated
Comment thread src/node/rpc/node_frontend.h
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@achamayou
Amaury Chamayou (achamayou) merged commit f2f0203 into main Oct 5, 2026
12 checks passed
@achamayou
Amaury Chamayou (achamayou) deleted the achamayou-studious-guide branch October 5, 2026 13:14
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.

5 participants