)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"0e372c1363a62ad29cea6deb46d3e7d197ea8930","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"82079567_dc1b492e","updated":"2026-06-30 13:50:23.000000000","message":"the proposal is sound and well explained imo, I only spotted a couple of minor things to clear up","commit_id":"49db5f8940c297fec7af47612746428d71d2a843"}],"specs/2026.2/approved/action-plan-transformers.rst":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e08a82f0444ffaa337b02fcc8e296faba7d4963a","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":8,"context_line":"Action Plan Transformers Framework"},{"line_number":9,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"https://blueprints.launchpad.net/watcher/+spec/action-plan-transformers"},{"line_number":12,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"f1ffc271_0e38080d","line":9,"updated":"2026-06-24 09:12:55.000000000","message":"Several RST section title underlines are 1-2 characters longer than their title text. Affected sections: line 9 (title 34 chars, underline 36), line 187 (31 vs 33), line 232 (27 vs 28), line 380 (38 vs 39), line 428 (34 vs 36).\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Matching underline lengths to title text is clean RST practice. While Sphinx tolerates over-long underlines, exact matching avoids potential warnings and is the convention used throughout the rest of this spec.\n\n**Recommendation**:\nTrim each mismatched underline to exactly match the title length above it. For example, line 9 should be 34 equals characters, line 187 should be 31 dash characters, etc.","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"9b1a14ce1cf39a2616442b6072290a67c3d61d4c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":8,"context_line":"Action Plan Transformers Framework"},{"line_number":9,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"https://blueprints.launchpad.net/watcher/+spec/action-plan-transformers"},{"line_number":12,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"c230f4ac_3ab30cec","line":9,"in_reply_to":"f1ffc271_0e38080d","updated":"2026-06-24 09:38:28.000000000","message":"done","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e08a82f0444ffaa337b02fcc8e296faba7d4963a","unresolved":false,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"This spec also introduces the **composable planner** architecture: an"},{"line_number":88,"context_line":"abstract base class ``ComposablePlanner`` derived from ``BasePlanner``"},{"line_number":89,"context_line":"and implementa **all** the ordering and dependency setting logic to"},{"line_number":90,"context_line":"a transformer chain. Concrete subclasses define their specific transformer"},{"line_number":91,"context_line":"chain by overriding a ``transformers`` property. Each subclass is"},{"line_number":92,"context_line":"registered as a stevedore planner plugin, and different strategies can"}],"source_content_type":"text/x-rst","patch_set":1,"id":"b8c18791_fe53c02b","line":89,"updated":"2026-06-24 09:12:55.000000000","message":"Grammar error on line 89: \u0027and implementa all the ordering and dependency setting logic to a transformer chain\u0027 contains a misspelling. \u0027implementa\u0027 should be \u0027implements\u0027, and \u0027to\u0027 should likely be \u0027via\u0027 or \u0027through\u0027 for clarity.\n\n**Severity**: WARNING | **Confidence**: 1.0\n\n**Impact**: Spelling and grammar errors in a specification document reduce readability and professional polish, and can create ambiguity about intended meaning.\n\n**Suggestion**:\nChange to \u0027and implements all of the ordering and dependency-setting logic through a transformer chain.\u0027","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"9b1a14ce1cf39a2616442b6072290a67c3d61d4c","unresolved":false,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"This spec also introduces the **composable planner** architecture: an"},{"line_number":88,"context_line":"abstract base class ``ComposablePlanner`` derived from ``BasePlanner``"},{"line_number":89,"context_line":"and implementa **all** the ordering and dependency setting logic to"},{"line_number":90,"context_line":"a transformer chain. Concrete subclasses define their specific transformer"},{"line_number":91,"context_line":"chain by overriding a ``transformers`` property. Each subclass is"},{"line_number":92,"context_line":"registered as a stevedore planner plugin, and different strategies can"}],"source_content_type":"text/x-rst","patch_set":1,"id":"77b41348_e1f20623","line":89,"in_reply_to":"b8c18791_fe53c02b","updated":"2026-06-24 09:38:28.000000000","message":"done","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e08a82f0444ffaa337b02fcc8e296faba7d4963a","unresolved":false,"context_lines":[{"line_number":151,"context_line":"    [watcher_planner]"},{"line_number":152,"context_line":"    # Global default planner. Used for any strategy that does not have"},{"line_number":153,"context_line":"    # a specific override below."},{"line_number":154,"context_line":"    planner \u003d weight"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    # Per-strategy planner overrides."},{"line_number":157,"context_line":"    # Format: \u003cstrategy_name\u003e_planner \u003d \u003cplanner_name\u003e"}],"source_content_type":"text/x-rst","patch_set":1,"id":"b03dad04_b65a657d","line":154,"updated":"2026-06-24 09:12:55.000000000","message":"Contradictory global planner option name. Planner Selection section (line 154) uses \u0027planner \u003d weight\u0027, but Other deployer impact (line 649) and Work Items (line 695) both call it \u0027default_planner\u0027. Same option, two different names.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: An implementer following this spec will not know which option name to use, leading to either a mismatch between documented and implemented behavior or a broken planner selection mechanism.\n\n**Priority**: Before merge\n**Why This Matters**: Configuration option names are part of the public API surface for operators. A contradictory spec will produce an implementation that does not match documentation, confusing operators and developers alike.\n\n**Recommendation**:\nPick one name and use it consistently across all sections (Planner Selection example, resolution order, Other deployer impact, Work Items). Recommend standardizing on \u0027default_planner\u0027 since it is less ambiguous, and update lines 145, 154, and 164-165 accordingly.","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"9b1a14ce1cf39a2616442b6072290a67c3d61d4c","unresolved":false,"context_lines":[{"line_number":151,"context_line":"    [watcher_planner]"},{"line_number":152,"context_line":"    # Global default planner. Used for any strategy that does not have"},{"line_number":153,"context_line":"    # a specific override below."},{"line_number":154,"context_line":"    planner \u003d weight"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    # Per-strategy planner overrides."},{"line_number":157,"context_line":"    # Format: \u003cstrategy_name\u003e_planner \u003d \u003cplanner_name\u003e"}],"source_content_type":"text/x-rst","patch_set":1,"id":"820e6e68_80403aaf","line":154,"in_reply_to":"b03dad04_b65a657d","updated":"2026-06-24 09:38:28.000000000","message":"done","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e08a82f0444ffaa337b02fcc8e296faba7d4963a","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    # Per-strategy planner overrides."},{"line_number":157,"context_line":"    # Format: \u003cstrategy_name\u003e_planner \u003d \u003cplanner_name\u003e"},{"line_number":158,"context_line":"    zone_migration_planner \u003d composable"},{"line_number":159,"context_line":"    workload_stabilization_planner \u003d composable"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"The resolution order for which planner to use is:"}],"source_content_type":"text/x-rst","patch_set":1,"id":"4276fc32_cf6432dd","line":158,"updated":"2026-06-24 09:12:55.000000000","message":"Example config uses \u0027composable\u0027 as planner name (lines 158-159) but no planner with that name is registered. Stevedore entry points (lines 183-184) only register \u0027weight_composable\u0027 and \u0027interleave_composable\u0027.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: An operator following this example would configure a planner name that does not exist, causing planner loading to fail at runtime. The example is technically incorrect as written.\n\n**Priority**: Before merge\n**Why This Matters**: The example configuration in the Planner Selection section is the primary reference operators will use. If it references a non-existent planner name, it undermines trust in the entire spec and will cause deployment failures.\n\n**Recommendation**:\nReplace \u0027composable\u0027 in lines 158-159 and lines 171-172 with a real registered planner name (e.g., \u0027weight_composable\u0027 or \u0027interleave_composable\u0027), or add a \u0027composable\u0027 entry point to the registration listing at lines 178-184 if a generic composable planner is intended.","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"9b1a14ce1cf39a2616442b6072290a67c3d61d4c","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    # Per-strategy planner overrides."},{"line_number":157,"context_line":"    # Format: \u003cstrategy_name\u003e_planner \u003d \u003cplanner_name\u003e"},{"line_number":158,"context_line":"    zone_migration_planner \u003d composable"},{"line_number":159,"context_line":"    workload_stabilization_planner \u003d composable"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"The resolution order for which planner to use is:"}],"source_content_type":"text/x-rst","patch_set":1,"id":"3abd83fb_9cc20dcd","line":158,"in_reply_to":"4276fc32_cf6432dd","updated":"2026-06-24 09:38:28.000000000","message":"done","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e08a82f0444ffaa337b02fcc8e296faba7d4963a","unresolved":false,"context_lines":[{"line_number":524,"context_line":"``nova_service_state_lifecycle``"},{"line_number":525,"context_line":"  Inspects ``change_nova_service_state`` actions and, based on a"},{"line_number":526,"context_line":"  configurable target state parameter, adds parent dependencies to"},{"line_number":527,"context_line":"  ensure that those actions run after migrations affecting the same"},{"line_number":528,"context_line":"  host have completed. This allows expressing ordering constraints"},{"line_number":529,"context_line":"  within a single action type based on action parameters, which cannot"},{"line_number":530,"context_line":"  be achieved by ``weight_order`` alone since it groups by action type."}],"source_content_type":"text/x-rst","patch_set":1,"id":"045a5182_700e6abb","line":527,"updated":"2026-06-24 09:12:55.000000000","message":"The nova_service_state_lifecycle transformer description (lines 524-534) references a \u0027configurable target state parameter\u0027 but, unlike all three other built-in transformers, lacks a \u0027Configuration options:\u0027 subsection listing its options.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: The spec is incomplete for this transformer. Developers and operators will not know what configuration parameters it accepts or their defaults, making it inconsistent with the other transformer descriptions.\n\n**Suggestion**:\nAdd a \u0027Configuration options:\u0027 subsection after the nova_service_state_lifecycle description, documenting the target state parameter (e.g., \u0027target_state: the Nova service state that triggers the lifecycle ordering, such as disabled or enabled\u0027).","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"3c2bf50efcc0879610ec3451c47dc09e8bbe43d3","unresolved":false,"context_lines":[{"line_number":524,"context_line":"``nova_service_state_lifecycle``"},{"line_number":525,"context_line":"  Inspects ``change_nova_service_state`` actions and, based on a"},{"line_number":526,"context_line":"  configurable target state parameter, adds parent dependencies to"},{"line_number":527,"context_line":"  ensure that those actions run after migrations affecting the same"},{"line_number":528,"context_line":"  host have completed. This allows expressing ordering constraints"},{"line_number":529,"context_line":"  within a single action type based on action parameters, which cannot"},{"line_number":530,"context_line":"  be achieved by ``weight_order`` alone since it groups by action type."}],"source_content_type":"text/x-rst","patch_set":1,"id":"05440293_9fcaade1","line":527,"in_reply_to":"045a5182_700e6abb","updated":"2026-06-24 09:27:57.000000000","message":"Parameters of execution of nova_service_state_lifecycle would not depend on system-config parameters from file but on parameters used  when calling it from the planner. i.e. nova_service_state_lifecycle could be called with an option to move all the actions whose target state is enabled after all the migrate actions (that\u0027s what current node_resource_consolidation planner does). In other cases we may want to put at the end the disabled... Those be parameters in call-time, not static config file options.","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e08a82f0444ffaa337b02fcc8e296faba7d4963a","unresolved":false,"context_lines":[{"line_number":551,"context_line":"in the workload stabilization and node resource consolidation planners"},{"line_number":552,"context_line":"can be reimplemented as transformers (e.g., a ``resource_serialization``"},{"line_number":553,"context_line":"transformer that serializes actions affecting the same resource). However"},{"line_number":554,"context_line":"this logic is currently not used as the workload_stabilization strategy"},{"line_number":555,"context_line":"is only creation ``migrate`` actions, and not ``resize`` ones."},{"line_number":556,"context_line":""},{"line_number":557,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":1,"id":"fe050ae1_67fde979","line":554,"updated":"2026-06-24 09:12:55.000000000","message":"Grammar error on line 554: \u0027is only creation migrate actions\u0027 should be \u0027is only creating migrate actions\u0027. The word \u0027creation\u0027 is used as a verb but is a noun.\n\n**Severity**: WARNING | **Confidence**: 1.0\n\n**Impact**: Grammatical error reduces clarity of the explanation about why the resource_serialization transformer is not needed for workload_stabilization.\n\n**Suggestion**:\nChange \u0027is only creation\u0027 to \u0027is only creating\u0027. Full sentence: \u0027this logic is currently not used as the workload_stabilization strategy is only creating migrate actions, and not resize ones.\u0027","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"9b1a14ce1cf39a2616442b6072290a67c3d61d4c","unresolved":false,"context_lines":[{"line_number":551,"context_line":"in the workload stabilization and node resource consolidation planners"},{"line_number":552,"context_line":"can be reimplemented as transformers (e.g., a ``resource_serialization``"},{"line_number":553,"context_line":"transformer that serializes actions affecting the same resource). However"},{"line_number":554,"context_line":"this logic is currently not used as the workload_stabilization strategy"},{"line_number":555,"context_line":"is only creation ``migrate`` actions, and not ``resize`` ones."},{"line_number":556,"context_line":""},{"line_number":557,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":1,"id":"68c8e278_4dd748f5","line":554,"in_reply_to":"fe050ae1_67fde979","updated":"2026-06-24 09:38:28.000000000","message":"done","commit_id":"16df524c50eb8701776cda9b6673d8475b5c0f26"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"34441f25f94db10820ad6a39138b872546ff838c","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    class ComposablePlanner(BasePlanner):"},{"line_number":202,"context_line":"        \"\"\"Abstract base for transformer-based planners.\"\"\""},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        @abc.abstractproperty"},{"line_number":205,"context_line":"        def transformers(self):"},{"line_number":206,"context_line":"            \"\"\"Return ordered list of transformer plugin names.\"\"\""},{"line_number":207,"context_line":"            raise NotImplementedError()"}],"source_content_type":"text/x-rst","patch_set":2,"id":"d833a9a0_1b2ea45e","line":204,"updated":"2026-06-24 09:59:31.000000000","message":"abc.abstractproperty is deprecated since Python 3.3 and will be removed. The proposed ComposablePlanner code sample uses it for the \u0027transformers\u0027 property. Implementation should use @property combined with @abc.abstractmethod instead.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Using deprecated API in a spec\u0027s reference code sample sets a poor precedent for implementers. If copied verbatim, it may emit DeprecationWarnings and break on future Python versions where abstractproperty is removed.\n\n**Suggestion**:\nReplace \u0027@abc.abstractproperty\u0027 with stacked decorators \u0027@property\u0027 and \u0027@abc.abstractmethod\u0027 in the code sample to model current Python best practices.","commit_id":"49db5f8940c297fec7af47612746428d71d2a843"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"1cb07d56239f44d71f42de4898f1a6558a62a555","unresolved":true,"context_lines":[{"line_number":231,"context_line":"subclass at call time. These are developer-defined parameters that"},{"line_number":232,"context_line":"control the transformer\u0027s behavior as part of the chain\u0027s design — for"},{"line_number":233,"context_line":"example, sort direction or grouping mode. The exact mechanism for"},{"line_number":234,"context_line":"passing invocation parameters will be defined during implementation."},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"Composable Planner Workflow"},{"line_number":237,"context_line":"---------------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"f2fb72b6_b2d6cb4f","line":234,"updated":"2026-06-30 13:49:48.000000000","message":"even if we defer the decision to the implementation phase, we should consider the possibilities here. Off the top of my mind, I can think of two:\n1. Configuration options - this would scale badly if we want to have different paramters for different strategies\n2. Audit input parameters - this would have considerable implact on the strategies, but would be doable I think if we could reuse the parameters across strategies\n\nEDIT: It seems later https://review.opendev.org/c/openstack/watcher-specs/+/994607/2/specs/2026.2/approved/action-plan-transformers.rst#408 that it\u0027s suggested to use configuration options for this, am I understanding it correctly or am I mixing things up?","commit_id":"49db5f8940c297fec7af47612746428d71d2a843"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"0b06c0562103426e127f0b9ee3f370449c465a7d","unresolved":false,"context_lines":[{"line_number":231,"context_line":"subclass at call time. These are developer-defined parameters that"},{"line_number":232,"context_line":"control the transformer\u0027s behavior as part of the chain\u0027s design — for"},{"line_number":233,"context_line":"example, sort direction or grouping mode. The exact mechanism for"},{"line_number":234,"context_line":"passing invocation parameters will be defined during implementation."},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"Composable Planner Workflow"},{"line_number":237,"context_line":"---------------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"8e6eeefa_6aefe606","line":234,"in_reply_to":"c8f9cb3d_98608691","updated":"2026-07-01 10:24:38.000000000","message":"ok, I see, this more restricted in scope that I first understood, I\u0027m ok with this being decised in the implementation","commit_id":"49db5f8940c297fec7af47612746428d71d2a843"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"3dd80bef66d9156b7e840377cbfff11160692a05","unresolved":true,"context_lines":[{"line_number":231,"context_line":"subclass at call time. These are developer-defined parameters that"},{"line_number":232,"context_line":"control the transformer\u0027s behavior as part of the chain\u0027s design — for"},{"line_number":233,"context_line":"example, sort direction or grouping mode. The exact mechanism for"},{"line_number":234,"context_line":"passing invocation parameters will be defined during implementation."},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"Composable Planner Workflow"},{"line_number":237,"context_line":"---------------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"c8f9cb3d_98608691","line":234,"in_reply_to":"f2fb72b6_b2d6cb4f","updated":"2026-07-01 10:14:47.000000000","message":"We have two different cases:\n1. Config based parameters that would be always taken from configuration as shown in the **Configuration** section. I expect it to be the main config mechanism with defaults and the option to per-strategy override as described in the spec.\n2. Invocation parameters. There may be cases where we need that the behavior of a transformer depends on audit parameters or solution results. i.e. think in a case where we want to order the migrations depending on a criteria configurable via audit parameter. In that case, we need to call the transformer with different parameters depending on the audit params, etc...\n\n\nWhile initially I see mainly using [1], I\u0027d like the framework to support [2].\n\nThe way to store those parameters in the ComposablePlanner (i.e. via dict attribute in the ComposablePlanner) is what I think we can decide in the implementation, although, i probably should mention that `def transform(self, actions, solution)` may be `def transform(self, actions, solution, **kwargs):`.","commit_id":"49db5f8940c297fec7af47612746428d71d2a843"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"34441f25f94db10820ad6a39138b872546ff838c","unresolved":false,"context_lines":[{"line_number":536,"context_line":"  * ``max_parallel``: comma-separated ``type:count`` pairs specifying"},{"line_number":537,"context_line":"    the maximum number of concurrent actions per type."},{"line_number":538,"context_line":""},{"line_number":539,"context_line":"``nova_service_state_lifecycle``"},{"line_number":540,"context_line":"  Inspects ``change_nova_service_state`` actions and, based on a"},{"line_number":541,"context_line":"  configurable target state parameter, adds parent dependencies to"},{"line_number":542,"context_line":"  ensure that those actions run after migrations affecting the same"}],"source_content_type":"text/x-rst","patch_set":2,"id":"e41c40ec_94dabc1b","line":539,"updated":"2026-06-24 09:59:31.000000000","message":"nova_service_state_lifecycle is one of four built-in transformers but is not used by either proposed composable planner subclass. It reproduces node resource consolidation logic, yet no corresponding composable planner subclass is defined, leaving its use case without a delivery path.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: The transformer is implemented and registered but has no planner chain that uses it. This creates dead infrastructure on landing and leaves the stated goal of reproducing the node resource consolidation planner as composable transformers unfulfilled.\n\n**Suggestion**:\nEither add a third composable planner subclass whose chain includes nova_service_state_lifecycle, or explicitly state that this transformer is provided for out-of-tree planner subclasses and defer its built-in chain to a follow-up.","commit_id":"49db5f8940c297fec7af47612746428d71d2a843"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"34441f25f94db10820ad6a39138b872546ff838c","unresolved":false,"context_lines":[{"line_number":537,"context_line":"    the maximum number of concurrent actions per type."},{"line_number":538,"context_line":""},{"line_number":539,"context_line":"``nova_service_state_lifecycle``"},{"line_number":540,"context_line":"  Inspects ``change_nova_service_state`` actions and, based on a"},{"line_number":541,"context_line":"  configurable target state parameter, adds parent dependencies to"},{"line_number":542,"context_line":"  ensure that those actions run after migrations affecting the same"},{"line_number":543,"context_line":"  host have completed. This allows expressing ordering constraints"}],"source_content_type":"text/x-rst","patch_set":2,"id":"8c2a4dd4_d9b605b4","line":540,"updated":"2026-06-24 09:59:31.000000000","message":"nova_service_state_lifecycle mentions \u0027a configurable target state parameter\u0027 but omits the \u0027Configuration options\u0027 sub-section that all three sibling transformers include. The option name, format, and default value are undocumented.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Implementers and operators lack the information needed to configure or test this transformer\u0027s parameter. The inconsistency with sibling transformer descriptions makes the spec ambiguous at the point a developer would write the code.\n\n**Suggestion**:\nAdd a \u0027Configuration options\u0027 block for nova_service_state_lifecycle documenting the target state option name, accepted values, and default, matching the format used by the other three built-in transformers.","commit_id":"49db5f8940c297fec7af47612746428d71d2a843"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"34441f25f94db10820ad6a39138b872546ff838c","unresolved":false,"context_lines":[{"line_number":698,"context_line":"-----------"},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"Primary assignee:"},{"line_number":701,"context_line":"  \u003ctbd\u003e"},{"line_number":702,"context_line":""},{"line_number":703,"context_line":"Other contributors:"},{"line_number":704,"context_line":"  None"}],"source_content_type":"text/x-rst","patch_set":2,"id":"936c7ca1_d5c2bc9d","line":701,"updated":"2026-06-24 09:59:31.000000000","message":"The Assignee field is \u0027\u003ctbd\u003e\u0027. While acceptable for early spec review, an approved spec in the 2026.2 cycle should identify at least a primary assignee to signal implementation ownership and improve accountability.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: A named assignee clarifies who drives implementation, helps reviewers track progress, and aligns with the template\u0027s intent that a primary author or contact be designated.\n\n**Recommendation**:\nReplace \u0027\u003ctbd\u003e\u0027 with the launchpad ID of the primary implementer before or at final approval.","commit_id":"49db5f8940c297fec7af47612746428d71d2a843"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"1cb07d56239f44d71f42de4898f1a6558a62a555","unresolved":true,"context_lines":[{"line_number":698,"context_line":"-----------"},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"Primary assignee:"},{"line_number":701,"context_line":"  \u003ctbd\u003e"},{"line_number":702,"context_line":""},{"line_number":703,"context_line":"Other contributors:"},{"line_number":704,"context_line":"  None"}],"source_content_type":"text/x-rst","patch_set":2,"id":"b9a1998a_dfa9232c","line":701,"in_reply_to":"936c7ca1_d5c2bc9d","updated":"2026-06-30 13:49:48.000000000","message":"this is a good point. If we intend to merge the spec and work on this in the current cycle we need an assignee. If the goal is just to discuss the proposal and POC then it\u0027s ok as is for now","commit_id":"49db5f8940c297fec7af47612746428d71d2a843"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"262adb2a2dee6e0c34d96010b3ef33a709caf63c","unresolved":false,"context_lines":[{"line_number":698,"context_line":"-----------"},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"Primary assignee:"},{"line_number":701,"context_line":"  \u003ctbd\u003e"},{"line_number":702,"context_line":""},{"line_number":703,"context_line":"Other contributors:"},{"line_number":704,"context_line":"  None"}],"source_content_type":"text/x-rst","patch_set":2,"id":"7aeef1d5_a3ab1492","line":701,"in_reply_to":"b9a1998a_dfa9232c","updated":"2026-07-01 10:44:28.000000000","message":"Done","commit_id":"49db5f8940c297fec7af47612746428d71d2a843"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e349246758cf85e51e4d875bd9c35547a5bfef50","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"* Per-host parallel migration limits enforced by Nova."},{"line_number":33,"context_line":"* The need to drain specific hosts before others in maintenance scenarios."},{"line_number":34,"context_line":"* Grouping and ordering actions by instances characteristics"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Each time a new ordering or grouping requirement arises, the planner code"},{"line_number":37,"context_line":"must be modified or a new planner must be written. There is no way for an"}],"source_content_type":"text/x-rst","patch_set":3,"id":"43ee1995_94ccdab9","line":34,"updated":"2026-07-01 11:00:57.000000000","message":"Grammar error on line 34: \u0027Grouping and ordering actions by instances characteristics\u0027 should be \u0027instance characteristics\u0027 or \u0027instances\\\u0027 characteristics\u0027.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Grammatical error in the Problem description, one of the first sections a reader encounters. Minor but affects spec polish.\n\n**Suggestion**:\nChange \u0027instances characteristics\u0027 to \u0027instance characteristics\u0027 (preferred, using singular noun as modifier).","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"62e50ad7900b448814ff654f84b67be6beff534a","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"* Per-host parallel migration limits enforced by Nova."},{"line_number":33,"context_line":"* The need to drain specific hosts before others in maintenance scenarios."},{"line_number":34,"context_line":"* Grouping and ordering actions by instances characteristics"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Each time a new ordering or grouping requirement arises, the planner code"},{"line_number":37,"context_line":"must be modified or a new planner must be written. There is no way for an"}],"source_content_type":"text/x-rst","patch_set":3,"id":"30de5240_9421846e","line":34,"in_reply_to":"43ee1995_94ccdab9","updated":"2026-07-02 08:01:05.000000000","message":"Done","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e349246758cf85e51e4d875bd9c35547a5bfef50","unresolved":false,"context_lines":[{"line_number":81,"context_line":"dependencies between actions. Transformers are chained in a configurable"},{"line_number":82,"context_line":"order, and each receives the output of the previous one."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"A transformer does not add, remove, or modify actions; it"},{"line_number":85,"context_line":"only changes their order, grouping, or dependency relationships."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"This spec also introduces the **composable planner** architecture: an"}],"source_content_type":"text/x-rst","patch_set":3,"id":"686c2c98_34ffaac1","line":84,"updated":"2026-07-01 11:00:57.000000000","message":"Line 84-85 states \u0027A transformer does not add, remove, or modify actions\u0027 but the Transformer Contract section (lines 304-310) explicitly allows transformers to \u0027Set or modify parent dependencies\u0027. Modifying the parents field IS modifying the action object, creating a contradiction.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: The contradictory wording will confuse implementers about what transformers may change. A reader encountering line 84 first may believe transformers cannot touch any action field, then find the contract section says they can modify parents.\n\n**Suggestion**:\nReword line 84 to clarify scope: e.g., \u0027A transformer does not add, remove, or modify the semantic content of actions (action type, input parameters, target resource); it only changes their order, grouping, or dependency relationships.\u0027","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"62e50ad7900b448814ff654f84b67be6beff534a","unresolved":false,"context_lines":[{"line_number":81,"context_line":"dependencies between actions. Transformers are chained in a configurable"},{"line_number":82,"context_line":"order, and each receives the output of the previous one."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"A transformer does not add, remove, or modify actions; it"},{"line_number":85,"context_line":"only changes their order, grouping, or dependency relationships."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"This spec also introduces the **composable planner** architecture: an"}],"source_content_type":"text/x-rst","patch_set":3,"id":"e346d7ae_83af0ad6","line":84,"in_reply_to":"686c2c98_34ffaac1","updated":"2026-07-02 08:01:05.000000000","message":"Done","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e349246758cf85e51e4d875bd9c35547a5bfef50","unresolved":false,"context_lines":[{"line_number":154,"context_line":"    planner \u003d weight"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    # Per-strategy planner overrides."},{"line_number":157,"context_line":"    # Format: \u003cstrategy_name\u003e_planner \u003d \u003cplanner_name\u003e"},{"line_number":158,"context_line":"    zone_migration_planner \u003d weight_composable"},{"line_number":159,"context_line":"    workload_stabilization_planner \u003d interleave_composable"},{"line_number":160,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"4ef8b8d7_a88572c6","line":157,"updated":"2026-07-01 11:00:57.000000000","message":"The per-strategy planner override format \u003cstrategy_name\u003e_planner (line 157) is ambiguous for strategies with underscores in their names (e.g., workload_stabilization_planner could be strategy \u0027workload_stabilization\u0027 or \u0027workload\u0027 + \u0027stabilization_planner\u0027).\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Resolving the parsing ambiguity prevents configuration bugs where the wrong strategy name is extracted from a config key, leading to planner overrides being silently ignored or misapplied.\n\n**Recommendation**:\nClarify the parsing algorithm: the config loader should enumerate known strategy names from the watcher_strategies namespace and match the longest prefix ending in \u0027_planner\u0027, rather than naively splitting on the suffix.","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"62e50ad7900b448814ff654f84b67be6beff534a","unresolved":false,"context_lines":[{"line_number":154,"context_line":"    planner \u003d weight"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    # Per-strategy planner overrides."},{"line_number":157,"context_line":"    # Format: \u003cstrategy_name\u003e_planner \u003d \u003cplanner_name\u003e"},{"line_number":158,"context_line":"    zone_migration_planner \u003d weight_composable"},{"line_number":159,"context_line":"    workload_stabilization_planner \u003d interleave_composable"},{"line_number":160,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"b5c2edd5_0a08bde9","line":157,"in_reply_to":"4ef8b8d7_a88572c6","updated":"2026-07-02 08:01:05.000000000","message":"Actually, I don\u0027t see ambiguity given that the list of strategies is a set of predefined and discoverable names given that there are not other config param names finished in `_planner`. For each strategy from the list of declared plugins it\u0027s \u003cstrategy name\u003e_planner. In case adding a new option would lead to duplicated names, we\u0027d detect that in CI when registering options.","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e349246758cf85e51e4d875bd9c35547a5bfef50","unresolved":false,"context_lines":[{"line_number":206,"context_line":"            \"\"\"Return ordered list of transformer plugin names.\"\"\""},{"line_number":207,"context_line":"            raise NotImplementedError()"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        def schedule(self, context, audit_id, solution):"},{"line_number":210,"context_line":"            # Steps 1-6 as described below"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"42a7b890_9646380f","line":209,"updated":"2026-07-01 11:00:57.000000000","message":"The spec does not describe whether the existing planner interface (the schedule method signature) changes. The schedule signature is shown (line 209) but the spec does not confirm it matches the existing BasePlanner.schedule interface.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Clarifying interface compatibility helps developers understand whether ComposablePlanner is a drop-in subclass of BasePlanner or requires interface changes that could affect other consumers.\n\n**Recommendation**:\nAdd a sentence stating whether the schedule(self, context, audit_id, solution) signature is identical to the existing BasePlanner.schedule method, confirming backward compatibility of the interface.","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"62e50ad7900b448814ff654f84b67be6beff534a","unresolved":false,"context_lines":[{"line_number":206,"context_line":"            \"\"\"Return ordered list of transformer plugin names.\"\"\""},{"line_number":207,"context_line":"            raise NotImplementedError()"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        def schedule(self, context, audit_id, solution):"},{"line_number":210,"context_line":"            # Steps 1-6 as described below"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"2004480d_abb3dd63","line":209,"in_reply_to":"42a7b890_9646380f","updated":"2026-07-02 08:01:05.000000000","message":"Not indicating that this changes the existing Planner contract implies that it does not change it, imo.","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e349246758cf85e51e4d875bd9c35547a5bfef50","unresolved":false,"context_lines":[{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    class WeightComposablePlanner(ComposablePlanner):"},{"line_number":214,"context_line":"        \"\"\"Reproduces current weight planner behavior.\"\"\""},{"line_number":215,"context_line":"        transformers \u003d [\u0027weight_order\u0027, \u0027parallelization\u0027]"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    class InterleaveComposablePlanner(ComposablePlanner):"}],"source_content_type":"text/x-rst","patch_set":3,"id":"20c0e170_9a4793c5","line":215,"updated":"2026-07-01 11:00:57.000000000","message":"The spec does not address what happens when a ComposablePlanner subclass references a transformer name in its chain that is not installed (missing stevedore plugin). This is an important error-handling edge case.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Defining the failure mode for missing transformers prevents implementation ambiguity and ensures operators get a clear error rather than a silent failure or traceback during audit execution.\n\n**Recommendation**:\nAdd a sentence specifying that if a transformer in the chain cannot be loaded via stevedore, the planner should raise a clear configuration error at planner initialization or audit time, rather than silently skipping it.","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"62e50ad7900b448814ff654f84b67be6beff534a","unresolved":false,"context_lines":[{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    class WeightComposablePlanner(ComposablePlanner):"},{"line_number":214,"context_line":"        \"\"\"Reproduces current weight planner behavior.\"\"\""},{"line_number":215,"context_line":"        transformers \u003d [\u0027weight_order\u0027, \u0027parallelization\u0027]"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    class InterleaveComposablePlanner(ComposablePlanner):"}],"source_content_type":"text/x-rst","patch_set":3,"id":"730afa9e_a22958a7","line":215,"in_reply_to":"20c0e170_9a4793c5","updated":"2026-07-02 08:01:05.000000000","message":"I think, that\u0027s too low level detail for the spec, but imo, the audit should fail.","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e349246758cf85e51e4d875bd9c35547a5bfef50","unresolved":false,"context_lines":[{"line_number":381,"context_line":"services."},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"Additionally, each transformer may accept additional named arguments"},{"line_number":384,"context_line":"that can be used ty the ComposablePlanners at invocation time when the"},{"line_number":385,"context_line":"required value depends on audit parameters, i.e."},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"Transformer Registration and Discovery"}],"source_content_type":"text/x-rst","patch_set":3,"id":"490aeb68_27daa423","line":384,"updated":"2026-07-01 11:00:57.000000000","message":"Typo on line 384: \u0027used ty the ComposablePlanners\u0027 should read \u0027used by the ComposablePlanners\u0027.\n\n**Severity**: WARNING | **Confidence**: 1.0\n\n**Impact**: Spelling error in a technical specification reduces professional quality and may confuse non-native English readers.\n\n**Suggestion**:\nReplace \u0027used ty the ComposablePlanners\u0027 with \u0027used by the ComposablePlanner\u0027 (or \u0027ComposablePlanners\u0027 depending on intended plurality).","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"62e50ad7900b448814ff654f84b67be6beff534a","unresolved":false,"context_lines":[{"line_number":381,"context_line":"services."},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"Additionally, each transformer may accept additional named arguments"},{"line_number":384,"context_line":"that can be used ty the ComposablePlanners at invocation time when the"},{"line_number":385,"context_line":"required value depends on audit parameters, i.e."},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"Transformer Registration and Discovery"}],"source_content_type":"text/x-rst","patch_set":3,"id":"6ce242a7_11ec172a","line":384,"in_reply_to":"490aeb68_27daa423","updated":"2026-07-02 08:01:05.000000000","message":"Done","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e349246758cf85e51e4d875bd9c35547a5bfef50","unresolved":false,"context_lines":[{"line_number":539,"context_line":"  * ``max_parallel``: comma-separated ``type:count`` pairs specifying"},{"line_number":540,"context_line":"    the maximum number of concurrent actions per type."},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"``nova_service_state_lifecycle``"},{"line_number":543,"context_line":"  Inspects ``change_nova_service_state`` actions and, based on a"},{"line_number":544,"context_line":"  configurable target state parameter, adds parent dependencies to"},{"line_number":545,"context_line":"  ensure that those actions run after migrations affecting the same"}],"source_content_type":"text/x-rst","patch_set":3,"id":"7c971927_4904b395","line":542,"updated":"2026-07-01 11:00:57.000000000","message":"The nova_service_state_lifecycle transformer (lines 542-552) is the only built-in transformer with no configuration options documented. The description mentions a \u0027configurable target state parameter\u0027 but no config option is listed.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Documenting the configuration option ensures the transformer is fully specified and implementable without guesswork about the target state parameter format.\n\n**Recommendation**:\nAdd a Configuration options subsection listing the target state parameter (e.g., target_state: the Nova service state that triggers parent dependency creation, such as \u0027disabled\u0027 or \u0027enabled\u0027).","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e349246758cf85e51e4d875bd9c35547a5bfef50","unresolved":false,"context_lines":[{"line_number":539,"context_line":"  * ``max_parallel``: comma-separated ``type:count`` pairs specifying"},{"line_number":540,"context_line":"    the maximum number of concurrent actions per type."},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"``nova_service_state_lifecycle``"},{"line_number":543,"context_line":"  Inspects ``change_nova_service_state`` actions and, based on a"},{"line_number":544,"context_line":"  configurable target state parameter, adds parent dependencies to"},{"line_number":545,"context_line":"  ensure that those actions run after migrations affecting the same"}],"source_content_type":"text/x-rst","patch_set":3,"id":"c3dd8277_cdd8796b","line":542,"updated":"2026-07-01 11:00:57.000000000","message":"The nova_service_state_lifecycle transformer is registered as an entry point and documented as built-in, but no ComposablePlanner subclass includes it in its chain. No planner uses it and no design description explains its activation path.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: Implementation ambiguity: a developer following this spec will not know which composable planner chain should include this transformer. The node resource consolidation use case is described but no planner subclass is defined for it.\n\n**Priority**: Before merge\n**Why This Matters**: This is the only built-in transformer that reproduces node resource consolidation planner logic, yet it has no corresponding composable planner subclass. Without a defined chain, the transformer cannot be exercised and becomes dead code on arrival.\n\n**Recommendation**:\nEither define a third ComposablePlanner subclass (e.g., NodeResourceConsolidationComposablePlanner with chain [\u0027weight_order\u0027, \u0027nova_service_state_lifecycle\u0027, \u0027parallelization\u0027]) and register it, or explicitly state this transformer is provided for future or out-of-tree planner chains. Also document its configuration options, which are currently missing.","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"62e50ad7900b448814ff654f84b67be6beff534a","unresolved":false,"context_lines":[{"line_number":539,"context_line":"  * ``max_parallel``: comma-separated ``type:count`` pairs specifying"},{"line_number":540,"context_line":"    the maximum number of concurrent actions per type."},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"``nova_service_state_lifecycle``"},{"line_number":543,"context_line":"  Inspects ``change_nova_service_state`` actions and, based on a"},{"line_number":544,"context_line":"  configurable target state parameter, adds parent dependencies to"},{"line_number":545,"context_line":"  ensure that those actions run after migrations affecting the same"}],"source_content_type":"text/x-rst","patch_set":3,"id":"a8d15bbb_2c61c2e3","line":542,"in_reply_to":"7c971927_4904b395","updated":"2026-07-02 08:01:05.000000000","message":"I\u0027ve finally changed my original idea and in new PS I\u0027m proposing to implement this use case via a more gereric weight_order parameter-aware implementation. See the new weight_order.","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e349246758cf85e51e4d875bd9c35547a5bfef50","unresolved":false,"context_lines":[{"line_number":813,"context_line":".. _Watcher Architecture:"},{"line_number":814,"context_line":"   https://docs.openstack.org/watcher/latest/architecture.html"},{"line_number":815,"context_line":".. _Planner Plugin Development:"},{"line_number":816,"context_line":"   https://docs.openstack.org/watcher/latest/contributor/plugin/planner-plugin.html"},{"line_number":817,"context_line":".. _Audit Pipeline:"},{"line_number":818,"context_line":"   https://blueprints.launchpad.net/watcher/+spec/audit-pipeline"},{"line_number":819,"context_line":".. _Solution:"}],"source_content_type":"text/x-rst","patch_set":3,"id":"c8882b9c_a15c2437","line":816,"updated":"2026-07-01 11:00:57.000000000","message":"Line 816 exceeds the 79-column wrapping limit required by the spec template (line 50 of template.rst). The URL reference is 83 characters wide.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Violates the explicit 79-column wrapping rule from the project template. While a single URL is minor, it sets a precedent for ignoring the wrapping guideline.\n\n**Suggestion**:\nRST link target URLs generally cannot be split across lines reliably. Consider shortening via an OpenStack redirect, or accept the single-line URL as a documented exception since it is inside a link target block.","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"62e50ad7900b448814ff654f84b67be6beff534a","unresolved":false,"context_lines":[{"line_number":813,"context_line":".. _Watcher Architecture:"},{"line_number":814,"context_line":"   https://docs.openstack.org/watcher/latest/architecture.html"},{"line_number":815,"context_line":".. _Planner Plugin Development:"},{"line_number":816,"context_line":"   https://docs.openstack.org/watcher/latest/contributor/plugin/planner-plugin.html"},{"line_number":817,"context_line":".. _Audit Pipeline:"},{"line_number":818,"context_line":"   https://blueprints.launchpad.net/watcher/+spec/audit-pipeline"},{"line_number":819,"context_line":".. _Solution:"}],"source_content_type":"text/x-rst","patch_set":3,"id":"b9ab8bdb_e40147dd","line":816,"in_reply_to":"c8882b9c_a15c2437","updated":"2026-07-02 08:01:05.000000000","message":"Done","commit_id":"7995eae195c91960e4b2e4286de0d19c6d1cd79e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c5666f1dec9e8f43b4e322738e6d1b41b829dcf7","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    class ComposablePlanner(BasePlanner):"},{"line_number":202,"context_line":"        \"\"\"Abstract base for transformer-based planners.\"\"\""},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        @abc.abstractproperty"},{"line_number":205,"context_line":"        def transformers(self):"},{"line_number":206,"context_line":"            \"\"\"Return ordered list of transformer plugin names.\"\"\""},{"line_number":207,"context_line":"            raise NotImplementedError()"}],"source_content_type":"text/x-rst","patch_set":4,"id":"00a65ef4_9b19a474","line":204,"updated":"2026-07-02 08:12:58.000000000","message":"@abc.abstractproperty is deprecated since Python 3.3. The ComposablePlanner example uses @abc.abstractproperty for the transformers property (line 204), propagating a deprecated pattern to future implementers.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Spec examples propagate a deprecated decorator pattern to future implementers; functionally harmless but sets a poor precedent.\n\n**Suggestion**:\nReplace @abc.abstractproperty with @property combined with @abc.abstractmethod stacking in the ComposablePlanner example.","commit_id":"accbdfaaf8a0e9b9af49cef6c1c12a0287d22ba6"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c5666f1dec9e8f43b4e322738e6d1b41b829dcf7","unresolved":false,"context_lines":[{"line_number":206,"context_line":"            \"\"\"Return ordered list of transformer plugin names.\"\"\""},{"line_number":207,"context_line":"            raise NotImplementedError()"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        def schedule(self, context, audit_id, solution):"},{"line_number":210,"context_line":"            # Steps 1-6 as described below"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"3bac43ea_ec486aec","line":209,"updated":"2026-07-02 08:12:58.000000000","message":"Planner interface method name \u0027schedule\u0027 (line 209) is not validated against the actual BasePlanner abstract method signature in watcher.decision_engine.planner.base, risking a mismatch.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: If the real method is named differently (e.g., \u0027plan\u0027 or \u0027create_plan\u0027) or has a different signature, developers following the spec will produce code that does not integrate with BasePlanner.\n\n**Suggestion**:\nVerify the exact BasePlanner abstract method name and signature against watcher source, correct the example if needed, and add a note confirming the method overrides the existing planner entry point.","commit_id":"accbdfaaf8a0e9b9af49cef6c1c12a0287d22ba6"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c5666f1dec9e8f43b4e322738e6d1b41b829dcf7","unresolved":false,"context_lines":[{"line_number":380,"context_line":"coupling them to a specific strategy implementation or calling external"},{"line_number":381,"context_line":"services."},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"Additionally, each transformer may accept additional named arguments"},{"line_number":384,"context_line":"that can be used by the ComposablePlanners at invocation time when the"},{"line_number":385,"context_line":"required value depends on audit parameters, i.e."},{"line_number":386,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"8fa45b53_d2c43558","line":383,"updated":"2026-07-02 08:12:58.000000000","message":"Dangling/incomplete sentence ending with \u0027i.e.\u0027 at lines 383-385. The paragraph introduces invocation-time transformer kwargs but never continues; the next line is a new section header, leaving the interface contract undefined.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: The interface contract for invocation-time kwargs is left undefined, creating ambiguity for implementers who follow the spec.\n\n**Priority**: Before merge\n**Why This Matters**: Implementers following the spec will not understand how invocation-time parameters are passed to transformers, risking divergent or broken implementations.\n\n**Recommendation**:\nComplete the sentence with a concrete example (e.g., a code snippet showing a planner passing **kwargs to transform()), or remove the paragraph if invocation parameters are out of scope for this spec.","commit_id":"accbdfaaf8a0e9b9af49cef6c1c12a0287d22ba6"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ac6ab3baeb2874707759ab75ef2bf303c05d6196","unresolved":true,"context_lines":[{"line_number":380,"context_line":"coupling them to a specific strategy implementation or calling external"},{"line_number":381,"context_line":"services."},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"Additionally, each transformer may accept additional named arguments"},{"line_number":384,"context_line":"that can be used by the ComposablePlanners at invocation time when the"},{"line_number":385,"context_line":"required value depends on audit parameters, i.e."},{"line_number":386,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"86d7ebe6_494e7183","line":383,"in_reply_to":"8fa45b53_d2c43558","updated":"2026-07-02 10:05:43.000000000","message":"the sentence does look like it was cut midway","commit_id":"accbdfaaf8a0e9b9af49cef6c1c12a0287d22ba6"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c5666f1dec9e8f43b4e322738e6d1b41b829dcf7","unresolved":false,"context_lines":[{"line_number":678,"context_line":"better, as deployers can select composable planners tailored to their"},{"line_number":679,"context_line":"environment."},{"line_number":680,"context_line":""},{"line_number":681,"context_line":"Performance Impact"},{"line_number":682,"context_line":"------------------"},{"line_number":683,"context_line":""},{"line_number":684,"context_line":"Each transformer adds a small computational overhead during the planning"}],"source_content_type":"text/x-rst","patch_set":4,"id":"3d4ca8ac_432986a0","line":681,"updated":"2026-07-02 08:12:58.000000000","message":"Heading capitalization inconsistency: \u0027Performance Impact\u0027 is title-cased while all sibling subsection headings use sentence case (\u0027Data model impact\u0027, \u0027REST API impact\u0027, etc.), deviating from the template pattern.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Minor inconsistency with project template conventions; cosmetic but should match sibling headings for uniformity.\n\n**Suggestion**:\nChange \u0027Performance Impact\u0027 to \u0027Performance impact\u0027 to match the sentence-case pattern of all other subsection headings and the template.","commit_id":"accbdfaaf8a0e9b9af49cef6c1c12a0287d22ba6"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c5666f1dec9e8f43b4e322738e6d1b41b829dcf7","unresolved":false,"context_lines":[{"line_number":693,"context_line":"are required. The performance impact of each new transformer should be"},{"line_number":694,"context_line":"carefully considered when designing and coding new transformers."},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"Other deployer impact"},{"line_number":697,"context_line":"---------------------"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"New configuration options will be added:"}],"source_content_type":"text/x-rst","patch_set":4,"id":"a762f3bb_79162a62","line":696,"updated":"2026-07-02 08:12:58.000000000","message":"No explicit upgrade/deprecation path for the existing per-strategy _planner attribute in the \u0027Other deployer impact\u0027 section (lines 696-711), leaving the transition window and fallback handling unclear.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Clearer operator guidance on the transition window and deprecation schedule for the built-in _planner fallback.\n\n**Recommendation**:\nAdd a sentence to \u0027Other deployer impact\u0027 stating the intended deprecation cycle (e.g., \u0027the built-in _planner fallback will be retained for at least N releases and deprecated with a standard Oslo deprecation warning before removal\u0027).","commit_id":"accbdfaaf8a0e9b9af49cef6c1c12a0287d22ba6"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c5666f1dec9e8f43b4e322738e6d1b41b829dcf7","unresolved":false,"context_lines":[{"line_number":842,"context_line":""},{"line_number":843,"context_line":"* `Watcher Architecture`_"},{"line_number":844,"context_line":"* `Planner Plugin Development`_"},{"line_number":845,"context_line":"* `Audit Pipeline Spec`__"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":".. _Watcher Architecture:"},{"line_number":848,"context_line":"   https://docs.openstack.org/watcher/latest/architecture.html"}],"source_content_type":"text/x-rst","patch_set":4,"id":"9da625a8_06d8f646","line":845,"updated":"2026-07-02 08:12:58.000000000","message":"Cross-reference to the Audit Pipeline spec uses a mix of named and anonymous hyperlink targets (lines 845-865), referencing the same launchpad URL through two mechanisms, which is unnecessarily complex.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Simpler, more maintainable cross-reference structure with less chance of Sphinx warnings.\n\n**Recommendation**:\nUnify on a single named reference target for the Audit Pipeline spec throughout the document, removing the redundant anonymous alias.","commit_id":"accbdfaaf8a0e9b9af49cef6c1c12a0287d22ba6"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":142,"context_line":"in each strategy class (e.g., ``\u0027weight\u0027`` for most strategies,"},{"line_number":143,"context_line":"``\u0027workload_stabilization\u0027`` for the workload stabilization strategy)."},{"line_number":144,"context_line":"There is an existing ``[watcher_planner]`` configuration section with a"},{"line_number":145,"context_line":"``planner`` option, but it is not used in the actual selection flow."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"This spec proposes making planner selection **operator-configurable**"},{"line_number":148,"context_line":"through the ``[watcher_planner]`` configuration section. The operator can"}],"source_content_type":"text/x-rst","patch_set":5,"id":"6c267fce_132dd596","line":145,"updated":"2026-07-02 10:50:02.000000000","message":"The spec states the existing \u0027[watcher_planner] planner\u0027 option \u0027is not used in the actual selection flow\u0027 (lines 144-145) as an unqualified fact about the current codebase.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: This is a load-bearing claim that justifies the new selection design, but the Watcher source is not present in this change to verify it. If the option is in fact consulted somewhere, the rationale is weakened.\n\n**Suggestion**:\nSoften to \u0027is not consulted during per-strategy planner selection\u0027 or add a reference to the code path that confirms the claim, so reviewers can verify it during implementation.","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":158,"context_line":"    zone_migration_planner \u003d weight_composable"},{"line_number":159,"context_line":"    workload_stabilization_planner \u003d interleave_composable"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"The resolution order for which planner to use is:"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"1. **Per-strategy config override** (e.g., ``zone_migration_planner``),"},{"line_number":164,"context_line":"   if set."}],"source_content_type":"text/x-rst","patch_set":5,"id":"acf6600e_e6857fcb","line":161,"updated":"2026-07-02 10:50:02.000000000","message":"The spec does not specify what happens when a configured planner override references a planner name that is not registered as a stevedore plugin.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Defining the error behavior (fail fast with a clear message vs. fall back to the strategy default) removes an ambiguity that could cause silent misselection or confusing crashes at audit time.\n\n**Recommendation**:\nAdd one sentence to the Planner Selection resolution order, e.g. \u0027If a configured planner name is not registered, the planner loader raises an error and the audit fails with a descriptive message rather than silently falling back.\u0027","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":297,"context_line":"* **Change insertion order.** A transformer may return the actions in a"},{"line_number":298,"context_line":"  different order than it received them. The planner will preserve this"},{"line_number":299,"context_line":"  order when persisting actions to the Action Plan. This is how a"},{"line_number":300,"context_line":"  interleave transformer distributes migrations across source hosts: by"},{"line_number":301,"context_line":"  interleaving actions so that consecutive actions in insertion order come"},{"line_number":302,"context_line":"  from different hosts, the parallelization window naturally spreads load"},{"line_number":303,"context_line":"  across hosts rather than draining one host at a time."}],"source_content_type":"text/x-rst","patch_set":5,"id":"8610dd1c_47d94375","line":300,"updated":"2026-07-02 10:50:02.000000000","message":"\u0027a interleave transformer\u0027 should be \u0027an interleave transformer\u0027 (indefinite article).\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Minor grammar error in published specification text.\n\n**Suggestion**:\nChange \u0027This is how a interleave transformer distributes\u0027 to \u0027This is how an interleave transformer distributes\u0027.","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":315,"context_line":"* **Add or remove actions.** The returned list must contain exactly the"},{"line_number":316,"context_line":"  same actions as the input list. A transformer that adds new actions,"},{"line_number":317,"context_line":"  drops existing ones, or returns duplicates is a programming error."},{"line_number":318,"context_line":"  The planner will detect and reject such results by comparing the set of"},{"line_number":319,"context_line":"  action UUIDs before and after the transformation."},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"* **Modify action parameters.** A transformer must not change the action"}],"source_content_type":"text/x-rst","patch_set":5,"id":"70ca85f4_24b39a64","line":318,"updated":"2026-07-02 10:50:02.000000000","message":"The transformer contract forbids removing/adding actions and states the planner \u0027will detect and reject\u0027 mismatched UUID sets (lines 318-319), but does not specify the failure mode.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Stating whether a contract violation raises immediately, skips the offending transformer, or aborts the whole Action Plan makes the behavior predictable for transformer authors and operators.\n\n**Recommendation**:\nSpecify that contract violations raise an exception that aborts audit execution and is logged with the transformer name and the differing UUIDs.","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":384,"context_line":"Additionally, each transformer may accept additional named arguments"},{"line_number":385,"context_line":"that can be used by the ComposablePlanners at invocation time when the"},{"line_number":386,"context_line":"required value depends on audit parameters or other use cases where static"},{"line_number":387,"context_line":"configuration per-strategy is not appropiate."},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"Transformer Registration and Discovery"},{"line_number":390,"context_line":"--------------------------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"a4e7d85d_15fbe028","line":387,"updated":"2026-07-02 10:50:02.000000000","message":"Spelling error: \u0027appropiate\u0027 should be \u0027appropriate\u0027.\n\n**Severity**: WARNING | **Confidence**: 1.0\n\n**Impact**: Typo in a specification document that will be published; undermines the professional polish of an otherwise well-written spec.\n\n**Suggestion**:\nChange \u0027configuration per-strategy is not appropiate\u0027 to \u0027configuration per-strategy is not appropriate\u0027.","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":434,"context_line":"3. **Transformer\u0027s in-code defaults** (hardcoded in the transformer"},{"line_number":435,"context_line":"   class)."},{"line_number":436,"context_line":""},{"line_number":437,"context_line":"The composable planner resolves the strategy name from"},{"line_number":438,"context_line":"``solution.strategy.name`` and passes the resolved configuration to each"},{"line_number":439,"context_line":"transformer in the chain."},{"line_number":440,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"b226415e_ffd593e6","line":437,"updated":"2026-07-02 10:50:02.000000000","message":"The composable planner is said to resolve the strategy name from solution.strategy.name (lines 437-438), but no fallback is defined if that attribute is unset or None.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Defining the fallback (use in-code defaults only, or raise) prevents an implementation-time decision that could diverge from reviewer expectations.\n\n**Recommendation**:\nAdd: \u0027If solution.strategy.name is unavailable, only the transformer in-code defaults and default_\u003cparam\u003e options are used; per-strategy overrides are skipped.\u0027","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":452,"context_line":"group."},{"line_number":453,"context_line":""},{"line_number":454,"context_line":"This follows the same dynamic registration pattern used by"},{"line_number":455,"context_line":"``DefaultLoader._load_plugin_config()`` for planner and strategy options."},{"line_number":456,"context_line":"New strategies and transformers are automatically picked up without any"},{"line_number":457,"context_line":"manual option registration."},{"line_number":458,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"e4a39c57_4b0a81c4","line":455,"updated":"2026-07-02 10:50:02.000000000","message":"The claim that dynamic registration \u0027follows the same pattern used by DefaultLoader._load_plugin_config()\u0027 (lines 454-455) is presented as fact without a code reference.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: If the actual DefaultLoader behavior differs (e.g. it does not register per-strategy prefixed options), the proposed dynamic registration design may not be directly reusable, creating implementation risk.\n\n**Suggestion**:\nCite the module path (e.g. watcher.decision_engine.loading.default.DefaultLoader) so the claim can be verified, or soften to \u0027is inspired by the pattern in DefaultLoader\u0027.","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":524,"context_line":"  Sorts actions by a configurable priority and sets parent dependencies"},{"line_number":525,"context_line":"  between weight groups. All actions in a higher-weight group must"},{"line_number":526,"context_line":"  complete before actions in a lower-weight group can start. This"},{"line_number":527,"context_line":"  reproduces the type-based ordering currently embedded in the weight"},{"line_number":528,"context_line":"  planner."},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"  Weight entries support two formats:"}],"source_content_type":"text/x-rst","patch_set":5,"id":"5fc86b5f_821857c9","line":527,"updated":"2026-07-02 10:50:02.000000000","message":"weight_order is described as reproducing the existing weight planner, but the parameter-aware type.param.value:weight syntax (lines 530-547) is new behavior the existing planner does not have.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: The \u0027reproduces the type-based ordering\u0027 claim (line 527) and equivalence-test claim (lines 794-795) overstate equivalence: the new parameter-level matching has no current-planner counterpart, so equivalence holds only when parameter-level entries are absent.\n\n**Suggestion**:\nSoften to \u0027reproduces the type-based ordering ... and additionally supports parameter-level sub-type weighting\u0027, and note in Testing that equivalence is asserted for type-level-only configurations.","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":531,"context_line":""},{"line_number":532,"context_line":"  * **Type-level**: ``action_type:weight`` — assigns a weight to all"},{"line_number":533,"context_line":"    actions of that type."},{"line_number":534,"context_line":"  * **Type and Parameter-level**: ``action_type.param.value:weight`` —"},{"line_number":535,"context_line":"    assigns a weight to actions of that type where"},{"line_number":536,"context_line":"    ``input_parameters[param] \u003d\u003d value``."},{"line_number":537,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"9b8c5bd6_791541fb","line":534,"updated":"2026-07-02 10:50:02.000000000","message":"The node_resource_consolidation example uses change_nova_service_state.state.enabled (lines 480, 559) as the parameter path, but the actual input_parameters key/value of the change_nova_service_state action is not verified in this spec.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: If the real action stores the state under a different key (e.g. \u0027state\u0027 with value True/False rather than \u0027enabled\u0027/\u0027disabled\u0027), the documented type.param.value syntax example would be wrong and mislead implementers.\n\n**Suggestion**:\nAdd a one-line note that the exact parameter name/value is illustrative and will be confirmed against the ChangeNovaServiceState action definition during implementation, or correct it to the real key.","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":569,"context_line":""},{"line_number":570,"context_line":"  Configuration options:"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"  * ``weights``: comma-separated weight entries. Each entry is either"},{"line_number":573,"context_line":"    ``type:weight`` or ``type.param.value:weight``. Higher values are"},{"line_number":574,"context_line":"    executed first."},{"line_number":575,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"4e86d9e9_9c671fc9","line":572,"updated":"2026-07-02 10:50:02.000000000","message":"The per-transformer option names documented in the \u0027Configuration options\u0027 bullets do not match the actual registered option names shown in every config example and implied by the default_\u003cparam\u003e/\u003cstrategy\u003e_\u003cparam\u003e registration pattern.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: Dynamic registration (lines 444-452) creates default_\u003cparam\u003e/\u003cstrategy\u003e_\u003cparam\u003e keys (e.g. default_weights, default_max_parallel), but the Configuration options bullets document bare \u0027weights\u0027 (572) and \u0027max_parallel\u0027 (587), which are never registered and silently ignored.\n\n**Priority**: Before merge\n**Why This Matters**: Implementers and operators reading the Configuration options section will use option names that do not exist, producing silently-ignored config and hard-to-diagnose behavior.\n\n**Recommendation**:\nReconcile the documented option names with the registration pattern: document them as default_weights/\u003cstrategy\u003e_weights and default_max_parallel/\u003cstrategy\u003e_max_parallel (matching the examples and the default_\u003cparam\u003e rule), or state that \u003cparam\u003e expands to weights/max_parallel and only the prefixed forms are registered.","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":597,"context_line":""},{"line_number":598,"context_line":"  Configuration options:"},{"line_number":599,"context_line":""},{"line_number":600,"context_line":"  * ``action_types``: comma-separated list of action types to which"},{"line_number":601,"context_line":"    interleave applies. Default: ``migrate``."},{"line_number":602,"context_line":""},{"line_number":603,"context_line":"Additional transformers can be developed as out-of-tree plugins or"}],"source_content_type":"text/x-rst","patch_set":5,"id":"92bed228_753843cf","line":600,"updated":"2026-07-02 10:50:02.000000000","message":"The source_host_interleave option name is inconsistent between the config example (\u0027default_action_types\u0027, line 489) and the documented option name (\u0027action_types\u0027, line 600).\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: Same mismatch as weights/max_parallel: under the default_\u003cparam\u003e/\u003cstrategy\u003e_\u003cparam\u003e registration pattern the registered key is default_action_types (as the example shows), but the documented bullet names only \u0027action_types\u0027, which would not exist.\n\n**Priority**: Before merge\n**Why This Matters**: Operators will write \u0027action_types \u003d migrate\u0027 and have it silently ignored instead of \u0027default_action_types \u003d migrate\u0027.\n\n**Recommendation**:\nDocument the option as default_action_types/\u003cstrategy\u003e_action_types, or explicitly state that \u003cparam\u003e \u003d action_types and only the prefixed forms are registered.","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":853,"context_line":""},{"line_number":854,"context_line":"* `Watcher Architecture`_"},{"line_number":855,"context_line":"* `Planner Plugin Development`_"},{"line_number":856,"context_line":"* `Audit Pipeline Spec`__"},{"line_number":857,"context_line":""},{"line_number":858,"context_line":".. _Watcher Architecture:"},{"line_number":859,"context_line":"   https://docs.openstack.org/watcher/latest/architecture.html"}],"source_content_type":"text/x-rst","patch_set":5,"id":"080c2095_eab7411d","line":856,"updated":"2026-07-02 10:50:02.000000000","message":"The reference display text \u0027Audit Pipeline Spec\u0027 (line 856) points at a target whose defined identity is \u0027Audit Pipeline\u0027, and the target is undefined (see the high-severity finding), making the rendered text ambiguous.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Once the label is defined, the bullet will read \u0027Audit Pipeline Spec\u0027 but link to the Audit Pipeline spec; minor, but worth aligning the label and display text.\n\n**Suggestion**:\nDefine a single \u0027.. _Audit Pipeline:\u0027 label and reference it as `Audit Pipeline`_ consistently in the References section and body (lines 51, 811).","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8124a0af37091c3ce476e9bb130e638203bb4fa9","unresolved":false,"context_lines":[{"line_number":871,"context_line":".. _stevedore:"},{"line_number":872,"context_line":"   https://docs.openstack.org/stevedore/latest/"},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"__ `Audit Pipeline`_"},{"line_number":875,"context_line":""},{"line_number":876,"context_line":""},{"line_number":877,"context_line":"History"}],"source_content_type":"text/x-rst","patch_set":5,"id":"c6a034b1_7ff92933","line":874,"updated":"2026-07-02 10:50:02.000000000","message":"The \u0027Audit Pipeline\u0027 hyperlink label is used in three places but never defined in the file with a \u0027.. _Audit Pipeline:\u0027 target, so all references are broken.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: docutils emits \u0027Unknown target name: Audit Pipeline\u0027 warnings and renders the references as plain text instead of links. Lines 51, 811, and 856 all fail to resolve to a URL.\n\n**Priority**: Before merge\n**Why This Matters**: The spec cannot link to the dependent Audit Pipeline spec it repeatedly references, and the RST no longer builds cleanly, which the template explicitly requires.\n\n**Recommendation**:\nAdd an explicit label target, e.g. \u0027.. _Audit Pipeline:\\n   https://review.opendev.org/.../audit-pipeline.rst\u0027 near the other label definitions (line 858), then use it consistently. Remove the redundant \u0027__ `Audit Pipeline`_\u0027 anonymous target at line 874 in favor of the named label.","commit_id":"b335aae96de7d2dd4d9a0ed800b04b4e3b5fb6e2"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8d2097f395b4e604f139e6469134fae1500a9035","unresolved":false,"context_lines":[{"line_number":297,"context_line":"* **Change insertion order.** A transformer may return the actions in a"},{"line_number":298,"context_line":"  different order than it received them. The planner will preserve this"},{"line_number":299,"context_line":"  order when persisting actions to the Action Plan. This is how a"},{"line_number":300,"context_line":"  interleave transformer distributes migrations across source hosts: by"},{"line_number":301,"context_line":"  interleaving actions so that consecutive actions in insertion order come"},{"line_number":302,"context_line":"  from different hosts, the parallelization window naturally spreads load"},{"line_number":303,"context_line":"  across hosts rather than draining one host at a time."}],"source_content_type":"text/x-rst","patch_set":6,"id":"dcaea80d_3831c70b","line":300,"updated":"2026-07-02 11:53:42.000000000","message":"Grammar: \u0027a interleave transformer\u0027 should be \u0027an interleave transformer\u0027 (vowel sound).\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Minor grammatical error in the Transformer Contract section describing insertion-order behavior; the article \u0027a\u0027 before the vowel-initial \u0027interleave\u0027 reads incorrectly.\n\n**Suggestion**:\nChange \u0027a interleave transformer\u0027 to \u0027an interleave transformer\u0027.","commit_id":"837877c3f2a04a9c0e007efbb9dbb8d25f06f52e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8d2097f395b4e604f139e6469134fae1500a9035","unresolved":false,"context_lines":[{"line_number":384,"context_line":"Additionally, each transformer may accept additional named arguments"},{"line_number":385,"context_line":"that can be used by the ComposablePlanners at invocation time when the"},{"line_number":386,"context_line":"required value depends on audit parameters or other use cases where static"},{"line_number":387,"context_line":"configuration per-strategy is not appropiate."},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"Transformer Registration and Discovery"},{"line_number":390,"context_line":"--------------------------------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"d9d4e835_cc4d6699","line":387,"updated":"2026-07-02 11:53:42.000000000","message":"Typo: \u0027appropiate\u0027 should be \u0027appropriate\u0027 in the transformer invocation-parameters paragraph.\n\n**Severity**: WARNING | **Confidence**: 1.0\n\n**Impact**: Spelling error in a specification that will be referenced by implementers and operators; reduces polish of an otherwise well-written document.\n\n**Suggestion**:\nReplace \u0027not appropiate\u0027 with \u0027not appropriate\u0027.","commit_id":"837877c3f2a04a9c0e007efbb9dbb8d25f06f52e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8d2097f395b4e604f139e6469134fae1500a9035","unresolved":false,"context_lines":[{"line_number":612,"context_line":"------------"},{"line_number":613,"context_line":""},{"line_number":614,"context_line":"**Refactor existing planners in place.** Instead of introducing a new"},{"line_number":615,"context_line":"``composable`` planners, the existing planners could be refactored to"},{"line_number":616,"context_line":"extract their ordering logic into transformers. This was considered but"},{"line_number":617,"context_line":"rejected because it forces all operators to adopt the new architecture"},{"line_number":618,"context_line":"at once, with no way to fall back to the proven planner implementations"}],"source_content_type":"text/x-rst","patch_set":6,"id":"6cbbc374_5a403856","line":615,"updated":"2026-07-02 11:53:42.000000000","message":"Grammar: \u0027introducing a new composable planners\u0027 mixes a singular article with a plural noun.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Improves readability of the Alternatives section, which is otherwise clearly argued.\n\n**Recommendation**:\nRephrase to \u0027introducing new composable planners\u0027 or \u0027introducing a new composable planner\u0027.","commit_id":"837877c3f2a04a9c0e007efbb9dbb8d25f06f52e"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8198bbb1581688cd98db699f95d88c141af75ef0","unresolved":false,"context_lines":[{"line_number":84,"context_line":"A transformer does not add, remove, or modify action type parameters; it"},{"line_number":85,"context_line":"only changes their order, grouping, or dependency relationships."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"This spec also introduces the **composable planner** architecture: an"},{"line_number":88,"context_line":"abstract base class ``ComposablePlanner`` derived from ``BasePlanner``"},{"line_number":89,"context_line":"and implements **all** the ordering and dependency-setting logic through"},{"line_number":90,"context_line":"a transformer chain. Concrete subclasses define their specific transformer"}],"source_content_type":"text/x-rst","patch_set":7,"id":"f1cc1c3a_012e57e1","line":87,"updated":"2026-07-02 12:50:09.000000000","message":"Sentence at lines 87-89 has structural ambiguity: \u0027an abstract base class ComposablePlanner derived from BasePlanner and implements all the ordering...\u0027 The verb \u0027implements\u0027 lacks a clear grammatical subject.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Improves readability of the core architectural description that introduces the ComposablePlanner concept.\n\n**Recommendation**:\nRestructure to clarify the subject, e.g., \u0027an abstract base class ComposablePlanner that derives from BasePlanner and implements all the ordering and dependency-setting logic through a transformer chain.\u0027","commit_id":"95b0a7adf08f59fd139b527a4d539b19d2dd4ac7"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8198bbb1581688cd98db699f95d88c141af75ef0","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"3. Create in-memory Action objects with UUIDs"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"4. New ordering: transformer chain handles ALL ordering"},{"line_number":251,"context_line":"    for transformer in configured_transformers:"},{"line_number":252,"context_line":"        action_objects \u003d transformer.transform(action_objects, solution)"},{"line_number":253,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"aa033e74_cc788e29","line":250,"updated":"2026-07-02 12:50:09.000000000","message":"The transformer chain execution loop (lines 250-253) shows no error handling discussion. If a transformer raises an exception mid-chain, the spec does not define whether partially-transformed actions are persisted or the entire plan is discarded.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Defining error-handling semantics for transformer failures would prevent ambiguity during implementation and ensure consistent behavior across planner subclasses.\n\n**Recommendation**:\nAdd a sentence to the Composable Planner Workflow section clarifying that a transformer exception aborts the planning operation (no actions persisted) and propagates to the audit engine as a planning failure.","commit_id":"95b0a7adf08f59fd139b527a4d539b19d2dd4ac7"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8198bbb1581688cd98db699f95d88c141af75ef0","unresolved":false,"context_lines":[{"line_number":297,"context_line":"* **Change insertion order.** A transformer may return the actions in a"},{"line_number":298,"context_line":"  different order than it received them. The planner will preserve this"},{"line_number":299,"context_line":"  order when persisting actions to the Action Plan. This is how a"},{"line_number":300,"context_line":"  interleave transformer distributes migrations across source hosts: by"},{"line_number":301,"context_line":"  interleaving actions so that consecutive actions in insertion order come"},{"line_number":302,"context_line":"  from different hosts, the parallelization window naturally spreads load"},{"line_number":303,"context_line":"  across hosts rather than draining one host at a time."}],"source_content_type":"text/x-rst","patch_set":7,"id":"90474134_7a5e718a","line":300,"updated":"2026-07-02 12:50:09.000000000","message":"Grammar: \u0027how a interleave transformer\u0027 should use the article \u0027an\u0027 before the vowel-initial word \u0027interleave\u0027.\n\n**Severity**: WARNING | **Confidence**: 1.0\n\n**Impact**: Minor grammar error in the Transformer Contract section, a core part of the spec that developers will read closely.\n\n**Suggestion**:\nChange \u0027how a interleave transformer distributes\u0027 to \u0027how an interleave transformer distributes\u0027.","commit_id":"95b0a7adf08f59fd139b527a4d539b19d2dd4ac7"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8198bbb1581688cd98db699f95d88c141af75ef0","unresolved":false,"context_lines":[{"line_number":302,"context_line":"  from different hosts, the parallelization window naturally spreads load"},{"line_number":303,"context_line":"  across hosts rather than draining one host at a time."},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"* **Set or modify parent dependencies.** A transformer may set the"},{"line_number":306,"context_line":"  ``parents`` field on actions to introduce new ordering constraints that"},{"line_number":307,"context_line":"  cannot be expressed by insertion order alone. For example, a transformer"},{"line_number":308,"context_line":"  that enforces \"drain host X completely before starting migrations from"}],"source_content_type":"text/x-rst","patch_set":7,"id":"ca79d92d_81858526","line":305,"updated":"2026-07-02 12:50:09.000000000","message":"Design gap: transformers can set cumulative parent dependencies but the spec never addresses cycle detection. weight_order sets inter-group parents; parallelization adds inter-chunk parents. Together they can form a circular dependency the applier cannot resolve, causing a deadlocked Action Plan.\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Risk**: A circular parent dependency graph would cause the Watcher applier to deadlock or error at runtime, producing an Action Plan that can never execute. The spec defines UUID-set contract validation but omits equivalent dependency-graph cycle validation.\n\n**Priority**: Before merge\n**Why This Matters**: This is the most important design soundness gap. The transformer contract explicitly enables parent-setting across multiple chained transformers with cumulative (add-only) semantics, making cycles a realistic possibility rather than a theoretical edge case.\n\n**Recommendation**:\nAdd a subsection requiring the planner to validate the final dependency graph via topological sort after the chain completes, rejecting Action Plans with cycles. Include this in the Work Items alongside the existing UUID-set contract validation at line 776.","commit_id":"95b0a7adf08f59fd139b527a4d539b19d2dd4ac7"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8198bbb1581688cd98db699f95d88c141af75ef0","unresolved":false,"context_lines":[{"line_number":384,"context_line":"Additionally, each transformer may accept additional named arguments"},{"line_number":385,"context_line":"that can be used by the ComposablePlanners at invocation time when the"},{"line_number":386,"context_line":"required value depends on audit parameters or other use cases where static"},{"line_number":387,"context_line":"configuration per-strategy is not appropiate."},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"Transformer Registration and Discovery"},{"line_number":390,"context_line":"--------------------------------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"04e314ed_902ed66a","line":387,"updated":"2026-07-02 12:50:09.000000000","message":"Typo: \u0027appropiate\u0027 should be \u0027appropriate\u0027 on line 387.\n\n**Severity**: WARNING | **Confidence**: 1.0\n\n**Impact**: Minor professionalism issue in a specification document that will be referenced by developers and operators.\n\n**Suggestion**:\nReplace \u0027not appropiate\u0027 with \u0027not appropriate\u0027.","commit_id":"95b0a7adf08f59fd139b527a4d539b19d2dd4ac7"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8198bbb1581688cd98db699f95d88c141af75ef0","unresolved":false,"context_lines":[{"line_number":567,"context_line":"  get weight 20. The result is: disable (30) → migrate (20) →"},{"line_number":568,"context_line":"  enable (10)."},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"  Configuration options:"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"  * ``weights``: comma-separated weight entries. Each entry is either"},{"line_number":573,"context_line":"    ``type:weight`` or ``type.param.value:weight``. Higher values are"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3e135012_6ca45e07","line":570,"updated":"2026-07-02 12:50:09.000000000","message":"Config naming inconsistency: Built-in Transformers sections list base names (\u0027weights\u0027 line 572, \u0027max_parallel\u0027 line 587) but operators must use prefixed keys (\u0027default_weights\u0027, \u0027\u003cstrategy\u003e_weights\u0027). A reader could set \u0027weights \u003d ...\u0027 instead of \u0027default_weights \u003d ...\u0027.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Operator confusion when configuring transformers. The per-transformer Configuration options sections describe the abstract parameter name but do not clarify the actual oslo.config key is always prefixed with \u0027default_\u0027 or \u0027\u003cstrategy\u003e_\u0027.\n\n**Suggestion**:\nAdd a note in each Configuration options block clarifying that the listed name is the base parameter and actual config keys follow the default_\u003cparam\u003e / \u003cstrategy\u003e_\u003cparam\u003e pattern. For example: \u0027Base parameter: weights. Config keys: default_weights, \u003cstrategy\u003e_weights.\u0027","commit_id":"95b0a7adf08f59fd139b527a4d539b19d2dd4ac7"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8198bbb1581688cd98db699f95d88c141af75ef0","unresolved":false,"context_lines":[{"line_number":658,"context_line":"the final ordering produced by the planner after transformers have been"},{"line_number":659,"context_line":"applied."},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"Security impact"},{"line_number":662,"context_line":"---------------"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"None. Transformers operate exclusively on in-memory action objects during"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3ae7dfb5_54130093","line":661,"updated":"2026-07-02 12:50:09.000000000","message":"Security impact section claims \u0027None\u0027 but does not address risks from out-of-tree transformer plugins. Transformers execute during planning with access to the full Solution object. A malicious or buggy out-of-tree transformer registered via stevedore could violate contract constraints.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: The contract constraints (MUST NOT call external APIs, MUST NOT persist to DB) are enforced only by convention, not by technical controls. Operators installing out-of-tree transformers have no guarantee these constraints are respected.\n\n**Suggestion**:\nExpand the Security impact section to acknowledge the transformer contract is convention-based for out-of-tree plugins, note that operators should only install transformers from trusted sources, and mention in-tree transformers are validated during code review.","commit_id":"95b0a7adf08f59fd139b527a4d539b19d2dd4ac7"}]}