docs: Align documented behavior with the implementation - #9
Merged
Conversation
Several README and in-app documentation claims described behavior the code does not implement, which is a problem for a sample that readers use as a reference. Corrected claims: - Scope is the ESAM SPE/SPN signal-conditioning exchange, not the whole SCTE-130 specification. - Rules use first-match evaluation; there is no rule chaining. - Roles are admin and user. Operator, Viewer and per-channel ACLs do not exist. - Basic Authentication on /esam is optional and configured per channel. - Stateful mode tracks one active break per channel and does not rely on DynamoDB TTL for expiry. - External actions run inside the ESAM request path, so their timeouts and retries add to encoder response time. - The rule example now uses the real modification target (breakDuration) and documents the supported condition fields, operators and targets. - Cost figures are an indicative estimate, not a quote. - Cleanup covers runtime-created SSM parameters and retained log groups. - The fallback administrator procedure includes group membership, without which the account cannot administer the dashboard. Removed the unsubstantiated "~50 req/s" and "P95 < 150ms" figures from the in-app documentation. No benchmark exists in this repository, so the documentation now describes what to measure instead. Also wires the environment profiles that were declared but never passed to the API stack, so -c env=prod now applies its throttling limits and turns off API Gateway data trace logging, and adds an opt-in least-privilege MediaLive grant through -c mediaLiveChannelArns so the external-action plugin can work without granting account-wide access. Adds a Dashboard section to the README with three dashboard screens, since the repository had no interface images. Added a request path behavior section covering timeouts, throttling and the fail-open paths, so readers understand the failure modes before putting the service in a signal path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Aligns the README and the in-app documentation with what the code actually does, and adds the first dashboard screenshots to the README.
Several documented claims described behavior this repository does not implement. For a sample that readers use as a reference, those are the most costly kind of defect.
Corrected claims
adminanduser; no per-channel scope/esamuses Basic Authenticationcdk destroy --allremoves everythingThe rule example also used
break_duration, which is not a valid modification target. It is nowbreakDuration, matchingModificationTargetinbackend/domain/models/channel.py, and the README documents the supported condition fields, operators, actions and modification targets.The fallback administrator procedure previously listed only
admin-create-user. Withoutadmin-add-user-to-group, the resulting account cannot administer the dashboard.Removed unsubstantiated performance figures
The in-app documentation advertised
~50 req/sandP95 < 150ms. This repository has no benchmark, so those numbers were not supportable. The Metrics page now explains what the CloudWatch dashboard graphs and what the reader should measure. Publishing real percentiles is deliberately left for a separate change with a reproducible methodology.Behavior changes
Environment profiles are now wired to the API stack.
lib/config/environment.tsdeclaredenableDetailedLogging,enableXRayTracingand throttling limits per environment, but onlylogRetentionDaysreachedApiStack, so the README statement about environment profiles was not true. Verified by synth:devprodDataTraceEnabledtruefalseLoggingLevelINFOERRORThis also stops full ESAM XML payloads from being written to CloudWatch in the
prodprofile.Opt-in MediaLive permissions. The MediaLive plugin calls
BatchUpdateSchedule, but the signal-processing role had no MediaLive permissions, so the feature could not work as deployed. Passing-c mediaLiveChannelArns=<arn>[,<arn>]now grantsmedialive:BatchUpdateScheduleandmedialive:DescribeSchedulescoped to those channels. Without the flag, nothing is granted.Dashboard screenshots
The repository had no interface images. The README now shows the Channels page, the rule configuration form and the live SCTE-35 feed. The images were rendered from the application's own compiled stylesheet, component markup and lucide icon set, with fictitious sample data and no account identifiers, ARNs, API IDs or credentials.
Request path behavior
New README section describing the failure modes of the synchronous path: the 30-second Lambda timeout, stage throttling, the
noopfallback for unknown or disabled channels, thedefaultActionfallback on parse failure, fail-open break-state handling, and the fact that encoder behavior on an unreachable POIS is configured on the encoder.Testing
pytest --cov: 176 passed, 68.96% coverage (gate 65%)ruff check .andblack --check .: cleannpx tsc --noEmit,npm run lint,npm run build(frontend): cleannpm run build(infrastructure): cleannpx cdk synthfor thedevandprodAPI stacks, asserting the table above and the scoped MediaLive statementNo functional backend change;
LICENSEandCODE_OF_CONDUCT.mdare untouched.